* [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).