netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/6] slight optimization of addr compare for some modules
@ 2013-12-16  8:24 Ding Tianhong
  2013-12-16 14:45 ` Joe Perches
  0 siblings, 1 reply; 12+ messages in thread
From: Ding Tianhong @ 2013-12-16  8:24 UTC (permalink / raw)
  To: David S. Miller, Netdev, Joe Perches

Joe Perches add ether_addr_equal_unaligned to test if
possibly unaligned to u16 Ethernet addresses are equal.

If CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set, this uses
the slightly faster generic routine ether_addr_equal,
otherwise this uses memcmp.

So I replace memcmp with ether_addr_equal_unaligned in some place
for slight optimization.

v1->v2: modify the patch 02/06's indentation.

Regards.
Ding

Ding Tianhong (6):
  bonding: use ether_addr_equal_unaligned for bond addr compare
  mac80211: slight optimization of addr compare
  fddi: slight optimization of addr compare
  isdn: slight optimization of addr compare
  pppoe: slight optimization of addr compare
  plip: slight optimization of addr compare

 drivers/isdn/i4l/isdn_net.c    |  4 +--
 drivers/net/bonding/bond_3ad.c | 62 ++++++++++++++++++++++--------------------
 drivers/net/plip/plip.c        |  2 +-
 drivers/net/ppp/pppoe.c        |  5 ++--
 net/802/fddi.c                 |  5 ++--
 net/mac80211/iface.c           |  8 +++---
 6 files changed, 46 insertions(+), 40 deletions(-)

-- 
1.8.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/6] slight optimization of addr compare for some modules
  2013-12-16  8:24 [PATCH net-next v2 0/6] slight optimization of addr compare for some modules Ding Tianhong
@ 2013-12-16 14:45 ` Joe Perches
  2013-12-16 14:53   ` Ding Tianhong
  0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2013-12-16 14:45 UTC (permalink / raw)
  To: Ding Tianhong; +Cc: David S. Miller, Netdev

On Mon, 2013-12-16 at 16:24 +0800, Ding Tianhong wrote:
> Joe Perches add ether_addr_equal_unaligned to test if
> possibly unaligned to u16 Ethernet addresses are equal.
> 
> If CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set, this uses
> the slightly faster generic routine ether_addr_equal,
> otherwise this uses memcmp.
> 
> So I replace memcmp with ether_addr_equal_unaligned in some place
> for slight optimization.

Hi.

These seem like sensible cleanups, thanks, but
my name doesn't need to go into the commit log
multiple times like this.

I suggest something like:

Use the recently added and possibly more efficient
ether_addr_equal_unaligned.

Are you intending to do more of these?

$ git grep -E "\bmemcmp\s*\([^,]*,[^,]*,\s*(ETH_ALEN|6)\s*\)" * | wc -l
299

Perhaps the majority of these should use ether_addr_equal
or ether_addr_equal_unaligned.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/6] slight optimization of addr compare for some modules
  2013-12-16 14:45 ` Joe Perches
@ 2013-12-16 14:53   ` Ding Tianhong
  2013-12-16 15:16     ` Joe Perches
  0 siblings, 1 reply; 12+ messages in thread
From: Ding Tianhong @ 2013-12-16 14:53 UTC (permalink / raw)
  To: Joe Perches, Ding Tianhong; +Cc: David S. Miller, Netdev

于 2013/12/16 22:45, Joe Perches 写道:
> On Mon, 2013-12-16 at 16:24 +0800, Ding Tianhong wrote:
>> Joe Perches add ether_addr_equal_unaligned to test if
>> possibly unaligned to u16 Ethernet addresses are equal.
>>
>> If CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set, this uses
>> the slightly faster generic routine ether_addr_equal,
>> otherwise this uses memcmp.
>>
>> So I replace memcmp with ether_addr_equal_unaligned in some place
>> for slight optimization.
> 
> Hi.
> 
> These seem like sensible cleanups, thanks, but
> my name doesn't need to go into the commit log
> multiple times like this.
> 
> I suggest something like:
> 
> Use the recently added and possibly more efficient
> ether_addr_equal_unaligned.
> 
> Are you intending to do more of these?
> 
> $ git grep -E "\bmemcmp\s*\([^,]*,[^,]*,\s*(ETH_ALEN|6)\s*\)" * | wc -l
> 299
> 
> Perhaps the majority of these should use ether_addr_equal
> or ether_addr_equal_unaligned.
> 
> 

