[0/4] Introduce bitmap_size()

Message

Christophe JAILLET July 2, 2022, 6:28 p.m. UTC

This serie introduces bitmap_size() which returns the size, in bytes, of a
bitmap. Such a function is useful to simplify some drivers that use vmalloc() or
other functions to allocate some butmaps.
It also hides some implementation details about how bitmaps are stored (array of
longs)

Before introducing this function in patch 3, patch 1 and 2 rename some functions
with the same name but with different meaning.

Finaly, patch 4 makes use of the new function in bitmap.h.


Other follow-up patches to simplify some drivers will be proposed later if/when
this serie is merged.

Christophe JAILLET (4):
  s390/cio: Rename bitmap_size() as idset_bitmap_size()
  fs/ntfs3: Rename bitmap_size() as ntfs3_bitmap_size()
  bitmap: Introduce bitmap_size()
  bitmap: Use bitmap_size()

 drivers/md/dm-clone-metadata.c |  5 -----
 drivers/s390/cio/idset.c       |  8 ++++----
 fs/ntfs3/bitmap.c              |  4 ++--
 fs/ntfs3/fsntfs.c              |  2 +-
 fs/ntfs3/index.c               |  6 +++---
 fs/ntfs3/ntfs_fs.h             |  2 +-
 fs/ntfs3/super.c               |  2 +-
 include/linux/bitmap.h         | 15 +++++++++------
 lib/math/prime_numbers.c       |  2 --
 9 files changed, 21 insertions(+), 25 deletions(-)

Comments

Andy Shevchenko July 2, 2022, 6:58 p.m. UTC | #1

On Sat, Jul 02, 2022 at 08:29:27PM +0200, Christophe JAILLET wrote:
> In order to introduce a bitmap_size() function in the bitmap API, we have
> to rename functions with a similar name.

...

>  /* NTFS uses quad aligned bitmaps. */
> -static inline size_t bitmap_size(size_t bits)
> +static inline size_t ntfs3_bitmap_size(size_t bits)
>  {
>  	return ALIGN((bits + 7) >> 3, 8);

It would be easier to understand in a way

	return BITS_TO_BYTES(ALIGN(bits, 64));

>  }

Yury Norov July 2, 2022, 8:44 p.m. UTC | #2

On Sat, Jul 02, 2022 at 08:28:53PM +0200, Christophe JAILLET wrote:
> This serie introduces bitmap_size() which returns the size, in bytes, of a
> bitmap. Such a function is useful to simplify some drivers that use vmalloc() or
> other functions to allocate some butmaps.

This generally looks like a step in a wrong direction. Bitmap is by
definition an array of bits. If someone has a reason to handle bitmap
on a per-byte basis, the guy is probably doing something wrong.

We already have quite comprehensive list of functions that help to
allocate, fill, clear, copy etc bitmap without considering it as an
array of bytes or words.

> ... some drivers that use vmalloc() ...

If a driver needs vmalloc() for a bitmap, we should introduce
bitmap_vmalloc(), not bitmap_size().

> It also hides some implementation details about how bitmaps are stored (array of
> longs)

Sorry, I don't understand that. How bitmap_size() helps to hide a fact that
bitmap is an array of unsigned longs? (Except that it makes an impression
that it's an array of bytes.)

> Before introducing this function in patch 3, patch 1 and 2 rename some functions
> with the same name but with different meaning.
> 
> Finaly, patch 4 makes use of the new function in bitmap.h.

You mentioned, you need bitmap_size() to use with vmalloc(), but in
patch 4, there's no such code. 
 
> Other follow-up patches to simplify some drivers will be proposed later if/when
> this serie is merged.

This series doesn't show an example of how you're going to use new
API, and therefore it's hard to judge, do we really need bitmap_size(),
or we just need more helpers around.

As I already said, bitmaps are evolving in 2nd direction, which is the
right approach, I think.

Thanks,
Yury

> Christophe JAILLET (4):
>   s390/cio: Rename bitmap_size() as idset_bitmap_size()
>   fs/ntfs3: Rename bitmap_size() as ntfs3_bitmap_size()
>   bitmap: Introduce bitmap_size()
>   bitmap: Use bitmap_size()
> 
>  drivers/md/dm-clone-metadata.c |  5 -----
>  drivers/s390/cio/idset.c       |  8 ++++----
>  fs/ntfs3/bitmap.c              |  4 ++--
>  fs/ntfs3/fsntfs.c              |  2 +-
>  fs/ntfs3/index.c               |  6 +++---
>  fs/ntfs3/ntfs_fs.h             |  2 +-
>  fs/ntfs3/super.c               |  2 +-
>  include/linux/bitmap.h         | 15 +++++++++------
>  lib/math/prime_numbers.c       |  2 --
>  9 files changed, 21 insertions(+), 25 deletions(-)
> 
> -- 
> 2.34.1

--
dm-devel mailing list
[e-mail]
https://listman.redhat.com/mailman/listinfo/dm-devel