Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Date:   Tue, 26 Sep 2023 19:10:33 -0700
From:   Yury Norov <yury.norov@gmail.com>
To:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc:     Linus Walleij <linus.walleij@linaro.org>,
        Bartosz Golaszewski <bartosz.golaszewski@linaro.org>,
        linux-gpio@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org,
        Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>,
        Srinivas Neeli <srinivas.neeli@amd.com>,
        Michal Simek <michal.simek@amd.com>,
        Bartosz Golaszewski <brgl@bgdev.pl>,
        Andy Shevchenko <andy@kernel.org>,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        Marek =?iso-8859-1?Q?Beh=FAn?= <kabel@kernel.org>
Subject: Re: [PATCH v1 2/5] lib/bitmap: Introduce bitmap_scatter() and
 bitmap_gather() helpers
Message-ID: <ZROO9skQbYw1N9YP@yury-ThinkPad>
References: <20230926052007.3917389-1-andriy.shevchenko@linux.intel.com>
 <20230926052007.3917389-3-andriy.shevchenko@linux.intel.com>
 <ZRN2adZZaGeqWNlY@yury-ThinkPad>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZRN2adZZaGeqWNlY@yury-ThinkPad>
Precedence: bulk

> > +unsigned int bitmap_gather(unsigned long *dst, const unsigned long *src,
> > +			   const unsigned long *mask, unsigned int nbits)
> > +{
> > +	unsigned int bit;
> > +	int n = 0;
> > +
> > +	bitmap_zero(dst, nbits);
> > +
> > +	for_each_set_bit(bit, mask, nbits)
> > +		__assign_bit(n++, dst, test_bit(bit, src));
> > +
> > +	return n;
> > +}
> > +EXPORT_SYMBOL(bitmap_gather);

So, if mask is 0b01, and src is 0b10, the output will be 0b00.
To me it sounds like you've gathered nothing, while the intention
was to gather all source bits to bit #0. This is my understanding
of the word 'gather', and this is how bitmap_remap() works.

bitmap_remap() handles it by wrapping around 0:
        set_bit(find_nth_bit(new, nbits, n % w), dst);

In your case, it may look like:
        n = off = 0;
        while (1) {
                off += n;
                n = 0;
        	for_each_set_bit(bit, mask, nbits) {
                        if (bit + off >= nbits)
                                return;
        		__assign_bit(n++, dst, test_bit(bit + off, src));
                }
        }

(Not tested, except that on piece of paper.)

If you claim you're replacing bitmap_remap(), you should correctly handle
the above case; when src == dst; when mask is empty, and probably more...

Thanks,
Yury