On Mon, Nov 16, 2020 at 10:38:58AM +0100, Hannes Reinecke wrote:
> On 11/16/20 10:07 AM, Ming Lei wrote:
> > SCSI's .device_busy will be converted to sbitmap, and sbitmap_weight
> > is needed, so export the helper.
> >
> > Cc: Omar Sandoval <osandov@xxxxxx>
> > Cc: Kashyap Desai <kashyap.desai@xxxxxxxxxxxx>
> > Cc: Sumanesh Samanta <sumanesh.samanta@xxxxxxxxxxxx>
> > Cc: Ewan D. Milne <emilne@xxxxxxxxxx>
> > Reviewed-by: Hannes Reinecke <hare@xxxxxxx>
> > Tested-by: Sumanesh Samanta <sumanesh.samanta@xxxxxxxxxxxx>
> > Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
> > ---
> > include/linux/sbitmap.h | 9 +++++++++
> > lib/sbitmap.c | 11 ++++++-----
> > 2 files changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/sbitmap.h b/include/linux/sbitmap.h
> > index 103b41c03311..34343ce3ef6c 100644
> > --- a/include/linux/sbitmap.h
> > +++ b/include/linux/sbitmap.h
> > @@ -346,6 +346,15 @@ static inline int sbitmap_test_bit(struct sbitmap *sb, unsigned int bitnr)
> > */
> > void sbitmap_show(struct sbitmap *sb, struct seq_file *m);
> > +
> > +/**
> > + * sbitmap_weight() - Return how many real bits set in a &struct sbitmap.
> > + * @sb: Bitmap to check.
> > + *
> > + * Return: How many real bits set
> > + */
> > +unsigned int sbitmap_weight(const struct sbitmap *sb);
> > +
> > /**
> > * sbitmap_bitmap_show() - Write a hex dump of a &struct sbitmap to a &struct
> > * seq_file.
> > diff --git a/lib/sbitmap.c b/lib/sbitmap.c
> > index dcd6a89b4d2f..fb1d3c2f70a2 100644
> > --- a/lib/sbitmap.c
> > +++ b/lib/sbitmap.c
> > @@ -342,20 +342,21 @@ static unsigned int __sbitmap_weight(const struct sbitmap *sb, bool set)
> > return weight;
> > }
> > -static unsigned int sbitmap_weight(const struct sbitmap *sb)
> > +static unsigned int sbitmap_cleared(const struct sbitmap *sb)
> > {
> > - return __sbitmap_weight(sb, true);
> > + return __sbitmap_weight(sb, false);
> > }
> > -static unsigned int sbitmap_cleared(const struct sbitmap *sb)
> > +unsigned int sbitmap_weight(const struct sbitmap *sb)
> > {
> > - return __sbitmap_weight(sb, false);
> > + return __sbitmap_weight(sb, true) - sbitmap_cleared(sb);
> > }
> > +EXPORT_SYMBOL_GPL(sbitmap_weight);
> That is extremely confusing. Why do you change the meaning of
> 'sbitmap_weight' from __sbitmap_weight(sb, true) to
> __sbitmap_weight(sb, true) - __sbitmap_weight(sb, false)?
Because the only user of sbitmap_weight() just uses the following way:
sbitmap_weight(sb) - sbitmap_cleared(sb)
Frankly, I think sbitmap_weight(sb) should return real busy bits.
> Does this mean that the original definition was wrong?
> Or does this mean that this patch implies a different meaning of
> 'sbitmap_weight'?
Yeah, this patch changes meaning of sbitmap_weight(), now it is
exported, and we should make it more accurate/readable from user view.
Thanks,
Ming