yes, it is a juge work to review the whole places and I think it should be
finished by several times, maybe start from this patchset.

I will modify the changelog and simplify it. Thanks.

Regards
Ding


> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/6] slight optimization of addr compare for some modules
  2013-12-16 14:53   ` Ding Tianhong
@ 2013-12-16 15:16     ` Joe Perches
  2013-12-16 17:25       ` Joe Perches
  0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2013-12-16 15:16 UTC (permalink / raw)
  To: Ding Tianhong; +Cc: Ding Tianhong, David S. Miller, Netdev

On Mon, 2013-12-16 at 22:53 +0800, Ding Tianhong wrote:
> 于 2013/12/16 22:45, Joe Perches 写道:
> > Are you intending to do more of these?
> > 
> > $ git grep -E "\bmemcmp\s*\([^,]*,[^,]*,\s*(ETH_ALEN|6)\s*\)" * | wc -l
> > 299
> > 
> > Perhaps the majority of these should use ether_addr_equal
> > or ether_addr_equal_unaligned.
>
> yes, it is a juge work to review the whole places and I think it should be
> finished by several times, maybe start from this patchset.

coccinelle (aka: spatch) can help find and change these

$ cat ether_addr_equal_unaligned.cocci
@@
expression e1;
expression e2;
@@

-	!memcmp(e1, e2, 6)
+	ether_addr_equal_unaligned(e1, e2)

@@
expression e1;
expression e2;
@@

-	memcmp(e1, e2, 6) == 0
+	ether_addr_equal_unaligned(e1, e2)

@@
expression e1;
expression e2;
@@

-	memcmp(e1, e2, 6)
+	!ether_addr_equal_unaligned(e1, e2)

@@
expression e1;
expression e2;
@@

-	memcmp(e1, e2, 6) != 0
+	!ether_addr_equal_unaligned(e1, e2)

$ spatch -sp-file ether_addr_equal.cocci drivers/media/dvb-core/dvb_net.c
init_defs_builtins: /usr/local/share/coccinelle/standard.h
HANDLING: drivers/media/dvb-core/dvb_net.c
diff = 
--- drivers/media/dvb-core/dvb_net.c
+++ /tmp/cocci-output-18744-96d16c-dvb_net.c
@@ -837,7 +837,8 @@ static void dvb_net_sec(struct net_devic
 	}
 	if (pkt[5] & 0x02) {
 		/* handle LLC/SNAP, see rfc-1042 */
-		if (pkt_len < 24 || memcmp(&pkt[12], "\xaa\xaa\x03\0\0\0", 6)) {
+		if (pkt_len < 24 || !ether_addr_equal_unaligned(&pkt[12],
+								"\xaa\xaa\x03\0\0\0")) {
 			stats->rx_dropped++;
 			return;
 		}

I presume many of these should be ether_addr_equal
and not ether_addr_equal_unaligned, but I can't
think of a way to automate that easily.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/6] slight optimization of addr compare for some modules
  2013-12-16 15:16     ` Joe Perches
@ 2013-12-16 17:25       ` Joe Perches
  2013-12-17  1:58         ` Ding Tianhong
  0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2013-12-16 17:25 UTC (permalink / raw)
  To: Ding Tianhong; +Cc: Ding Tianhong, David S. Miller, Netdev

Hi again Ding.

These should still be inspected for appropriate use of
ether_addr_equal or ether_addr_equal_unaligned, but a
better cocci input sp-file is:

$ cat ether_addr_equal_unaligned.cocci 
@@
expression e1;
expression e2;
@@

-	!memcmp(e1, e2, \(6\|ETH_ALEN\))
+	ether_addr_equal_unaligned(e1, e2)

@@
expression e1;
expression e2;
@@

-	memcmp(e1, e2, \(6\|ETH_ALEN\)) == 0
+	ether_addr_equal_unaligned(e1, e2)

@@
expression e1;
expression e2;
@@

-	memcmp(e1, e2, \(6\|ETH_ALEN\))
+	!ether_addr_equal_unaligned(e1, e2)

@@
expression e1;
expression e2;
@@

-	memcmp(e1, e2, \(6\|ETH_ALEN\)) != 0
+	!ether_addr_equal_unaligned(e1, e2)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/6] slight optimization of addr compare for some modules
  2013-12-16 17:25       ` Joe Perches
@ 2013-12-17  1:58         ` Ding Tianhong
  2013-12-18  8:47           ` Ding Tianhong
  0 siblings, 1 reply; 12+ messages in thread
From: Ding Tianhong @ 2013-12-17  1:58 UTC (permalink / raw)
  To: Joe Perches, Ding Tianhong; +Cc: David S. Miller, Netdev

On 2013/12/17 1:25, Joe Perches wrote:
> Hi again Ding.
> 
> These should still be inspected for appropriate use of
> ether_addr_equal or ether_addr_equal_unaligned, but a
> better cocci input sp-file is:
> 
> $ cat ether_addr_equal_unaligned.cocci 
> @@
> expression e1;
> expression e2;
> @@
> 
> -	!memcmp(e1, e2, \(6\|ETH_ALEN\))
> +	ether_addr_equal_unaligned(e1, e2)
> 
> @@
> expression e1;
> expression e2;
> @@
> 
> -	memcmp(e1, e2, \(6\|ETH_ALEN\)) == 0
> +	ether_addr_equal_unaligned(e1, e2)
> 
> @@
> expression e1;
> expression e2;
> @@
> 
> -	memcmp(e1, e2, \(6\|ETH_ALEN\))
> +	!ether_addr_equal_unaligned(e1, e2)
> 
> @@
> expression e1;
> expression e2;
> @@
> 
> -	memcmp(e1, e2, \(6\|ETH_ALEN\)) != 0
> +	!ether_addr_equal_unaligned(e1, e2)
> 
> 
> 

great, thanks, I 'll try and test.

Regards
ding

> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/6] slight optimization of addr compare for some modules
  2013-12-17  1:58         ` Ding Tianhong
@ 2013-12-18  8:47           ` Ding Tianhong
  2013-12-18  9:17             ` Joe Perches
  0 siblings, 1 reply; 12+ messages in thread
From: Ding Tianhong @ 2013-12-18  8:47 UTC (permalink / raw)
  To: Joe Perches, Ding Tianhong; +Cc: David S. Miller, Netdev

On 2013/12/17 9:58, Ding Tianhong wrote:
> On 2013/12/17 1:25, Joe Perches wrote:
>> Hi again Ding.
>>
>> These should still be inspected for appropriate use of
>> ether_addr_equal or ether_addr_equal_unaligned, but a
>> better cocci input sp-file is:
>>
>> $ cat ether_addr_equal_unaligned.cocci 
>> @@
>> expression e1;
>> expression e2;
>> @@
>>
>> -	!memcmp(e1, e2, \(6\|ETH_ALEN\))
>> +	ether_addr_equal_unaligned(e1, e2)
>>
>> @@
>> expression e1;
>> expression e2;
>> @@
>>
>> -	memcmp(e1, e2, \(6\|ETH_ALEN\)) == 0
>> +	ether_addr_equal_unaligned(e1, e2)
>>
>> @@
>> expression e1;
>> expression e2;
>> @@
>>
>> -	memcmp(e1, e2, \(6\|ETH_ALEN\))
>> +	!ether_addr_equal_unaligned(e1, e2)
>>
>> @@
>> expression e1;
>> expression e2;
>> @@
>>
>> -	memcmp(e1, e2, \(6\|ETH_ALEN\)) != 0
>> +	!ether_addr_equal_unaligned(e1, e2)
>>
>>
>>
> 
> great, thanks, I 'll try and test.
> 
> Regards
> ding
> 

Hi Joe:

There are too many places need to be changed, should I make it in one patch or several pathset,
pls give me some advise. thanks

Regards
Ding

>>
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/6] slight optimization of addr compare for some modules
  2013-12-18  8:47           ` Ding Tianhong
@ 2013-12-18  9:17             ` Joe Perches
  2013-12-18  9:35               ` Ding Tianhong
  2013-12-18 10:06               ` Ding Tianhong
  0 siblings, 2 replies; 12+ messages in thread
From: Joe Perches @ 2013-12-18  9:17 UTC (permalink / raw)
  To: Ding Tianhong; +Cc: Ding Tianhong, David S. Miller, Netdev

