mbox series

[v2,0/2,net] bitfield.h cleanups

Message ID 20200708230402.1644819-1-ndesaulniers@google.com
Headers show
Series bitfield.h cleanups | expand

Message

Nick Desaulniers July 8, 2020, 11:04 p.m. UTC
Two patches, one that removes a BUILD_BUG_ON for a case that is not a
compile time bug (exposed by compiler optimization).

The second is a general cleanup in the area.

I decided to leave the BUILD_BUG_ON case first, as I hope it will
simplify being able to backport it to stable, and because I don't think
there's any type promotion+conversion bugs there.

Though it would be nice to use consistent types widths and signedness,
equality against literal zero is not an issue.

Jakub Kicinski (1):
  bitfield.h: don't compile-time validate _val in FIELD_FIT

Nick Desaulniers (1):
  bitfield.h: split up __BF_FIELD_CHECK macro

 .../netronome/nfp/nfpcore/nfp_nsp_eth.c       | 11 ++++----
 include/linux/bitfield.h                      | 26 +++++++++++++------
 2 files changed, 24 insertions(+), 13 deletions(-)

-- 
2.27.0.383.g050319c2ae-goog

Comments

Nick Desaulniers July 14, 2020, 6:23 p.m. UTC | #1
On Wed, Jul 8, 2020 at 4:04 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
>

> Two patches, one that removes a BUILD_BUG_ON for a case that is not a

> compile time bug (exposed by compiler optimization).

>

> The second is a general cleanup in the area.

>

> I decided to leave the BUILD_BUG_ON case first, as I hope it will

> simplify being able to backport it to stable, and because I don't think

> there's any type promotion+conversion bugs there.

>

> Though it would be nice to use consistent types widths and signedness,

> equality against literal zero is not an issue.

>

> Jakub Kicinski (1):

>   bitfield.h: don't compile-time validate _val in FIELD_FIT

>

> Nick Desaulniers (1):

>   bitfield.h: split up __BF_FIELD_CHECK macro

>

>  .../netronome/nfp/nfpcore/nfp_nsp_eth.c       | 11 ++++----

>  include/linux/bitfield.h                      | 26 +++++++++++++------

>  2 files changed, 24 insertions(+), 13 deletions(-)

>

> --

> 2.27.0.383.g050319c2ae-goog

>


Hey David, when you have a chance, could you please consider picking
up this series?
-- 
Thanks,
~Nick Desaulniers
Nick Desaulniers July 20, 2020, 7:48 p.m. UTC | #2
On Tue, Jul 14, 2020 at 11:23 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>

> On Wed, Jul 8, 2020 at 4:04 PM Nick Desaulniers <ndesaulniers@google.com> wrote:

> >

> > Two patches, one that removes a BUILD_BUG_ON for a case that is not a

> > compile time bug (exposed by compiler optimization).

> >

> > The second is a general cleanup in the area.

> >

> > I decided to leave the BUILD_BUG_ON case first, as I hope it will

> > simplify being able to backport it to stable, and because I don't think

> > there's any type promotion+conversion bugs there.

> >

> > Though it would be nice to use consistent types widths and signedness,

> > equality against literal zero is not an issue.

> >

> > Jakub Kicinski (1):

> >   bitfield.h: don't compile-time validate _val in FIELD_FIT

> >

> > Nick Desaulniers (1):

> >   bitfield.h: split up __BF_FIELD_CHECK macro

> >

> >  .../netronome/nfp/nfpcore/nfp_nsp_eth.c       | 11 ++++----

> >  include/linux/bitfield.h                      | 26 +++++++++++++------

> >  2 files changed, 24 insertions(+), 13 deletions(-)

> >

> > --

> > 2.27.0.383.g050319c2ae-goog

> >

>

> Hey David, when you have a chance, could you please consider picking

> up this series?

> --


Hi David, bumping for review.
-- 
Thanks,
~Nick Desaulniers
David Miller July 20, 2020, 11:34 p.m. UTC | #3
From: Nick Desaulniers <ndesaulniers@google.com>

Date: Mon, 20 Jul 2020 12:48:38 -0700

> Hi David, bumping for review.


If it's not active in my networking patchwork you can't just bump your original
submission like this.

You have to submit your series entirely again.

And if it is in patchwork, such "bumping" is %100 unnecessary.  It's
in my queue, it is not lost, and I will process it when I have the
time.

Thank you.
Nick Desaulniers July 30, 2020, 10:37 p.m. UTC | #4
On Mon, Jul 20, 2020 at 4:35 PM David Miller <davem@davemloft.net> wrote:
>

