* [PATCH] staging: media: Replace a bit shift by a use of BIT.
@ 2017-03-22 4:20 Arushi Singhal
2017-03-22 10:23 ` [lustre-devel] " Julia Lawall
2017-03-22 12:11 ` Dilger, Andreas
0 siblings, 2 replies; 5+ messages in thread
From: Arushi Singhal @ 2017-03-22 4:20 UTC (permalink / raw)
To: oleg.drokin
Cc: Andreas Dilger, James Simmons, Greg Kroah-Hartman, lustre-devel,
devel, linux-kernel, outreachy-kernel
This patch replaces bit shifting on 1 with the BIT(x) macro.
This was done with coccinelle:
@@
constant c;
@@
-1 << c
+BIT(c)
Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
---
drivers/staging/octeon/ethernet-tx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/octeon/ethernet-tx.c b/drivers/staging/octeon/ethernet-tx.c
index ff4119e8de42..f00186ac4e27 100644
--- a/drivers/staging/octeon/ethernet-tx.c
+++ b/drivers/staging/octeon/ethernet-tx.c
@@ -381,7 +381,7 @@ int cvm_oct_xmit(struct sk_buff *skb, struct net_device *dev)
(ip_hdr(skb)->version == 4) &&
(ip_hdr(skb)->ihl == 5) &&
((ip_hdr(skb)->frag_off == 0) ||
- (ip_hdr(skb)->frag_off == htons(1 << 14))) &&
+ (ip_hdr(skb)->frag_off == htons(BIT(14)))) &&
((ip_hdr(skb)->protocol == IPPROTO_TCP) ||
(ip_hdr(skb)->protocol == IPPROTO_UDP))) {
/* Use hardware checksum calc */
@@ -613,7 +613,7 @@ int cvm_oct_xmit_pow(struct sk_buff *skb, struct net_device *dev)
#endif
work->word2.s.is_frag = !((ip_hdr(skb)->frag_off == 0) ||
(ip_hdr(skb)->frag_off ==
- 1 << 14));
+ BIT(14)));
#if 0
/* Assume Linux is sending a good packet */
work->word2.s.IP_exc = 0;
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: media: Replace a bit shift by a use of BIT.
2017-03-22 4:20 [PATCH] staging: media: Replace a bit shift by a use of BIT Arushi Singhal
@ 2017-03-22 10:23 ` Julia Lawall
2017-03-22 12:11 ` Dilger, Andreas
1 sibling, 0 replies; 5+ messages in thread
From: Julia Lawall @ 2017-03-22 10:23 UTC (permalink / raw)
To: Arushi Singhal
Cc: oleg.drokin, Andreas Dilger, James Simmons, Greg Kroah-Hartman,
lustre-devel, devel, linux-kernel, outreachy-kernel
On Wed, 22 Mar 2017, Arushi Singhal wrote:
> This patch replaces bit shifting on 1 with the BIT(x) macro.
> This was done with coccinelle:
> @@
> constant c;
> @@
>
> -1 << c
> +BIT(c)
>
> Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
> ---
> drivers/staging/octeon/ethernet-tx.c | 4 ++--
What is the connection to media? You also seem to have picked up the
Lustre maintainers, who are not relevant.
julia
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/octeon/ethernet-tx.c b/drivers/staging/octeon/ethernet-tx.c
> index ff4119e8de42..f00186ac4e27 100644
> --- a/drivers/staging/octeon/ethernet-tx.c
> +++ b/drivers/staging/octeon/ethernet-tx.c
> @@ -381,7 +381,7 @@ int cvm_oct_xmit(struct sk_buff *skb, struct net_device *dev)
> (ip_hdr(skb)->version == 4) &&
> (ip_hdr(skb)->ihl == 5) &&
> ((ip_hdr(skb)->frag_off == 0) ||
> - (ip_hdr(skb)->frag_off == htons(1 << 14))) &&
> + (ip_hdr(skb)->frag_off == htons(BIT(14)))) &&
> ((ip_hdr(skb)->protocol == IPPROTO_TCP) ||
> (ip_hdr(skb)->protocol == IPPROTO_UDP))) {
> /* Use hardware checksum calc */
> @@ -613,7 +613,7 @@ int cvm_oct_xmit_pow(struct sk_buff *skb, struct net_device *dev)
> #endif
> work->word2.s.is_frag = !((ip_hdr(skb)->frag_off == 0) ||
> (ip_hdr(skb)->frag_off ==
> - 1 << 14));
> + BIT(14)));
> #if 0
> /* Assume Linux is sending a good packet */
> work->word2.s.IP_exc = 0;
> --
> 2.11.0
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170322042028.GA22848%40arushi-HP-Pavilion-Notebook.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [lustre-devel] [Outreachy kernel] [PATCH] staging: media: Replace a bit shift by a use of BIT.
@ 2017-03-22 10:23 ` Julia Lawall
0 siblings, 0 replies; 5+ messages in thread
From: Julia Lawall @ 2017-03-22 10:23 UTC (permalink / raw)
To: Arushi Singhal
Cc: oleg.drokin, Andreas Dilger, James Simmons, Greg Kroah-Hartman,
lustre-devel, devel, linux-kernel, outreachy-kernel
On Wed, 22 Mar 2017, Arushi Singhal wrote:
> This patch replaces bit shifting on 1 with the BIT(x) macro.
> This was done with coccinelle:
> @@
> constant c;
> @@
>
> -1 << c
> +BIT(c)
>
> Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
> ---
> drivers/staging/octeon/ethernet-tx.c | 4 ++--
What is the connection to media? You also seem to have picked up the
Lustre maintainers, who are not relevant.
julia
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/octeon/ethernet-tx.c b/drivers/staging/octeon/ethernet-tx.c
> index ff4119e8de42..f00186ac4e27 100644
> --- a/drivers/staging/octeon/ethernet-tx.c
> +++ b/drivers/staging/octeon/ethernet-tx.c
> @@ -381,7 +381,7 @@ int cvm_oct_xmit(struct sk_buff *skb, struct net_device *dev)
> (ip_hdr(skb)->version == 4) &&
> (ip_hdr(skb)->ihl == 5) &&
> ((ip_hdr(skb)->frag_off == 0) ||
> - (ip_hdr(skb)->frag_off == htons(1 << 14))) &&
> + (ip_hdr(skb)->frag_off == htons(BIT(14)))) &&
> ((ip_hdr(skb)->protocol == IPPROTO_TCP) ||
> (ip_hdr(skb)->protocol == IPPROTO_UDP))) {
> /* Use hardware checksum calc */
> @@ -613,7 +613,7 @@ int cvm_oct_xmit_pow(struct sk_buff *skb, struct net_device *dev)
> #endif
> work->word2.s.is_frag = !((ip_hdr(skb)->frag_off == 0) ||
> (ip_hdr(skb)->frag_off ==
> - 1 << 14));
> + BIT(14)));
> #if 0
> /* Assume Linux is sending a good packet */
> work->word2.s.IP_exc = 0;
> --
> 2.11.0
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe at googlegroups.com.
> To post to this group, send email to outreachy-kernel at googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170322042028.GA22848%40arushi-HP-Pavilion-Notebook.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: media: Replace a bit shift by a use of BIT.
2017-03-22 4:20 [PATCH] staging: media: Replace a bit shift by a use of BIT Arushi Singhal
2017-03-22 10:23 ` [lustre-devel] " Julia Lawall
@ 2017-03-22 12:11 ` Dilger, Andreas
2017-03-22 13:53 ` Arushi Singhal
1 sibling, 1 reply; 5+ messages in thread
From: Dilger, Andreas @ 2017-03-22 12:11 UTC (permalink / raw)
To: Arushi Singhal
Cc: Drokin, Oleg, James Simmons, Greg Kroah-Hartman, devel, LKML,
outreachy-kernel, Luuk Paulussen, Hamish Martin
On Mar 22, 2017, at 00:20, Arushi Singhal <arushisinghal19971997@gmail.com> wrote:
>
> This patch replaces bit shifting on 1 with the BIT(x) macro.
> This was done with coccinelle:
> @@
> constant c;
> @@
>
> -1 << c
> +BIT(c)
Hi Arushi,
thanks for taking time to contribute to the kernel. There are a few problems with
this patch.
Firstly, I'm not sure why you sent this patch to lustre-devel, Oleg, James, and me,
since it has nothing to do with Lustre. Conversely, the actual maintainer(s) of this
code would normally be better suited to review the patch. This isn't totally clear
in this case since get_maintainer.pl lists another person submitting minor cleanups,
but "git log" does return some more likely candidates:
- David Daney <ddaney@caviumnetworks.com> (original author)
- Luuk Paulussen <luuk.paulussen@alliedtelesis.co.nz> (recent non-trivial patcher)
- Kyeong Yoo <kyeong.yoo@alliedtelesis.co.nz> (recent non-trivial reviewer)
- Richard Laing <richard.laing@alliedtelesis.co.nz> " "
- Chris Packham <chris.packham@alliedtelesis.co.nz>
- Tim Beale <tim.beale@alliedtelesis.co.nz>
- Hamish Martin <hamish.martin@alliedtelesis.co.nz>
Secondly, as mentioned with the other patch you submitted, you need to stop and look
at whether your changes actually make sense. In this case, I don't think they do
make sense, and this patch should probably just be abandoned.
> Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
> ---
> drivers/staging/octeon/ethernet-tx.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/octeon/ethernet-tx.c b/drivers/staging/octeon/ethernet-tx.c
> index ff4119e8de42..f00186ac4e27 100644
> --- a/drivers/staging/octeon/ethernet-tx.c
> +++ b/drivers/staging/octeon/ethernet-tx.c
> @@ -381,7 +381,7 @@ int cvm_oct_xmit(struct sk_buff *skb, struct net_device *dev)
> (ip_hdr(skb)->version == 4) &&
> (ip_hdr(skb)->ihl == 5) &&
> ((ip_hdr(skb)->frag_off == 0) ||
> - (ip_hdr(skb)->frag_off == htons(1 << 14))) &&
> + (ip_hdr(skb)->frag_off == htons(BIT(14)))) &&
In this case, it is checking if the fragment offset in bytes is 16384, rather than doing
a bit comparison, so I don't think the use of BIT() is appropriate here.
> ((ip_hdr(skb)->protocol == IPPROTO_TCP) ||
> (ip_hdr(skb)->protocol == IPPROTO_UDP))) {
> /* Use hardware checksum calc */
> @@ -613,7 +613,7 @@ int cvm_oct_xmit_pow(struct sk_buff *skb, struct net_device *dev)
> #endif
> work->word2.s.is_frag = !((ip_hdr(skb)->frag_off == 0) ||
> (ip_hdr(skb)->frag_off ==
> - 1 << 14));
> + BIT(14)));
Same.
Cheers, Andreas
--
Andreas Dilger
Lustre Principal Architect
Intel Corporation
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: media: Replace a bit shift by a use of BIT.
2017-03-22 12:11 ` Dilger, Andreas
@ 2017-03-22 13:53 ` Arushi Singhal
0 siblings, 0 replies; 5+ messages in thread
From: Arushi Singhal @ 2017-03-22 13:53 UTC (permalink / raw)
To: Dilger, Andreas
Cc: Drokin, Oleg, James Simmons, Greg Kroah-Hartman, devel, LKML,
outreachy-kernel, Luuk Paulussen, Hamish Martin
[-- Attachment #1: Type: text/plain, Size: 3250 bytes --]
On Wed, Mar 22, 2017 at 5:41 PM, Dilger, Andreas <andreas.dilger@intel.com>
wrote:
> On Mar 22, 2017, at 00:20, Arushi Singhal <arushisinghal19971997@gmail.com>
> wrote:
> >
> > This patch replaces bit shifting on 1 with the BIT(x) macro.
> > This was done with coccinelle:
> > @@
> > constant c;
> > @@
> >
> > -1 << c
> > +BIT(c)
>
> Hi Arushi,
> thanks for taking time to contribute to the kernel. There are a few
> problems with
> this patch.
>
> Firstly, I'm not sure why you sent this patch to lustre-devel, Oleg,
> James, and me,
> since it has nothing to do with Lustre. Conversely, the actual
> maintainer(s) of this
> code would normally be better suited to review the patch. This isn't
> totally clear
> in this case since get_maintainer.pl lists another person submitting
> minor cleanups,
> but "git log" does return some more likely candidates:
> - David Daney <ddaney@caviumnetworks.com> (original author)
> - Luuk Paulussen <luuk.paulussen@alliedtelesis.co.nz> (recent non-trivial
> patcher)
> - Kyeong Yoo <kyeong.yoo@alliedtelesis.co.nz> (recent non-trivial
> reviewer)
> - Richard Laing <richard.laing@alliedtelesis.co.nz> " "
> - Chris Packham <chris.packham@alliedtelesis.co.nz>
> - Tim Beale <tim.beale@alliedtelesis.co.nz>
> - Hamish Martin <hamish.martin@alliedtelesis.co.nz>
>
>
> Secondly, as mentioned with the other patch you submitted, you need to
> stop and look
> at whether your changes actually make sense. In this case, I don't think
> they do
> make sense, and this patch should probably just be abandoned.
>
>
> > Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
> > ---
> > drivers/staging/octeon/ethernet-tx.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/octeon/ethernet-tx.c
> b/drivers/staging/octeon/ethernet-tx.c
> > index ff4119e8de42..f00186ac4e27 100644
> > --- a/drivers/staging/octeon/ethernet-tx.c
> > +++ b/drivers/staging/octeon/ethernet-tx.c
> > @@ -381,7 +381,7 @@ int cvm_oct_xmit(struct sk_buff *skb, struct
> net_device *dev)
> > (ip_hdr(skb)->version == 4) &&
> > (ip_hdr(skb)->ihl == 5) &&
> > ((ip_hdr(skb)->frag_off == 0) ||
> > - (ip_hdr(skb)->frag_off == htons(1 << 14))) &&
> > + (ip_hdr(skb)->frag_off == htons(BIT(14)))) &&
>
> In this case, it is checking if the fragment offset in bytes is 16384,
> rather than doing
> a bit comparison, so I don't think the use of BIT() is appropriate here.
>
> > ((ip_hdr(skb)->protocol == IPPROTO_TCP) ||
> > (ip_hdr(skb)->protocol == IPPROTO_UDP))) {
> > /* Use hardware checksum calc */
> > @@ -613,7 +613,7 @@ int cvm_oct_xmit_pow(struct sk_buff *skb, struct
> net_device *dev)
> > #endif
> > work->word2.s.is_frag = !((ip_hdr(skb)->frag_off == 0) ||
> > (ip_hdr(skb)->frag_off ==
> > - 1 << 14));
> > + BIT(14)));
>
> Same.
>
Hi Andreas
Your suggestions are really fruitful.
I will keep in mind to do sensible changes.
Thanks
Arushi Singhal
>
> Cheers, Andreas
> --
> Andreas Dilger
> Lustre Principal Architect
> Intel Corporation
>
>
>
>
>
>
>
>
[-- Attachment #2: Type: text/html, Size: 5031 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-22 13:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-22 4:20 [PATCH] staging: media: Replace a bit shift by a use of BIT Arushi Singhal
2017-03-22 10:23 ` [Outreachy kernel] " Julia Lawall
2017-03-22 10:23 ` [lustre-devel] " Julia Lawall
2017-03-22 12:11 ` Dilger, Andreas
2017-03-22 13:53 ` Arushi Singhal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.