Re: [PATCH V4 05/12] sbitmap: export sbitmap_weight

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/17/20 3:10 AM, Ming Lei wrote:
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.

No argument about that. Just wanted to be clear that this is by intention.

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.

So can you please state this in the patch description?

Cheers,

Hannes
--
Dr. Hannes Reinecke                Kernel Storage Architect
hare@xxxxxxx                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux