lkml.org 
[lkml]   [2021]   [Jul]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v1 1/1] bitmap.h: add const modifier to bitmap_next_[set/clear]_region() argument
    On Wed, Jul 28, 2021 at 08:39:15AM +0200, Andrea Merello wrote:
    > Il giorno mar 27 lug 2021 alle ore 17:26 Yury Norov
    > <yury.norov@gmail.com> ha scritto:
    > >
    > > On Tue, Jul 27, 2021 at 02:48:42PM +0300, Andy Shevchenko wrote:
    > > > On Tue, Jul 27, 2021 at 11:44:41AM +0200, Andrea Merello wrote:
    > > > > Those two functions don't modify the bitmap, so their bitmap argument
    > > > > should be const. This patch add this.
    > >
    > > Thanks Andrea.
    > >
    > > Acked-by: Yury Norov <yury.norov@gmail.com>
    > >
    > > > Constification is always a good thing. No objections from me,
    > > > although Yuri is doing something with them in one of his patch series.
    > >
    > > I try to remove those two because in practice they bring more mess
    > > than good. All real use-cases for bitmap_next_{set,clear}_region
    > > relate to iterating the whole bitmap, ie nobody just wants to find a
    > > next region. Untill recently there was only a single user of the API,
    > > so I easily reworked the code to use find_first_bit/find_next bit and
    > > by chance return faster.
    >
    > Ah, that's interesting to know: I'm working on a new driver that used
    > find_next_zero_bit() and find_next_bit() [0]; Andy told me that my
    > algorithm could be rewritten exploiting some bitmap_* helpers, so I
    > rewrote my code using bitmap_for_each_set_region(), which in turn uses
    > bitmap_next_set_region().
    >
    > Should I revert to my previous implementation that uses
    > find_next_zero_bit() and find_next_bit() ?
    >
    > [0] https://lore.kernel.org/linux-iio/20210715141742.15072-3-andrea.merello@gmail.com/

    No, you're doing right. I just reimplemented and renamed
    bitmap_for_each_set_region to unify with other similar functions. The
    transition is very stragthforward, so you have nothing to worry on.

    https://patchwork.kernel.org/project/linux-mmc/patch/20210719021755.883182-3-yury.norov@gmail.com/

    >
    > > https://github.com/norov/linux/commit/1c870b5c3fcd2eea9b351a1e0af8d1e93be78e1e
    > >
    > > Recently in next-20210716, there appeared another user in fs/btrfs/extent_io.c:
    > > find_next_dirty_byte(). The fun is that in that case the length of bitmap
    > > is 16 bit, so it's probably simpler to return the bitmap by value,
    > > instead of calling prologue code up to 8 times. Anyways, I'll contact
    > > authors of the find_next_dirty_byte() and ask if it's possible to rework
    > > their code.
    > >
    > > > Yuri, do you have a public repo / branch that people can base their changes to
    > > > bitmap stuff against of?
    > >
    > > I collected not yet upstreamed bitmap patches here:
    > >
    > > https://github.com/norov/linux/commits/bitmap-20210716
    >
    > Looks like also bitmap_for_each_set_region() went away indeed ?

    Just replaced with for_each_{set,clear}_bitrange.

    Thanks,
    Yury

    \
     
     \ /
      Last update: 2021-07-28 15:48    [W:5.907 / U:0.080 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site