Re: [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()

[David Laight]

On Wed, 25 Feb 2026 15:44:46 +0100
David Sterba <dsterba@xxxxxxx> wrote:

> On Tue, Feb 24, 2026 at 02:55:55PM +0000, David Laight wrote:
> > On Tue, 24 Feb 2026 15:07:10 +1030
> > Qu Wenruo <quwenruo.btrfs@xxxxxxx> wrote:
> >   
> > > 在 2026/2/24 10:14, Miquel Sabaté Solà 写道:  
> > > > Commit 2932ba8d9c99 ("slab: Introduce kmalloc_obj() and family")
> > > > introduced, among many others, the kzalloc_objs() helper, which has some
> > > > benefits over kcalloc().
> > > > 
> > > > Cc: Kees Cook <kees@xxxxxxxxxx>
> > > > Signed-off-by: Miquel Sabaté Solà <mssola@xxxxxxxxxx>
> > > > ---
> > > >   fs/btrfs/block-group.c       | 2 +-
> > > >   fs/btrfs/raid56.c            | 8 ++++----
> > > >   fs/btrfs/tests/zoned-tests.c | 2 +-
> > > >   fs/btrfs/volumes.c           | 6 ++----
> > > >   fs/btrfs/zoned.c             | 5 ++---
> > > >   5 files changed, 10 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> > > > index 37bea850b3f0..8d85b4707690 100644
> > > > --- a/fs/btrfs/block-group.c
> > > > +++ b/fs/btrfs/block-group.c
> > > > @@ -2239,7 +2239,7 @@ int btrfs_rmap_block(struct btrfs_fs_info *fs_info, u64 chunk_start,
> > > >   	if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK)
> > > >   		io_stripe_size = btrfs_stripe_nr_to_offset(nr_data_stripes(map));
> > > >   
> > > > -	buf = kcalloc(map->num_stripes, sizeof(u64), GFP_NOFS);
> > > > +	buf = kzalloc_objs(*buf, map->num_stripes, GFP_NOFS);    
> > > 
> > > Not sure if we should use *buf for the type.
> > > 
> > > I still remember we had some bugs related to incorrect type usage.  
> > 
> > The global change really ought to have used u64 to add the type-check.
> > Otherwise it will have added 'very hard to find' bugs in the very code
> > it is trying to make better.
> > 
> > Using *buf for the type might be a reasonable pattern for new code.  
> 
> I find this a bit contradictory: I agree that using *buf as the argument
> can cause bugs hard to find, yet the next sentence recommends to use it.

The issue is that mechanically changing:
	buf = kzalloc(sizeof(type),...);
to:
	buf = kzalloc_obj(*buf, ...);
is that you've silently changed the size of the allocated memory
if 'type' wasn't actually the correct type.
Whereas changing it so:
	buf = kzalloc_obj(type, ...);
will give a compiler error if/when the types don't match.
(There may be places where this is exactly what is intended.)

For a big mechanical change you really want to err on the side of caution.

For new code it is a bit different.
kzalloc_obj() will pick up silly mistakes, so both:
	auto buf = kzalloc_obj(type, ...);
and:
	type *buf = kzalloc_obj(*buf, ...);
are reasonable patterns.
The former may actually read better as 'allocate an object of this type'
and doesn't require that you replicate the type.

	David

> 
> This kzalloc_obj way is new I'm analyzing what would be a good pattern
> and so far I don't like the "*buf" style of 1st argument. As the
> function is really a macro it does not dereference it but it still
> appears as it does.
> 
> Writing the type explicitly looks still more like a C to me. Types in
> arguments are in helpers like container_of or rb_entry and it makes it
> obvious that there's something special while for the kzalloc_obj I need
> to remember it.
> 
> The whole thing would read better as "allocate object of type", so I'm
> probably going to convert it to this pattern in btrfs code.