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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Thu, 2018-01-11 at 14:57 +0300, Yury Norov wrote:
> 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:

> > > > 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,

I don't know. There are like you said 51 user of the bitmap_fill().

If we lucky that developers are not so-o-o dumb to use bitmap_fill() as
a replacement of bitmap_set(..., 0, ...), nothing will need to be fixed.

>  and there are at least 2 cases like that?

Where this come from?

> 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.

It would be found much earlier in the core code, otherwise it's business
as usual.

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

So, instead of beating the air you can help to check the places, right?

> > >  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.

It's not mechanical, that is the point. (Incorrect) usage of
bitmap_fill() is a bug. Fix that helps to reveal it earlier is a good
one.

> > > 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@redhat.com>]
> 
> You can always ask tools/* maintainers what is better for them.

Yeah, notification is a good thing.

>  For me,
> people simply forget about tools/* and that's why maintainers have to
> sync sources periodically.

Might be, my at least one patch (and few pings) to tools code at the end
is left neither commented not applied for years, so, I gave up on them,
sorry @acme et al. OTOH fixes for Makefiles are usually go in quickly.

>  Anyway, if you think that your change is good
> enough for Linux kernel, why don't you think so for tools?

I didn't tell that.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy