Re: [PATCH v1 4/4] bitmap: Make bitmap_fill() and bitmap_zero() consistent

[Yury Norov]

On Wed, Jan 10, 2018 at 03:17:03PM +0200, Andy Shevchenko wrote:
> On Wed, 2018-01-10 at 11:49 +0300, Yury Norov wrote:
> > On Tue, Jan 09, 2018 at 07:24:30PM +0200, Andy Shevchenko wrote:
> > > Behaviour of bitmap_fill() differs from bitmap_zero() in a way
> > > how bits behind bitmap are handed. bitmap_zero() clears entire
> > > bitmap
> > > by unsigned long boundary, while bitmap_fill() mimics bitmap_set().
> > > 
> > > Here we change bitmap_fill() behaviour to be consistent with
> > > bitmap_zero()
> > > and add a note to documentation.
> > > 
> > > The change might reveal some bugs in the code where unused bits
> > > handled
> > > differently and in such cases bitmap_set() has to be used.
> > 
> > There is only 51 users of bitmap_fill() in the kernel, including
> > tests. If you propose this change, I think you'd check them all
> > manually.
> 
> Some of them might require 5 minutes to check while others (especially
> in the areas I don't know much about) 5+ hours. I rely on Rasmus
> assumption that there _were_ bugs, though they assumed to be fixed by
> now.
> 
> In any case I'm ready to take responsibility of possible breakage and
> fully into provide fixes by demand.

Is my understanding correct that you need almost a working day to
decide what function to use - bitmap_set() or bitmap_fill() in some
cases, and there are at least 2 cases like that?

If so, there's quite realistic chance that bug will hit us 6 month
after upstreaming the patch when affected kernel will be delivered
to end users by distro developers.

This is not acceptable scenario. If you have willing to take
responsibility, please do it now and don't wait when things go
broken.

> >  Sorry that.
> 
> I lost your thought here. What did you mean by this?

I only mean that I realize that I ask you to do big amount of boring
mechanical work, and I'm not happy with that.

> > Also, there's tools/include/linux/bitmap.h which has a copy of
> > bitmap_fill(), and should be consistent with main kernel sources.
> 
> tools is independent, although quite related, project to the kernel
> itself. They will decide by themselves how to proceed, I suppose.
> 
> At least what I see in the history of changes in the tools/ they usually
> follow the changes in main library after while.

[CC Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>]

You can always ask tools/* maintainers what is better for them. For me,
people simply forget about tools/* and that's why maintainers have to
sync sources periodically. Anyway, if you think that your change is good
enough for Linux kernel, why don't you think so for tools?

Yury