On Wed, 2013-12-18 at 16:47 +0800, Ding Tianhong wrote:
> On 2013/12/17 9:58, Ding Tianhong wrote:
> > On 2013/12/17 1:25, Joe Perches wrote:
> >> These should still be inspected for appropriate use of
> >> ether_addr_equal or ether_addr_equal_unaligned, but a
> >> better cocci input sp-file is:
> >>
> >> $ cat ether_addr_equal_unaligned.cocci 
> >> @@
> >> expression e1;
> >> expression e2;
> >> @@
[]
> There are too many places need to be changed, should I make it in one patch or several pathset,
> pls give me some advise. thanks

Separate per-maintainer patches are generally good.
It can take several attempts to get these applied
in all the various trees.

So maybe 1 patch for each of most of these.  Maybe
some of these like drivers/media, drivers/mtd and
drivers/staging could probably be single patches.

$ git grep --name-only -E "\bmemcmp\s*\([^,]+,[^,]+,\s*(ETH_ALEN|6)\s*\)"| \
  sed -r 's@\w+.[ch]$@@' | sort | uniq
arch/x86/kernel/apic/
block/partitions/
drivers/atm/
drivers/infiniband/hw/mlx4/
drivers/infiniband/ulp/ipoib/
drivers/isdn/i4l/
drivers/media/dvb-core/
drivers/media/usb/gspca/
drivers/mtd/
drivers/mtd/nand/
drivers/net/bonding/
drivers/net/ethernet/amd/
drivers/net/ethernet/atheros/atlx/
drivers/net/ethernet/broadcom/bnx2x/
drivers/net/ethernet/chelsio/cxgb3/
drivers/net/ethernet/cisco/enic/
drivers/net/ethernet/emulex/benet/
drivers/net/ethernet/freescale/
drivers/net/ethernet/intel/igbvf/
drivers/net/ethernet/mellanox/mlx4/
drivers/net/ethernet/micrel/
drivers/net/ethernet/neterion/vxge/vxge-
drivers/net/ethernet/qlogic/netxen/
drivers/net/ethernet/qlogic/qlcnic/
drivers/net/ethernet/renesas/
drivers/net/ethernet/seeq/
drivers/net/ethernet/sun/
drivers/net/ethernet/ti/
drivers/net/fddi/skfp/
drivers/net/hamradio/
drivers/net/plip/
drivers/net/ppp/
drivers/net/wireless/
drivers/net/wireless/ath/ath10k/
drivers/net/wireless/ath/ath6kl/
drivers/net/wireless/ath/wcn36xx/
drivers/net/wireless/ath/wil6210/
drivers/net/wireless/brcm80211/brcmfmac/
drivers/net/wireless/cw1200/
drivers/net/wireless/hostap/
drivers/net/wireless/ipw2x00/
drivers/net/wireless/libertas/
drivers/net/wireless/mwifiex/
drivers/net/wireless/orinoco/
drivers/net/wireless/prism54/
drivers/net/wireless/rtlwifi/
drivers/net/wireless/ti/wl1251/
drivers/net/wireless/zd1211rw/
drivers/s390/net/
drivers/scsi/sym53c8xx_2/
drivers/staging/ozwpan/
drivers/staging/rtl8187se/
drivers/staging/rtl8187se/ieee80211/
drivers/staging/rtl8188eu/core/
drivers/staging/rtl8188eu/os_dep/
drivers/staging/rtl8192e/
drivers/staging/rtl8192u/ieee80211/
drivers/staging/rtl8712/
drivers/staging/vt6655/
drivers/staging/vt6656/
drivers/staging/wlan-ng/
drivers/target/iscsi/
drivers/usb/atm/
drivers/usb/storage/
drivers/video/matrox/
fs/sysv/
include/linux/
init/
net/batman-adv/
net/batman-adv/translation-
net/bluetooth/bnep/
net/bluetooth/hidp/
net/bridge/
net/caif/
net/mac80211/
net/sunrpc/auth_gss/
scripts/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/6] slight optimization of addr compare for some modules
  2013-12-18  9:17             ` Joe Perches
@ 2013-12-18  9:35               ` Ding Tianhong
  2013-12-18 10:06               ` Ding Tianhong
  1 sibling, 0 replies; 12+ messages in thread
From: Ding Tianhong @ 2013-12-18  9:35 UTC (permalink / raw)
  To: Joe Perches; +Cc: Ding Tianhong, David S. Miller, Netdev

On 2013/12/18 17:17, Joe Perches wrote:
> On Wed, 2013-12-18 at 16:47 +0800, Ding Tianhong wrote:
>> On 2013/12/17 9:58, Ding Tianhong wrote:
>>> On 2013/12/17 1:25, Joe Perches wrote:
>>>> These should still be inspected for appropriate use of
>>>> ether_addr_equal or ether_addr_equal_unaligned, but a
>>>> better cocci input sp-file is:
>>>>
>>>> $ cat ether_addr_equal_unaligned.cocci 
>>>> @@
>>>> expression e1;
>>>> expression e2;
>>>> @@
> []
>> There are too many places need to be changed, should I make it in one patch or several pathset,
>> pls give me some advise. thanks
> 
> Separate per-maintainer patches are generally good.
> It can take several attempts to get these applied
> in all the various trees.
> 
> So maybe 1 patch for each of most of these.  Maybe
> some of these like drivers/media, drivers/mtd and
> drivers/staging could probably be single patches.
> 
> $ git grep --name-only -E "\bmemcmp\s*\([^,]+,[^,]+,\s*(ETH_ALEN|6)\s*\)"| \
>   sed -r 's@\w+.[ch]$@@' | sort | uniq
> arch/x86/kernel/apic/
> block/partitions/
> drivers/atm/
> drivers/infiniband/hw/mlx4/
> drivers/infiniband/ulp/ipoib/
> drivers/isdn/i4l/
> drivers/media/dvb-core/
> drivers/media/usb/gspca/
> drivers/mtd/
> drivers/mtd/nand/
> drivers/net/bonding/
> drivers/net/ethernet/amd/
> drivers/net/ethernet/atheros/atlx/
> drivers/net/ethernet/broadcom/bnx2x/
> drivers/net/ethernet/chelsio/cxgb3/
> drivers/net/ethernet/cisco/enic/
> drivers/net/ethernet/emulex/benet/
> drivers/net/ethernet/freescale/
> drivers/net/ethernet/intel/igbvf/
> drivers/net/ethernet/mellanox/mlx4/
> drivers/net/ethernet/micrel/
> drivers/net/ethernet/neterion/vxge/vxge-
> drivers/net/ethernet/qlogic/netxen/
> drivers/net/ethernet/qlogic/qlcnic/
> drivers/net/ethernet/renesas/
> drivers/net/ethernet/seeq/
> drivers/net/ethernet/sun/
> drivers/net/ethernet/ti/
> drivers/net/fddi/skfp/
> drivers/net/hamradio/
> drivers/net/plip/
> drivers/net/ppp/
> drivers/net/wireless/
> drivers/net/wireless/ath/ath10k/
> drivers/net/wireless/ath/ath6kl/
> drivers/net/wireless/ath/wcn36xx/
> drivers/net/wireless/ath/wil6210/
> drivers/net/wireless/brcm80211/brcmfmac/
> drivers/net/wireless/cw1200/
> drivers/net/wireless/hostap/
> drivers/net/wireless/ipw2x00/
> drivers/net/wireless/libertas/
> drivers/net/wireless/mwifiex/
> drivers/net/wireless/orinoco/
> drivers/net/wireless/prism54/
> drivers/net/wireless/rtlwifi/
> drivers/net/wireless/ti/wl1251/
> drivers/net/wireless/zd1211rw/
> drivers/s390/net/
> drivers/scsi/sym53c8xx_2/
> drivers/staging/ozwpan/
> drivers/staging/rtl8187se/
> drivers/staging/rtl8187se/ieee80211/
> drivers/staging/rtl8188eu/core/
> drivers/staging/rtl8188eu/os_dep/
> drivers/staging/rtl8192e/
> drivers/staging/rtl8192u/ieee80211/
> drivers/staging/rtl8712/
> drivers/staging/vt6655/
> drivers/staging/vt6656/
> drivers/staging/wlan-ng/
> drivers/target/iscsi/
> drivers/usb/atm/
> drivers/usb/storage/
> drivers/video/matrox/
> fs/sysv/
> include/linux/
> init/
> net/batman-adv/
> net/batman-adv/translation-
> net/bluetooth/bnep/
> net/bluetooth/hidp/
> net/bridge/
> net/caif/
> net/mac80211/
> net/sunrpc/auth_gss/
> scripts/
> 
> 

great, thanks, my team could make it together.

Regards
Ding

> .
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/6] slight optimization of addr compare for some modules
  2013-12-18  9:17             ` Joe Perches
  2013-12-18  9:35               ` Ding Tianhong
@ 2013-12-18 10:06               ` Ding Tianhong
  2013-12-18 16:51                 ` Joe Perches
  1 sibling, 1 reply; 12+ messages in thread
From: Ding Tianhong @ 2013-12-18 10:06 UTC (permalink / raw)
  To: Joe Perches; +Cc: Ding Tianhong, David S. Miller, Netdev

On 2013/12/18 17:17, Joe Perches wrote:
> On Wed, 2013-12-18 at 16:47 +0800, Ding Tianhong wrote:
>> On 2013/12/17 9:58, Ding Tianhong wrote:
>>> On 2013/12/17 1:25, Joe Perches wrote:
>>>> These should still be inspected for appropriate use of
>>>> ether_addr_equal or ether_addr_equal_unaligned, but a
>>>> better cocci input sp-file is:
>>>>
>>>> $ cat ether_addr_equal_unaligned.cocci 
>>>> @@
>>>> expression e1;
>>>> expression e2;
>>>> @@
> []
>> There are too many places need to be changed, should I make it in one patch or several pathset,
>> pls give me some advise. thanks
> 
> Separate per-maintainer patches are generally good.
> It can take several attempts to get these applied
> in all the various trees.
> 
> So maybe 1 patch for each of most of these.  Maybe
> some of these like drivers/media, drivers/mtd and
> drivers/staging could probably be single patches.
> 

Hi Joe:

I found there is a bug in spatch, it could not deal with 
-       memcmp(e1, e2, \(6\|ETH_ALEN\)) != 0
+       !ether_addr_equal_unaligned(e1, e2)



diff -u -p a/caif/cfrfml.c b/caif/cfrfml.c
--- a/caif/cfrfml.c
+++ b/caif/cfrfml.c
@@ -79,7 +79,7 @@ static struct cfpkt *rfm_append(struct c
                return NULL;

        /* Verify correct header */
-       if (memcmp(seghead, rfml->seghead, 6) != 0)
+       if (!ether_addr_equal_unaligned(seghead, rfml->seghead) != 0)
                return NULL;

        tmppkt = cfpkt_append(rfml->incomplete_frm, pkt,


Regards
Ding 

> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/6] slight optimization of addr compare for some modules
  2013-12-18 10:06               ` Ding Tianhong
@ 2013-12-18 16:51                 ` Joe Perches
  2013-12-19  1:24                   ` Ding Tianhong
  0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2013-12-18 16:51 UTC (permalink / raw)
  To: Ding Tianhong; +Cc: Ding Tianhong, David S. Miller, Netdev

On Wed, 2013-12-18 at 18:06 +0800, Ding Tianhong wrote:
> On 2013/12/18 17:17, Joe Perches wrote:
> > On Wed, 2013-12-18 at 16:47 +0800, Ding Tianhong wrote:
> >> On 2013/12/17 9:58, Ding Tianhong wrote:
> >>> On 2013/12/17 1:25, Joe Perches wrote:
> >>>> These should still be inspected for appropriate use of
> >>>> ether_addr_equal or ether_addr_equal_unaligned, but a
> >>>> better cocci input sp-file is:
> >>>>
> >>>> $ cat ether_addr_equal_unaligned.cocci 
> >>>> @@
> >>>> expression e1;
> >>>> expression e2;
> >>>> @@
> > []
> >> There are too many places need to be changed, should I make it in one patch or several pathset,
> >> pls give me some advise. thanks
> > 
> > Separate per-maintainer patches are generally good.
> > It can take several attempts to get these applied
> > in all the various trees.
> > 
> > So maybe 1 patch for each of most of these.  Maybe
> > some of these like drivers/media, drivers/mtd and
> > drivers/staging could probably be single patches.
> > 
> 
> Hi Joe:
> 
> I found there is a bug in spatch, it could not deal with 
> -       memcmp(e1, e2, \(6\|ETH_ALEN\)) != 0
> +       !ether_addr_equal_unaligned(e1, e2)

