Messages in this thread | | | Subject | Re: [PATCH v1 4/4] bitmap: Make bitmap_fill() and bitmap_zero() consistent | From | Andy Shevchenko <> | Date | Thu, 11 Jan 2018 14:46:03 +0200 |
| |
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
| |