> From: Nick Desaulniers <ndesaulniers@google.com>

> Date: Mon, 20 Jul 2020 12:48:38 -0700

>

> > Hi David, bumping for review.

>

> If it's not active in my networking patchwork you can't just bump your original

> submission like this.

>

> You have to submit your series entirely again.

>

> And if it is in patchwork, such "bumping" is %100 unnecessary.  It's

> in my queue, it is not lost, and I will process it when I have the

> time.

>

> Thank you.


Hi David,
Sorry, I'm not familiar with your workflow.  By patchwork, are you
referring to patchwork.ozlabs.org?  If so, I guess filtering on
Delegate=davem
(https://patchwork.ozlabs.org/project/netdev/list/?series=&submitter=&state=&q=&archive=&delegate=34)
is your queue?  I don't see my patches in the above query, but I do
see my series here:
https://patchwork.ozlabs.org/project/netdev/list/?series=188486&state=*
but they're marked "Not Applicable".  What does that mean?
-- 
Thanks,
~Nick Desaulniers
Nick Desaulniers Aug. 5, 2020, 5:44 p.m. UTC | #5
On Thu, Jul 30, 2020 at 3:37 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>

> On Mon, Jul 20, 2020 at 4:35 PM David Miller <davem@davemloft.net> wrote:

> >

> > From: Nick Desaulniers <ndesaulniers@google.com>

> > Date: Mon, 20 Jul 2020 12:48:38 -0700

> >

> > > Hi David, bumping for review.

> >

> > If it's not active in my networking patchwork you can't just bump your original

> > submission like this.

> >

> > You have to submit your series entirely again.

> >

> > And if it is in patchwork, such "bumping" is %100 unnecessary.  It's

> > in my queue, it is not lost, and I will process it when I have the

> > time.

> >

> > Thank you.

>

> Hi David,

> Sorry, I'm not familiar with your workflow.  By patchwork, are you

> referring to patchwork.ozlabs.org?  If so, I guess filtering on

> Delegate=davem

> (https://patchwork.ozlabs.org/project/netdev/list/?series=&submitter=&state=&q=&archive=&delegate=34)

> is your queue?  I don't see my patches in the above query, but I do

> see my series here:

> https://patchwork.ozlabs.org/project/netdev/list/?series=188486&state=*

> but they're marked "Not Applicable".  What does that mean?


Hi David,
I read through https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html#q-how-often-do-changes-from-these-trees-make-it-to-the-mainline-linus-tree
and noticed http://vger.kernel.org/~davem/net-next.html.  Since the
merge window just opened, it sounds like I'll need to wait 2 weeks for
it to close before resending? Is that correct? Based on:

> IMPORTANT: Do not send new net-next content to netdev during the period during which net-next tree is closed.


Then based on the next section in the doc, it sounds like I was
missing which tree to put the patch in, in the subject? I believe
these patches should target net-next (not net) since they're not
addressing regressions from the most recent cycle.

Do I have all that right?
-- 
Thanks,
~Nick Desaulniers
Jakub Kicinski Aug. 6, 2020, 6:13 p.m. UTC | #6
On Wed, 5 Aug 2020 10:44:30 -0700 Nick Desaulniers wrote:
> Hi David,

> I read through https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html#q-how-often-do-changes-from-these-trees-make-it-to-the-mainline-linus-tree

> and noticed http://vger.kernel.org/~davem/net-next.html.  Since the

> merge window just opened, it sounds like I'll need to wait 2 weeks for

> it to close before resending? Is that correct? Based on:

> 

> > IMPORTANT: Do not send new net-next content to netdev during the period during which net-next tree is closed.  

> 

> Then based on the next section in the doc, it sounds like I was

> missing which tree to put the patch in, in the subject? I believe

> these patches should target net-next (not net) since they're not

> addressing regressions from the most recent cycle.

> 

> Do I have all that right?


Nick, please repost the first patch only (to:dave, cc:netdev,...,
subject "[PATCH net resend] ...") and I'm pretty sure Dave will
re-consider it, it's a build fix after all. In any case reviewing 
a patch with a short explanation under the commit message on why it's
resend is easier [1] for a maintainer to process than digging through
conversations.

[1] may be related to the use of patchwork on netdev
Jakub Kicinski Aug. 6, 2020, 6:15 p.m. UTC | #7
On Thu, 6 Aug 2020 11:13:58 -0700 Jakub Kicinski wrote:
> please repost the first patch only


To be clear the second patch may need to wait for net-next since it's
refactoring. That's why I suggest only posting patch 1 now, i.e. the
fix.