[PATCH] xfs: replace kvmalloc_array with kvzalloc_objs

Lalit Shankar Chowdhury posted 1 patch 1 week ago
fs/xfs/xfs_zone_alloc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
[PATCH] xfs: replace kvmalloc_array with kvzalloc_objs
Posted by Lalit Shankar Chowdhury 1 week ago
Replace kvmalloc_array() with the more concise kvzalloc_objs()
implementation.

Signed-off-by: Lalit Shankar Chowdhury <lalitshankarch@gmail.com>
---
 fs/xfs/xfs_zone_alloc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
index 28c1e48909fa..82615045aa4c 100644
--- a/fs/xfs/xfs_zone_alloc.c
+++ b/fs/xfs/xfs_zone_alloc.c
@@ -1238,8 +1238,7 @@ static unsigned long *
 xfs_alloc_bucket_bitmap(
 	struct xfs_mount	*mp)
 {
-	return kvmalloc_array(BITS_TO_LONGS(mp->m_sb.sb_rgcount),
-			sizeof(unsigned long), GFP_KERNEL | __GFP_ZERO);
+	return kvzalloc_objs(unsigned long, BITS_TO_LONGS(mp->m_sb.sb_rgcount));
 }
 
 static struct xfs_zone_info *
-- 
2.53.0
Re: [PATCH] xfs: replace kvmalloc_array with kvzalloc_objs
Posted by Carlos Maiolino 6 days, 19 hours ago
On Thu, Sep 17, 2026 at 08:08:35PM +0000, Lalit Shankar Chowdhury wrote:
> Replace kvmalloc_array() with the more concise kvzalloc_objs()
> implementation.
> 
> Signed-off-by: Lalit Shankar Chowdhury <lalitshankarch@gmail.com>

NAK...

This is used to allocate composite objects managed through a
slab cache. This is not for fundamental basic data types....

> ---
>  fs/xfs/xfs_zone_alloc.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
> index 28c1e48909fa..82615045aa4c 100644
> --- a/fs/xfs/xfs_zone_alloc.c
> +++ b/fs/xfs/xfs_zone_alloc.c
> @@ -1238,8 +1238,7 @@ static unsigned long *
>  xfs_alloc_bucket_bitmap(
>  	struct xfs_mount	*mp)
>  {
> -	return kvmalloc_array(BITS_TO_LONGS(mp->m_sb.sb_rgcount),
> -			sizeof(unsigned long), GFP_KERNEL | __GFP_ZERO);
> +	return kvzalloc_objs(unsigned long, BITS_TO_LONGS(mp->m_sb.sb_rgcount));
>  }
>  
>  static struct xfs_zone_info *
> -- 
> 2.53.0
>
Re: [PATCH] xfs: replace kvmalloc_array with kvzalloc_objs
Posted by Christoph Hellwig 6 days, 13 hours ago
On Fri, Sep 18, 2026 at 07:16:32AM +0200, Carlos Maiolino wrote:
> On Thu, Sep 17, 2026 at 08:08:35PM +0000, Lalit Shankar Chowdhury wrote:
> > Replace kvmalloc_array() with the more concise kvzalloc_objs()
> > implementation.
> > 
> > Signed-off-by: Lalit Shankar Chowdhury <lalitshankarch@gmail.com>
> 
> NAK...
> 
> This is used to allocate composite objects managed through a
> slab cache. This is not for fundamental basic data types....

I don't think so.  While the naming of the helper is a bit
unfortunate, it is intended for allocating arrays.

But this function really should not exist, instead bitmap_alloc/free
should be switched to th kvmalloc family so that it will just work
for large allocations and we can kill the wrappers in XFS.
Re: [PATCH] xfs: replace kvmalloc_array with kvzalloc_objs
Posted by Carlos Maiolino 6 days, 12 hours ago
On Fri, Sep 18, 2026 at 04:16:46AM -0700, Christoph Hellwig wrote:
> On Fri, Sep 18, 2026 at 07:16:32AM +0200, Carlos Maiolino wrote:
> > On Thu, Sep 17, 2026 at 08:08:35PM +0000, Lalit Shankar Chowdhury wrote:
> > > Replace kvmalloc_array() with the more concise kvzalloc_objs()
> > > implementation.
> > > 
> > > Signed-off-by: Lalit Shankar Chowdhury <lalitshankarch@gmail.com>
> > 
> > NAK...
> > 
> > This is used to allocate composite objects managed through a
> > slab cache. This is not for fundamental basic data types....
> 
> I don't think so.  While the naming of the helper is a bit
> unfortunate, it is intended for allocating arrays.
> 
> But this function really should not exist, instead bitmap_alloc/free
> should be switched to th kvmalloc family so that it will just work
> for large allocations and we can kill the wrappers in XFS.
> 