Not an spatch bug but a defect in the ordering of
transforms in the ether_addr_equal_unaligned.cocci file

This should be better:

$ cat ether_addr_equal_unaligned.cocci
@@
expression e1;
expression e2;
@@

-	memcmp(e1, e2, \(6\|ETH_ALEN\)) == 0
+	ether_addr_equal_unaligned(e1, e2)

@@
expression e1;
expression e2;
@@

-	memcmp(e1, e2, \(6\|ETH_ALEN\)) != 0
+	!ether_addr_equal_unaligned(e1, e2)

@@
expression e1;
expression e2;
@@

-	!memcmp(e1, e2, \(6\|ETH_ALEN\))
+	ether_addr_equal_unaligned(e1, e2)

@@
expression e1;
expression e2;
@@

-	memcmp(e1, e2, \(6\|ETH_ALEN\))
+	!ether_addr_equal_unaligned(e1, e2)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/6] slight optimization of addr compare for some modules
  2013-12-18 16:51                 ` Joe Perches
@ 2013-12-19  1:24                   ` Ding Tianhong
  0 siblings, 0 replies; 12+ messages in thread
From: Ding Tianhong @ 2013-12-19  1:24 UTC (permalink / raw)
  To: Joe Perches; +Cc: Ding Tianhong, David S. Miller, Netdev

On 2013/12/19 0:51, Joe Perches wrote:
> On Wed, 2013-12-18 at 18:06 +0800, Ding Tianhong wrote:
>> On 2013/12/18 17:17, Joe Perches wrote:
>>> On Wed, 2013-12-18 at 16:47 +0800, Ding Tianhong wrote:
>>>> On 2013/12/17 9:58, Ding Tianhong wrote:
>>>>> On 2013/12/17 1:25, Joe Perches wrote:
>>>>>> These should still be inspected for appropriate use of
>>>>>> ether_addr_equal or ether_addr_equal_unaligned, but a
>>>>>> better cocci input sp-file is:
>>>>>>
>>>>>> $ cat ether_addr_equal_unaligned.cocci 
>>>>>> @@
>>>>>> expression e1;
>>>>>> expression e2;
>>>>>> @@
>>> []
>>>> There are too many places need to be changed, should I make it in one patch or several pathset,
>>>> pls give me some advise. thanks
>>>
>>> Separate per-maintainer patches are generally good.
>>> It can take several attempts to get these applied
>>> in all the various trees.
>>>
>>> So maybe 1 patch for each of most of these.  Maybe
>>> some of these like drivers/media, drivers/mtd and
>>> drivers/staging could probably be single patches.
>>>
>>
>> Hi Joe:
>>
>> I found there is a bug in spatch, it could not deal with 
>> -       memcmp(e1, e2, \(6\|ETH_ALEN\)) != 0
>> +       !ether_addr_equal_unaligned(e1, e2)
> 
> Not an spatch bug but a defect in the ordering of
> transforms in the ether_addr_equal_unaligned.cocci file
> 
> This should be better:
> 
> $ cat ether_addr_equal_unaligned.cocci
> @@
> expression e1;
> expression e2;
> @@
> 
> -	memcmp(e1, e2, \(6\|ETH_ALEN\)) == 0
> +	ether_addr_equal_unaligned(e1, e2)
> 
> @@
> expression e1;
> expression e2;
> @@
> 
> -	memcmp(e1, e2, \(6\|ETH_ALEN\)) != 0
> +	!ether_addr_equal_unaligned(e1, e2)
> 
> @@
> expression e1;
> expression e2;
> @@
> 
> -	!memcmp(e1, e2, \(6\|ETH_ALEN\))
> +	ether_addr_equal_unaligned(e1, e2)
> 
> @@
> expression e1;
> expression e2;
> @@
> 
> -	memcmp(e1, e2, \(6\|ETH_ALEN\))
> +	!ether_addr_equal_unaligned(e1, e2)
> 
> 
> 

Yes, it could works well, thanks a lot.

Regards
Ding

> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-12-19  1:25 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-16  8:24 [PATCH net-next v2 0/6] slight optimization of addr compare for some modules Ding Tianhong
2013-12-16 14:45 ` Joe Perches
2013-12-16 14:53   ` Ding Tianhong
2013-12-16 15:16     ` Joe Perches
2013-12-16 17:25       ` Joe Perches
2013-12-17  1:58         ` Ding Tianhong
2013-12-18  8:47           ` Ding Tianhong
2013-12-18  9:17             ` Joe Perches
2013-12-18  9:35               ` Ding Tianhong
2013-12-18 10:06               ` Ding Tianhong
2013-12-18 16:51                 ` Joe Perches
2013-12-19  1:24                   ` Ding Tianhong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).