You meant something like this? Leaving xfs_alloc_bucket_bitmap() looks
a bit better for me, to avoid overly long indentation.
I don't think there are free_bitmap_bucket though, buckets are straight
kvfree()'ed.


diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
index 28c1e48909fa..864020603dfb 100644
--- a/fs/xfs/xfs_zone_alloc.c
+++ b/fs/xfs/xfs_zone_alloc.c
@@ -1234,14 +1234,6 @@ xfs_calc_open_zones(
 	return 0;
 }
 
-static unsigned long *
-xfs_alloc_bucket_bitmap(
-	struct xfs_mount	*mp)
-{
-	return kvmalloc_array(BITS_TO_LONGS(mp->m_sb.sb_rgcount),
-			sizeof(unsigned long), GFP_KERNEL | __GFP_ZERO);
-}
-
 static struct xfs_zone_info *
 xfs_alloc_zone_info(
 	struct xfs_mount	*mp)
@@ -1260,7 +1252,10 @@ xfs_alloc_zone_info(
 	init_waitqueue_head(&zi->zi_zone_wait);
 	spin_lock_init(&zi->zi_used_buckets_lock);
 	for (i = 0; i < XFS_ZONE_USED_BUCKETS; i++) {
-		zi->zi_used_bucket_bitmap[i] = xfs_alloc_bucket_bitmap(mp);
+		zi->zi_used_bucket_bitmap[i] =
+			kvmalloc_array(BITS_TO_LONGS(mp->m_sb.sb_rgcount),
+					sizeof(unsigned long),
+					GFP_KERNEL | GFP_ZERO);
 		if (!zi->zi_used_bucket_bitmap[i])
 			goto out_free_bitmaps;
 	}
Re: [PATCH] xfs: replace kvmalloc_array with kvzalloc_objs
Posted by Christoph Hellwig 6 days, 10 hours ago
On Fri, Sep 18, 2026 at 01:44:12PM +0200, Carlos Maiolino wrote:
> You meant something like this? Leaving xfs_alloc_bucket_bitmap() looks
> a bit better for me, to avoid overly long indentation.
> I don't think there are free_bitmap_bucket though, buckets are straight
> kvfree()'ed.

No, I mean changing bitmap_*alloc* and bitmap_Free in lib/bitmap.c.
Re: [PATCH] xfs: replace kvmalloc_array with kvzalloc_objs
Posted by Carlos Maiolino 6 days, 13 hours ago
On Fri, Sep 18, 2026 at 04:16:46AM -0700, Christoph Hellwig wrote:
> On Fri, Sep 18, 2026 at 07:16:32AM +0200, Carlos Maiolino wrote:
> > On Thu, Sep 17, 2026 at 08:08:35PM +0000, Lalit Shankar Chowdhury wrote:
> > > Replace kvmalloc_array() with the more concise kvzalloc_objs()
> > > implementation.
> > > 
> > > Signed-off-by: Lalit Shankar Chowdhury <lalitshankarch@gmail.com>
> > 
> > NAK...
> > 
> > This is used to allocate composite objects managed through a
> > slab cache. This is not for fundamental basic data types....
> 
> I don't think so.  While the naming of the helper is a bit
> unfortunate, it is intended for allocating arrays.

I see your point, but what kv(z)alloc_objs() do is essentially

kmalloc(sizeof(object type) * count, GFP)...

While I see why it's useful for composite types, I don't see the much
the point of doing kvzalloc_objs(int, count)..
And still kvmalloc_array() looks a better fit in this case IMO than
kvmalloc_objs().

> 
> But this function really should not exist, instead bitmap_alloc/free
> should be switched to th kvmalloc family so that it will just work
> for large allocations and we can kill the wrappers in XFS.
> 

+1