public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()
@ 2026-02-23 23:44 Miquel Sabaté Solà
  2026-02-24  0:06 ` Kees Cook
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Miquel Sabaté Solà @ 2026-02-23 23:44 UTC (permalink / raw)
  To: dsterba
  Cc: clm, naohiro.aota, linux-btrfs, linux-kernel, kees,
	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@kernel.org>
Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>
---
 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);
 	if (!buf) {
 		ret = -ENOMEM;
 		goto out;
diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index 02105d68accb..1ebfed8f0a0a 100644
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -2110,8 +2110,8 @@ static int recover_sectors(struct btrfs_raid_bio *rbio)
 	 * @unmap_array stores copy of pointers that does not get reordered
 	 * during reconstruction so that kunmap_local works.
 	 */
-	pointers = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
-	unmap_array = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
+	pointers = kzalloc_objs(*pointers, rbio->real_stripes, GFP_NOFS);
+	unmap_array = kzalloc_objs(*unmap_array, rbio->real_stripes, GFP_NOFS);
 	if (!pointers || !unmap_array) {
 		ret = -ENOMEM;
 		goto out;
@@ -2844,8 +2844,8 @@ static int recover_scrub_rbio(struct btrfs_raid_bio *rbio)
 	 * @unmap_array stores copy of pointers that does not get reordered
 	 * during reconstruction so that kunmap_local works.
 	 */
-	pointers = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
-	unmap_array = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
+	pointers = kzalloc_objs(*pointers, rbio->real_stripes, GFP_NOFS);
+	unmap_array = kzalloc_objs(*unmap_array, rbio->real_stripes, GFP_NOFS);
 	if (!pointers || !unmap_array) {
 		ret = -ENOMEM;
 		goto out;
diff --git a/fs/btrfs/tests/zoned-tests.c b/fs/btrfs/tests/zoned-tests.c
index da21c7aea31a..2bc3b14baa41 100644
--- a/fs/btrfs/tests/zoned-tests.c
+++ b/fs/btrfs/tests/zoned-tests.c
@@ -58,7 +58,7 @@ static int test_load_zone_info(struct btrfs_fs_info *fs_info,
 		return -ENOMEM;
 	}
 
-	zone_info = kcalloc(test->num_stripes, sizeof(*zone_info), GFP_KERNEL);
+	zone_info = kzalloc_objs(*zone_info, test->num_stripes, GFP_KERNEL);
 	if (!zone_info) {
 		test_err("cannot allocate zone info");
 		return -ENOMEM;
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index e15e138c515b..c0cf8f7c5a8e 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5499,8 +5499,7 @@ static int calc_one_profile_avail(struct btrfs_fs_info *fs_info,
 		goto out;
 	}
 
-	devices_info = kcalloc(fs_devices->rw_devices, sizeof(*devices_info),
-			       GFP_NOFS);
+	devices_info = kzalloc_objs(*devices_info, fs_devices->rw_devices, GFP_NOFS);
 	if (!devices_info) {
 		ret = -ENOMEM;
 		goto out;
@@ -6067,8 +6066,7 @@ struct btrfs_block_group *btrfs_create_chunk(struct btrfs_trans_handle *trans,
 	ctl.space_info = space_info;
 	init_alloc_chunk_ctl(fs_devices, &ctl);
 
-	devices_info = kcalloc(fs_devices->rw_devices, sizeof(*devices_info),
-			       GFP_NOFS);
+	devices_info = kzalloc_objs(*devices_info, fs_devices->rw_devices, GFP_NOFS);
 	if (!devices_info)
 		return ERR_PTR(-ENOMEM);
 
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index ab330ec957bc..851b0de7bed7 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1697,8 +1697,7 @@ static int btrfs_load_block_group_raid10(struct btrfs_block_group *bg,
 		return -EINVAL;
 	}
 
-	raid0_allocs = kcalloc(map->num_stripes / map->sub_stripes, sizeof(*raid0_allocs),
-			       GFP_NOFS);
+	raid0_allocs = kzalloc_objs(*raid0_allocs, map->num_stripes / map->sub_stripes, GFP_NOFS);
 	if (!raid0_allocs)
 		return -ENOMEM;
 
@@ -1916,7 +1915,7 @@ int btrfs_load_block_group_zone_info(struct btrfs_block_group *cache, bool new)
 
 	cache->physical_map = map;
 
-	zone_info = kcalloc(map->num_stripes, sizeof(*zone_info), GFP_NOFS);
+	zone_info = kzalloc_objs(*zone_info, map->num_stripes, GFP_NOFS);
 	if (!zone_info) {
 		ret = -ENOMEM;
 		goto out;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()
  2026-02-23 23:44 [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs() Miquel Sabaté Solà
@ 2026-02-24  0:06 ` Kees Cook
  2026-02-24  6:23   ` Miquel Sabaté Solà
       [not found]   ` <699d43e6.170a0220.3a6e96.a235SMTPIN_ADDED_BROKEN@mx.google.com>
  2026-02-24  4:37 ` Qu Wenruo
  2026-02-24  6:32 ` Johannes Thumshirn
  2 siblings, 2 replies; 18+ messages in thread
From: Kees Cook @ 2026-02-24  0:06 UTC (permalink / raw)
  To: Miquel Sabaté Solà
  Cc: dsterba, clm, naohiro.aota, linux-btrfs, linux-kernel

On Tue, Feb 24, 2026 at 12:44:51AM +0100, Miquel Sabaté Solà wrote:
> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
> index 02105d68accb..1ebfed8f0a0a 100644
> --- a/fs/btrfs/raid56.c
> +++ b/fs/btrfs/raid56.c
> @@ -2110,8 +2110,8 @@ static int recover_sectors(struct btrfs_raid_bio *rbio)
>  	 * @unmap_array stores copy of pointers that does not get reordered
>  	 * during reconstruction so that kunmap_local works.
>  	 */
> -	pointers = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
> -	unmap_array = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
> +	pointers = kzalloc_objs(*pointers, rbio->real_stripes, GFP_NOFS);
> +	unmap_array = kzalloc_objs(*unmap_array, rbio->real_stripes, GFP_NOFS);
>  	if (!pointers || !unmap_array) {
>  		ret = -ENOMEM;
>  		goto out;

Just as a style option, I wanted to point out (for at least the above,
I didn't check the rest), you can do the definition and declaration at
once with "auto" and put the type in the alloc:

	auto pointers = kzalloc_objs(void *, rbio->real_stripes, GFP_NOFS);

But either way is fine. :) This patch looks good to me!

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()
  2026-02-23 23:44 [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs() Miquel Sabaté Solà
  2026-02-24  0:06 ` Kees Cook
@ 2026-02-24  4:37 ` Qu Wenruo
  2026-02-24  4:42   ` Qu Wenruo
                     ` (3 more replies)
  2026-02-24  6:32 ` Johannes Thumshirn
  2 siblings, 4 replies; 18+ messages in thread
From: Qu Wenruo @ 2026-02-24  4:37 UTC (permalink / raw)
  To: Miquel Sabaté Solà, dsterba
  Cc: clm, naohiro.aota, linux-btrfs, linux-kernel, kees



在 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@kernel.org>
> Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>
> ---
>   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.

Another thing is, we may not want to use the kzalloc version.
We don't want to waste CPU time just to zero out the content meanwhile 
we're ensured to re-assign the contents.

Thus kmalloc_objs() maybe better.

Thanks,
Qu


>   	if (!buf) {
>   		ret = -ENOMEM;
>   		goto out;
> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
> index 02105d68accb..1ebfed8f0a0a 100644
> --- a/fs/btrfs/raid56.c
> +++ b/fs/btrfs/raid56.c
> @@ -2110,8 +2110,8 @@ static int recover_sectors(struct btrfs_raid_bio *rbio)
>   	 * @unmap_array stores copy of pointers that does not get reordered
>   	 * during reconstruction so that kunmap_local works.
>   	 */
> -	pointers = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
> -	unmap_array = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
> +	pointers = kzalloc_objs(*pointers, rbio->real_stripes, GFP_NOFS);
> +	unmap_array = kzalloc_objs(*unmap_array, rbio->real_stripes, GFP_NOFS);
>   	if (!pointers || !unmap_array) {
>   		ret = -ENOMEM;
>   		goto out;
> @@ -2844,8 +2844,8 @@ static int recover_scrub_rbio(struct btrfs_raid_bio *rbio)
>   	 * @unmap_array stores copy of pointers that does not get reordered
>   	 * during reconstruction so that kunmap_local works.
>   	 */
> -	pointers = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
> -	unmap_array = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
> +	pointers = kzalloc_objs(*pointers, rbio->real_stripes, GFP_NOFS);
> +	unmap_array = kzalloc_objs(*unmap_array, rbio->real_stripes, GFP_NOFS);
>   	if (!pointers || !unmap_array) {
>   		ret = -ENOMEM;
>   		goto out;
> diff --git a/fs/btrfs/tests/zoned-tests.c b/fs/btrfs/tests/zoned-tests.c
> index da21c7aea31a..2bc3b14baa41 100644
> --- a/fs/btrfs/tests/zoned-tests.c
> +++ b/fs/btrfs/tests/zoned-tests.c
> @@ -58,7 +58,7 @@ static int test_load_zone_info(struct btrfs_fs_info *fs_info,
>   		return -ENOMEM;
>   	}
>   
> -	zone_info = kcalloc(test->num_stripes, sizeof(*zone_info), GFP_KERNEL);
> +	zone_info = kzalloc_objs(*zone_info, test->num_stripes, GFP_KERNEL);
>   	if (!zone_info) {
>   		test_err("cannot allocate zone info");
>   		return -ENOMEM;
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index e15e138c515b..c0cf8f7c5a8e 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -5499,8 +5499,7 @@ static int calc_one_profile_avail(struct btrfs_fs_info *fs_info,
>   		goto out;
>   	}
>   
> -	devices_info = kcalloc(fs_devices->rw_devices, sizeof(*devices_info),
> -			       GFP_NOFS);
> +	devices_info = kzalloc_objs(*devices_info, fs_devices->rw_devices, GFP_NOFS);
>   	if (!devices_info) {
>   		ret = -ENOMEM;
>   		goto out;
> @@ -6067,8 +6066,7 @@ struct btrfs_block_group *btrfs_create_chunk(struct btrfs_trans_handle *trans,
>   	ctl.space_info = space_info;
>   	init_alloc_chunk_ctl(fs_devices, &ctl);
>   
> -	devices_info = kcalloc(fs_devices->rw_devices, sizeof(*devices_info),
> -			       GFP_NOFS);
> +	devices_info = kzalloc_objs(*devices_info, fs_devices->rw_devices, GFP_NOFS);
>   	if (!devices_info)
>   		return ERR_PTR(-ENOMEM);
>   
> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> index ab330ec957bc..851b0de7bed7 100644
> --- a/fs/btrfs/zoned.c
> +++ b/fs/btrfs/zoned.c
> @@ -1697,8 +1697,7 @@ static int btrfs_load_block_group_raid10(struct btrfs_block_group *bg,
>   		return -EINVAL;
>   	}
>   
> -	raid0_allocs = kcalloc(map->num_stripes / map->sub_stripes, sizeof(*raid0_allocs),
> -			       GFP_NOFS);
> +	raid0_allocs = kzalloc_objs(*raid0_allocs, map->num_stripes / map->sub_stripes, GFP_NOFS);
>   	if (!raid0_allocs)
>   		return -ENOMEM;
>   
> @@ -1916,7 +1915,7 @@ int btrfs_load_block_group_zone_info(struct btrfs_block_group *cache, bool new)
>   
>   	cache->physical_map = map;
>   
> -	zone_info = kcalloc(map->num_stripes, sizeof(*zone_info), GFP_NOFS);
> +	zone_info = kzalloc_objs(*zone_info, map->num_stripes, GFP_NOFS);
>   	if (!zone_info) {
>   		ret = -ENOMEM;
>   		goto out;


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()
  2026-02-24  4:37 ` Qu Wenruo
@ 2026-02-24  4:42   ` Qu Wenruo
  2026-02-24 13:52     ` David Sterba
  2026-02-24  6:36   ` Miquel Sabaté Solà
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Qu Wenruo @ 2026-02-24  4:42 UTC (permalink / raw)
  To: Miquel Sabaté Solà, dsterba
  Cc: clm, naohiro.aota, linux-btrfs, linux-kernel, kees



在 2026/2/24 15:07, Qu Wenruo 写道:
> 
> 
> 在 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@kernel.org>
>> Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>
>> ---
>>   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.
> 
> Another thing is, we may not want to use the kzalloc version.
> We don't want to waste CPU time just to zero out the content meanwhile 
> we're ensured to re-assign the contents.
> 
> Thus kmalloc_objs() maybe better.

Sorry I only mean for some particular call sites, like this one.

Not all call sites can be converted to kmalloc version, and will need 
proper inspection one by one.

Thanks,
Qu

> 
> Thanks,
> Qu
> 
> 
>>       if (!buf) {
>>           ret = -ENOMEM;
>>           goto out;
>> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
>> index 02105d68accb..1ebfed8f0a0a 100644
>> --- a/fs/btrfs/raid56.c
>> +++ b/fs/btrfs/raid56.c
>> @@ -2110,8 +2110,8 @@ static int recover_sectors(struct btrfs_raid_bio 
>> *rbio)
>>        * @unmap_array stores copy of pointers that does not get reordered
>>        * during reconstruction so that kunmap_local works.
>>        */
>> -    pointers = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>> -    unmap_array = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>> +    pointers = kzalloc_objs(*pointers, rbio->real_stripes, GFP_NOFS);
>> +    unmap_array = kzalloc_objs(*unmap_array, rbio->real_stripes, 
>> GFP_NOFS);
>>       if (!pointers || !unmap_array) {
>>           ret = -ENOMEM;
>>           goto out;
>> @@ -2844,8 +2844,8 @@ static int recover_scrub_rbio(struct 
>> btrfs_raid_bio *rbio)
>>        * @unmap_array stores copy of pointers that does not get reordered
>>        * during reconstruction so that kunmap_local works.
>>        */
>> -    pointers = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>> -    unmap_array = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>> +    pointers = kzalloc_objs(*pointers, rbio->real_stripes, GFP_NOFS);
>> +    unmap_array = kzalloc_objs(*unmap_array, rbio->real_stripes, 
>> GFP_NOFS);
>>       if (!pointers || !unmap_array) {
>>           ret = -ENOMEM;
>>           goto out;
>> diff --git a/fs/btrfs/tests/zoned-tests.c b/fs/btrfs/tests/zoned-tests.c
>> index da21c7aea31a..2bc3b14baa41 100644
>> --- a/fs/btrfs/tests/zoned-tests.c
>> +++ b/fs/btrfs/tests/zoned-tests.c
>> @@ -58,7 +58,7 @@ static int test_load_zone_info(struct btrfs_fs_info 
>> *fs_info,
>>           return -ENOMEM;
>>       }
>> -    zone_info = kcalloc(test->num_stripes, sizeof(*zone_info), 
>> GFP_KERNEL);
>> +    zone_info = kzalloc_objs(*zone_info, test->num_stripes, GFP_KERNEL);
>>       if (!zone_info) {
>>           test_err("cannot allocate zone info");
>>           return -ENOMEM;
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index e15e138c515b..c0cf8f7c5a8e 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -5499,8 +5499,7 @@ static int calc_one_profile_avail(struct 
>> btrfs_fs_info *fs_info,
>>           goto out;
>>       }
>> -    devices_info = kcalloc(fs_devices->rw_devices, 
>> sizeof(*devices_info),
>> -                   GFP_NOFS);
>> +    devices_info = kzalloc_objs(*devices_info, fs_devices- 
>> >rw_devices, GFP_NOFS);
>>       if (!devices_info) {
>>           ret = -ENOMEM;
>>           goto out;
>> @@ -6067,8 +6066,7 @@ struct btrfs_block_group 
>> *btrfs_create_chunk(struct btrfs_trans_handle *trans,
>>       ctl.space_info = space_info;
>>       init_alloc_chunk_ctl(fs_devices, &ctl);
>> -    devices_info = kcalloc(fs_devices->rw_devices, 
>> sizeof(*devices_info),
>> -                   GFP_NOFS);
>> +    devices_info = kzalloc_objs(*devices_info, fs_devices- 
>> >rw_devices, GFP_NOFS);
>>       if (!devices_info)
>>           return ERR_PTR(-ENOMEM);
>> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
>> index ab330ec957bc..851b0de7bed7 100644
>> --- a/fs/btrfs/zoned.c
>> +++ b/fs/btrfs/zoned.c
>> @@ -1697,8 +1697,7 @@ static int btrfs_load_block_group_raid10(struct 
>> btrfs_block_group *bg,
>>           return -EINVAL;
>>       }
>> -    raid0_allocs = kcalloc(map->num_stripes / map->sub_stripes, 
>> sizeof(*raid0_allocs),
>> -                   GFP_NOFS);
>> +    raid0_allocs = kzalloc_objs(*raid0_allocs, map->num_stripes / 
>> map->sub_stripes, GFP_NOFS);
>>       if (!raid0_allocs)
>>           return -ENOMEM;
>> @@ -1916,7 +1915,7 @@ int btrfs_load_block_group_zone_info(struct 
>> btrfs_block_group *cache, bool new)
>>       cache->physical_map = map;
>> -    zone_info = kcalloc(map->num_stripes, sizeof(*zone_info), GFP_NOFS);
>> +    zone_info = kzalloc_objs(*zone_info, map->num_stripes, GFP_NOFS);
>>       if (!zone_info) {
>>           ret = -ENOMEM;
>>           goto out;
> 
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()
  2026-02-24  0:06 ` Kees Cook
@ 2026-02-24  6:23   ` Miquel Sabaté Solà
       [not found]   ` <699d43e6.170a0220.3a6e96.a235SMTPIN_ADDED_BROKEN@mx.google.com>
  1 sibling, 0 replies; 18+ messages in thread
From: Miquel Sabaté Solà @ 2026-02-24  6:23 UTC (permalink / raw)
  To: Kees Cook; +Cc: dsterba, clm, naohiro.aota, linux-btrfs, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1372 bytes --]

Kees Cook @ 2026-02-23 16:06 -08:

> On Tue, Feb 24, 2026 at 12:44:51AM +0100, Miquel Sabaté Solà wrote:
>> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
>> index 02105d68accb..1ebfed8f0a0a 100644
>> --- a/fs/btrfs/raid56.c
>> +++ b/fs/btrfs/raid56.c
>> @@ -2110,8 +2110,8 @@ static int recover_sectors(struct btrfs_raid_bio *rbio)
>>  	 * @unmap_array stores copy of pointers that does not get reordered
>>  	 * during reconstruction so that kunmap_local works.
>>  	 */
>> -	pointers = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>> -	unmap_array = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>> +	pointers = kzalloc_objs(*pointers, rbio->real_stripes, GFP_NOFS);
>> +	unmap_array = kzalloc_objs(*unmap_array, rbio->real_stripes, GFP_NOFS);
>>  	if (!pointers || !unmap_array) {
>>  		ret = -ENOMEM;
>>  		goto out;
>
> Just as a style option, I wanted to point out (for at least the above,
> I didn't check the rest), you can do the definition and declaration at
> once with "auto" and put the type in the alloc:
>
> 	auto pointers = kzalloc_objs(void *, rbio->real_stripes, GFP_NOFS);
>
> But either way is fine. :) This patch looks good to me!

I personally don't mind either way, but I don't what's the policy around
using "auto" in btrfs.

>
> Reviewed-by: Kees Cook <kees@kernel.org>

Thanks!
Miquel

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 897 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()
  2026-02-23 23:44 [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs() Miquel Sabaté Solà
  2026-02-24  0:06 ` Kees Cook
  2026-02-24  4:37 ` Qu Wenruo
@ 2026-02-24  6:32 ` Johannes Thumshirn
  2026-02-24  6:46   ` Miquel Sabaté Solà
  2 siblings, 1 reply; 18+ messages in thread
From: Johannes Thumshirn @ 2026-02-24  6:32 UTC (permalink / raw)
  To: Miquel Sabaté Solà, dsterba@suse.com
  Cc: clm@fb.com, Naohiro Aota, linux-btrfs@vger.kernel.org,
	linux-kernel@vger.kernel.org, kees@kernel.org

On 2/24/26 12:45 AM, Miquel Sabaté Solà wrote:
> Commit 2932ba8d9c99 ("slab: Introduce kmalloc_obj() and family")
> introduced, among many others, the kzalloc_objs() helper, which has some
> benefits over kcalloc().
Namely?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()
  2026-02-24  4:37 ` Qu Wenruo
  2026-02-24  4:42   ` Qu Wenruo
@ 2026-02-24  6:36   ` Miquel Sabaté Solà
       [not found]   ` <699d4704.050a0220.1a6450.86d7SMTPIN_ADDED_BROKEN@mx.google.com>
  2026-02-24 14:55   ` David Laight
  3 siblings, 0 replies; 18+ messages in thread
From: Miquel Sabaté Solà @ 2026-02-24  6:36 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: dsterba, clm, naohiro.aota, linux-btrfs, linux-kernel, kees

[-- Attachment #1: Type: text/plain, Size: 5931 bytes --]

Qu Wenruo @ 2026-02-24 15:07 +1030:

> 在 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@kernel.org>
>> Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>
>> ---
>>   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.

Considering the type of 'buf' and how kzalloc_objs() will resolve the
first argument '*buf', it should really just be equivalent to what was
written before.

>
> Another thing is, we may not want to use the kzalloc version.
> We don't want to waste CPU time just to zero out the content meanwhile we're
> ensured to re-assign the contents.
>
> Thus kmalloc_objs() maybe better.

Yes, having a second look at this function, it looks like kmalloc_objs()
might just be enough. If you don't mind, I will add another commit in v2
translating kzalloc_objs() into kmalloc_objs() wherever I see we can do
this from the ones I've touched. This way we can easily revert in case
things go south :)

>
> Thanks,
> Qu

Thanks,
Miquel

>
>
>>   	if (!buf) {
>>   		ret = -ENOMEM;
>>   		goto out;
>> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
>> index 02105d68accb..1ebfed8f0a0a 100644
>> --- a/fs/btrfs/raid56.c
>> +++ b/fs/btrfs/raid56.c
>> @@ -2110,8 +2110,8 @@ static int recover_sectors(struct btrfs_raid_bio *rbio)
>>   	 * @unmap_array stores copy of pointers that does not get reordered
>>   	 * during reconstruction so that kunmap_local works.
>>   	 */
>> -	pointers = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>> -	unmap_array = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>> +	pointers = kzalloc_objs(*pointers, rbio->real_stripes, GFP_NOFS);
>> +	unmap_array = kzalloc_objs(*unmap_array, rbio->real_stripes, GFP_NOFS);
>>   	if (!pointers || !unmap_array) {
>>   		ret = -ENOMEM;
>>   		goto out;
>> @@ -2844,8 +2844,8 @@ static int recover_scrub_rbio(struct btrfs_raid_bio *rbio)
>>   	 * @unmap_array stores copy of pointers that does not get reordered
>>   	 * during reconstruction so that kunmap_local works.
>>   	 */
>> -	pointers = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>> -	unmap_array = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>> +	pointers = kzalloc_objs(*pointers, rbio->real_stripes, GFP_NOFS);
>> +	unmap_array = kzalloc_objs(*unmap_array, rbio->real_stripes, GFP_NOFS);
>>   	if (!pointers || !unmap_array) {
>>   		ret = -ENOMEM;
>>   		goto out;
>> diff --git a/fs/btrfs/tests/zoned-tests.c b/fs/btrfs/tests/zoned-tests.c
>> index da21c7aea31a..2bc3b14baa41 100644
>> --- a/fs/btrfs/tests/zoned-tests.c
>> +++ b/fs/btrfs/tests/zoned-tests.c
>> @@ -58,7 +58,7 @@ static int test_load_zone_info(struct btrfs_fs_info *fs_info,
>>   		return -ENOMEM;
>>   	}
>>   -	zone_info = kcalloc(test->num_stripes, sizeof(*zone_info), GFP_KERNEL);
>> +	zone_info = kzalloc_objs(*zone_info, test->num_stripes, GFP_KERNEL);
>>   	if (!zone_info) {
>>   		test_err("cannot allocate zone info");
>>   		return -ENOMEM;
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index e15e138c515b..c0cf8f7c5a8e 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -5499,8 +5499,7 @@ static int calc_one_profile_avail(struct btrfs_fs_info *fs_info,
>>   		goto out;
>>   	}
>>   -	devices_info = kcalloc(fs_devices->rw_devices, sizeof(*devices_info),
>> -			       GFP_NOFS);
>> +	devices_info = kzalloc_objs(*devices_info, fs_devices->rw_devices, GFP_NOFS);
>>   	if (!devices_info) {
>>   		ret = -ENOMEM;
>>   		goto out;
>> @@ -6067,8 +6066,7 @@ struct btrfs_block_group *btrfs_create_chunk(struct btrfs_trans_handle *trans,
>>   	ctl.space_info = space_info;
>>   	init_alloc_chunk_ctl(fs_devices, &ctl);
>>   -	devices_info = kcalloc(fs_devices->rw_devices, sizeof(*devices_info),
>> -			       GFP_NOFS);
>> +	devices_info = kzalloc_objs(*devices_info, fs_devices->rw_devices, GFP_NOFS);
>>   	if (!devices_info)
>>   		return ERR_PTR(-ENOMEM);
>>   diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
>> index ab330ec957bc..851b0de7bed7 100644
>> --- a/fs/btrfs/zoned.c
>> +++ b/fs/btrfs/zoned.c
>> @@ -1697,8 +1697,7 @@ static int btrfs_load_block_group_raid10(struct btrfs_block_group *bg,
>>   		return -EINVAL;
>>   	}
>>   -	raid0_allocs = kcalloc(map->num_stripes / map->sub_stripes,
>> sizeof(*raid0_allocs),
>> -			       GFP_NOFS);
>> +	raid0_allocs = kzalloc_objs(*raid0_allocs, map->num_stripes / map->sub_stripes, GFP_NOFS);
>>   	if (!raid0_allocs)
>>   		return -ENOMEM;
>>   @@ -1916,7 +1915,7 @@ int btrfs_load_block_group_zone_info(struct
>> btrfs_block_group *cache, bool new)
>>     	cache->physical_map = map;
>>   -	zone_info = kcalloc(map->num_stripes, sizeof(*zone_info), GFP_NOFS);
>> +	zone_info = kzalloc_objs(*zone_info, map->num_stripes, GFP_NOFS);
>>   	if (!zone_info) {
>>   		ret = -ENOMEM;
>>   		goto out;

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 897 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()
  2026-02-24  6:32 ` Johannes Thumshirn
@ 2026-02-24  6:46   ` Miquel Sabaté Solà
  2026-02-24  6:54     ` Qu Wenruo
  0 siblings, 1 reply; 18+ messages in thread
From: Miquel Sabaté Solà @ 2026-02-24  6:46 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: dsterba@suse.com, clm@fb.com, Naohiro Aota,
	linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org,
	kees@kernel.org

[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]

Johannes Thumshirn @ 2026-02-24 06:32 GMT:

> On 2/24/26 12:45 AM, Miquel Sabaté Solà wrote:
>> Commit 2932ba8d9c99 ("slab: Introduce kmalloc_obj() and family")
>> introduced, among many others, the kzalloc_objs() helper, which has some
>> benefits over kcalloc().
> Namely?

I didn't want to repeat the arguments from the quoted commit
2932ba8d9c99 ("slab: Introduce kmalloc_obj() and family"). Namely:

> Internal introspection of the allocated type now becomes possible,
> allowing for future alignment-aware choices to be made by the
> allocator and future hardening work that can be type sensitive.

Should I put this in the commit message as well, regardless of the
commit explaining this being quoted?

There's also the argument of dropping 'sizeof' to be more ergonomic. To
me, though, and considering how these helpers have been applied
tree-wide, I see this change more as aligning us with this recent
tree-wide move, which also affected btrfs (see commit 69050f8d6d07
"treewide: Replace kmalloc with kmalloc_obj for non-scalar types").

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 897 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()
       [not found]   ` <699d4704.050a0220.1a6450.86d7SMTPIN_ADDED_BROKEN@mx.google.com>
@ 2026-02-24  6:48     ` Qu Wenruo
  2026-02-24  8:59       ` Miquel Sabaté Solà
  0 siblings, 1 reply; 18+ messages in thread
From: Qu Wenruo @ 2026-02-24  6:48 UTC (permalink / raw)
  To: Miquel Sabaté Solà, Qu Wenruo
  Cc: dsterba, clm, naohiro.aota, linux-btrfs, linux-kernel, kees



在 2026/2/24 17:06, Miquel Sabaté Solà 写道:
> Qu Wenruo @ 2026-02-24 15:07 +1030:
> 
>> 在 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@kernel.org>
>>> Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>
>>> ---
>>>    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.
> 
> Considering the type of 'buf' and how kzalloc_objs() will resolve the
> first argument '*buf', it should really just be equivalent to what was
> written before.

Good luck if you miss the '*' for a structure pointer, and compiler 
won't give you any warning.

I still remembered that Johannes exposed such bug for me.
Unfortunately I didn't have exact lore link for it.

> 
>>
>> Another thing is, we may not want to use the kzalloc version.
>> We don't want to waste CPU time just to zero out the content meanwhile we're
>> ensured to re-assign the contents.
>>
>> Thus kmalloc_objs() maybe better.
> 
> Yes, having a second look at this function, it looks like kmalloc_objs()
> might just be enough. If you don't mind, I will add another commit in v2
> translating kzalloc_objs() into kmalloc_objs() wherever I see we can do
> this from the ones I've touched. This way we can easily revert in case
> things go south :)
> 
>>
>> Thanks,
>> Qu
> 
> Thanks,
> Miquel
> 
>>
>>
>>>    	if (!buf) {
>>>    		ret = -ENOMEM;
>>>    		goto out;
>>> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
>>> index 02105d68accb..1ebfed8f0a0a 100644
>>> --- a/fs/btrfs/raid56.c
>>> +++ b/fs/btrfs/raid56.c
>>> @@ -2110,8 +2110,8 @@ static int recover_sectors(struct btrfs_raid_bio *rbio)
>>>    	 * @unmap_array stores copy of pointers that does not get reordered
>>>    	 * during reconstruction so that kunmap_local works.
>>>    	 */
>>> -	pointers = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>>> -	unmap_array = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>>> +	pointers = kzalloc_objs(*pointers, rbio->real_stripes, GFP_NOFS);
>>> +	unmap_array = kzalloc_objs(*unmap_array, rbio->real_stripes, GFP_NOFS);
>>>    	if (!pointers || !unmap_array) {
>>>    		ret = -ENOMEM;
>>>    		goto out;
>>> @@ -2844,8 +2844,8 @@ static int recover_scrub_rbio(struct btrfs_raid_bio *rbio)
>>>    	 * @unmap_array stores copy of pointers that does not get reordered
>>>    	 * during reconstruction so that kunmap_local works.
>>>    	 */
>>> -	pointers = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>>> -	unmap_array = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>>> +	pointers = kzalloc_objs(*pointers, rbio->real_stripes, GFP_NOFS);
>>> +	unmap_array = kzalloc_objs(*unmap_array, rbio->real_stripes, GFP_NOFS);
>>>    	if (!pointers || !unmap_array) {
>>>    		ret = -ENOMEM;
>>>    		goto out;
>>> diff --git a/fs/btrfs/tests/zoned-tests.c b/fs/btrfs/tests/zoned-tests.c
>>> index da21c7aea31a..2bc3b14baa41 100644
>>> --- a/fs/btrfs/tests/zoned-tests.c
>>> +++ b/fs/btrfs/tests/zoned-tests.c
>>> @@ -58,7 +58,7 @@ static int test_load_zone_info(struct btrfs_fs_info *fs_info,
>>>    		return -ENOMEM;
>>>    	}
>>>    -	zone_info = kcalloc(test->num_stripes, sizeof(*zone_info), GFP_KERNEL);
>>> +	zone_info = kzalloc_objs(*zone_info, test->num_stripes, GFP_KERNEL);
>>>    	if (!zone_info) {
>>>    		test_err("cannot allocate zone info");
>>>    		return -ENOMEM;
>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>> index e15e138c515b..c0cf8f7c5a8e 100644
>>> --- a/fs/btrfs/volumes.c
>>> +++ b/fs/btrfs/volumes.c
>>> @@ -5499,8 +5499,7 @@ static int calc_one_profile_avail(struct btrfs_fs_info *fs_info,
>>>    		goto out;
>>>    	}
>>>    -	devices_info = kcalloc(fs_devices->rw_devices, sizeof(*devices_info),
>>> -			       GFP_NOFS);
>>> +	devices_info = kzalloc_objs(*devices_info, fs_devices->rw_devices, GFP_NOFS);
>>>    	if (!devices_info) {
>>>    		ret = -ENOMEM;
>>>    		goto out;
>>> @@ -6067,8 +6066,7 @@ struct btrfs_block_group *btrfs_create_chunk(struct btrfs_trans_handle *trans,
>>>    	ctl.space_info = space_info;
>>>    	init_alloc_chunk_ctl(fs_devices, &ctl);
>>>    -	devices_info = kcalloc(fs_devices->rw_devices, sizeof(*devices_info),
>>> -			       GFP_NOFS);
>>> +	devices_info = kzalloc_objs(*devices_info, fs_devices->rw_devices, GFP_NOFS);
>>>    	if (!devices_info)
>>>    		return ERR_PTR(-ENOMEM);
>>>    diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
>>> index ab330ec957bc..851b0de7bed7 100644
>>> --- a/fs/btrfs/zoned.c
>>> +++ b/fs/btrfs/zoned.c
>>> @@ -1697,8 +1697,7 @@ static int btrfs_load_block_group_raid10(struct btrfs_block_group *bg,
>>>    		return -EINVAL;
>>>    	}
>>>    -	raid0_allocs = kcalloc(map->num_stripes / map->sub_stripes,
>>> sizeof(*raid0_allocs),
>>> -			       GFP_NOFS);
>>> +	raid0_allocs = kzalloc_objs(*raid0_allocs, map->num_stripes / map->sub_stripes, GFP_NOFS);
>>>    	if (!raid0_allocs)
>>>    		return -ENOMEM;
>>>    @@ -1916,7 +1915,7 @@ int btrfs_load_block_group_zone_info(struct
>>> btrfs_block_group *cache, bool new)
>>>      	cache->physical_map = map;
>>>    -	zone_info = kcalloc(map->num_stripes, sizeof(*zone_info), GFP_NOFS);
>>> +	zone_info = kzalloc_objs(*zone_info, map->num_stripes, GFP_NOFS);
>>>    	if (!zone_info) {
>>>    		ret = -ENOMEM;
>>>    		goto out;


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()
  2026-02-24  6:46   ` Miquel Sabaté Solà
@ 2026-02-24  6:54     ` Qu Wenruo
  2026-02-24  9:04       ` Miquel Sabaté Solà
  0 siblings, 1 reply; 18+ messages in thread
From: Qu Wenruo @ 2026-02-24  6:54 UTC (permalink / raw)
  To: Miquel Sabaté Solà, Johannes Thumshirn
  Cc: dsterba@suse.com, clm@fb.com, Naohiro Aota,
	linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org,
	kees@kernel.org



在 2026/2/24 17:16, Miquel Sabaté Solà 写道:
> Johannes Thumshirn @ 2026-02-24 06:32 GMT:
> 
>> On 2/24/26 12:45 AM, Miquel Sabaté Solà wrote:
>>> Commit 2932ba8d9c99 ("slab: Introduce kmalloc_obj() and family")
>>> introduced, among many others, the kzalloc_objs() helper, which has some
>>> benefits over kcalloc().
>> Namely?
> 
> I didn't want to repeat the arguments from the quoted commit
> 2932ba8d9c99 ("slab: Introduce kmalloc_obj() and family"). Namely:

If you assume every btrfs developer is aware of all slab/mm/vfs/whatever 
subsystem development, then I'd say you're wrong.

Thus you should mention the commit (which is not yet in our developmenet 
tree), and at least have a short reason on the benefit.

> 
>> Internal introspection of the allocated type now becomes possible,
>> allowing for future alignment-aware choices to be made by the
>> allocator and future hardening work that can be type sensitive.
> 
> Should I put this in the commit message as well, regardless of the
> commit explaining this being quoted?
> 
> There's also the argument of dropping 'sizeof' to be more ergonomic. To
> me, though, and considering how these helpers have been applied
> tree-wide, I see this change more as aligning us with this recent
> tree-wide move, which also affected btrfs (see commit 69050f8d6d07
> "treewide: Replace kmalloc with kmalloc_obj for non-scalar types").


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()
  2026-02-24  6:48     ` Qu Wenruo
@ 2026-02-24  8:59       ` Miquel Sabaté Solà
  0 siblings, 0 replies; 18+ messages in thread
From: Miquel Sabaté Solà @ 2026-02-24  8:59 UTC (permalink / raw)
  To: Qu Wenruo
  Cc: Qu Wenruo, dsterba, clm, naohiro.aota, linux-btrfs, linux-kernel,
	kees

[-- Attachment #1: Type: text/plain, Size: 7164 bytes --]

Qu Wenruo @ 2026-02-24 17:18 +1030:

> 在 2026/2/24 17:06, Miquel Sabaté Solà 写道:
>> Qu Wenruo @ 2026-02-24 15:07 +1030:
>>
>>> 在 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@kernel.org>
>>>> Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>
>>>> ---
>>>>    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.
>> Considering the type of 'buf' and how kzalloc_objs() will resolve the
>> first argument '*buf', it should really just be equivalent to what was
>> written before.
>
> Good luck if you miss the '*' for a structure pointer, and compiler won't give
> you any warning.
>
> I still remembered that Johannes exposed such bug for me.
> Unfortunately I didn't have exact lore link for it.

That's a good point. Fortunately for us, this is what I get if I remove
the star with GCC 15.2:

fs/btrfs/block-group.c:2242:13: error: assignment to 'u64 *' {aka 'long long unsigned int *'} from incompatible pointer type 'u64 **' {aka 'long long unsigned int **'} [-Wincompatible-pointer-types]
 2242 |         buf = kmalloc_objs(buf, map->num_stripes, GFP_NOFS);
      |

Hence, it looks like these kinds of errors will be catched by the
compiler with these new helpers. This doesn't mean that we are 100% safe
from corner cases, but at least there are some guarantees.

>
>>
>>>
>>> Another thing is, we may not want to use the kzalloc version.
>>> We don't want to waste CPU time just to zero out the content meanwhile we're
>>> ensured to re-assign the contents.
>>>
>>> Thus kmalloc_objs() maybe better.
>> Yes, having a second look at this function, it looks like kmalloc_objs()
>> might just be enough. If you don't mind, I will add another commit in v2
>> translating kzalloc_objs() into kmalloc_objs() wherever I see we can do
>> this from the ones I've touched. This way we can easily revert in case
>> things go south :)
>>
>>>
>>> Thanks,
>>> Qu
>> Thanks,
>> Miquel
>>
>>>
>>>
>>>>    	if (!buf) {
>>>>    		ret = -ENOMEM;
>>>>    		goto out;
>>>> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
>>>> index 02105d68accb..1ebfed8f0a0a 100644
>>>> --- a/fs/btrfs/raid56.c
>>>> +++ b/fs/btrfs/raid56.c
>>>> @@ -2110,8 +2110,8 @@ static int recover_sectors(struct btrfs_raid_bio *rbio)
>>>>    	 * @unmap_array stores copy of pointers that does not get reordered
>>>>    	 * during reconstruction so that kunmap_local works.
>>>>    	 */
>>>> -	pointers = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>>>> -	unmap_array = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>>>> +	pointers = kzalloc_objs(*pointers, rbio->real_stripes, GFP_NOFS);
>>>> +	unmap_array = kzalloc_objs(*unmap_array, rbio->real_stripes, GFP_NOFS);
>>>>    	if (!pointers || !unmap_array) {
>>>>    		ret = -ENOMEM;
>>>>    		goto out;
>>>> @@ -2844,8 +2844,8 @@ static int recover_scrub_rbio(struct btrfs_raid_bio *rbio)
>>>>    	 * @unmap_array stores copy of pointers that does not get reordered
>>>>    	 * during reconstruction so that kunmap_local works.
>>>>    	 */
>>>> -	pointers = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>>>> -	unmap_array = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>>>> +	pointers = kzalloc_objs(*pointers, rbio->real_stripes, GFP_NOFS);
>>>> +	unmap_array = kzalloc_objs(*unmap_array, rbio->real_stripes, GFP_NOFS);
>>>>    	if (!pointers || !unmap_array) {
>>>>    		ret = -ENOMEM;
>>>>    		goto out;
>>>> diff --git a/fs/btrfs/tests/zoned-tests.c b/fs/btrfs/tests/zoned-tests.c
>>>> index da21c7aea31a..2bc3b14baa41 100644
>>>> --- a/fs/btrfs/tests/zoned-tests.c
>>>> +++ b/fs/btrfs/tests/zoned-tests.c
>>>> @@ -58,7 +58,7 @@ static int test_load_zone_info(struct btrfs_fs_info *fs_info,
>>>>    		return -ENOMEM;
>>>>    	}
>>>>    -	zone_info = kcalloc(test->num_stripes, sizeof(*zone_info), GFP_KERNEL);
>>>> +	zone_info = kzalloc_objs(*zone_info, test->num_stripes, GFP_KERNEL);
>>>>    	if (!zone_info) {
>>>>    		test_err("cannot allocate zone info");
>>>>    		return -ENOMEM;
>>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>>> index e15e138c515b..c0cf8f7c5a8e 100644
>>>> --- a/fs/btrfs/volumes.c
>>>> +++ b/fs/btrfs/volumes.c
>>>> @@ -5499,8 +5499,7 @@ static int calc_one_profile_avail(struct btrfs_fs_info *fs_info,
>>>>    		goto out;
>>>>    	}
>>>>    -	devices_info = kcalloc(fs_devices->rw_devices, sizeof(*devices_info),
>>>> -			       GFP_NOFS);
>>>> +	devices_info = kzalloc_objs(*devices_info, fs_devices->rw_devices, GFP_NOFS);
>>>>    	if (!devices_info) {
>>>>    		ret = -ENOMEM;
>>>>    		goto out;
>>>> @@ -6067,8 +6066,7 @@ struct btrfs_block_group *btrfs_create_chunk(struct btrfs_trans_handle *trans,
>>>>    	ctl.space_info = space_info;
>>>>    	init_alloc_chunk_ctl(fs_devices, &ctl);
>>>>    -	devices_info = kcalloc(fs_devices->rw_devices, sizeof(*devices_info),
>>>> -			       GFP_NOFS);
>>>> +	devices_info = kzalloc_objs(*devices_info, fs_devices->rw_devices, GFP_NOFS);
>>>>    	if (!devices_info)
>>>>    		return ERR_PTR(-ENOMEM);
>>>>    diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
>>>> index ab330ec957bc..851b0de7bed7 100644
>>>> --- a/fs/btrfs/zoned.c
>>>> +++ b/fs/btrfs/zoned.c
>>>> @@ -1697,8 +1697,7 @@ static int btrfs_load_block_group_raid10(struct btrfs_block_group *bg,
>>>>    		return -EINVAL;
>>>>    	}
>>>>    -	raid0_allocs = kcalloc(map->num_stripes / map->sub_stripes,
>>>> sizeof(*raid0_allocs),
>>>> -			       GFP_NOFS);
>>>> +	raid0_allocs = kzalloc_objs(*raid0_allocs, map->num_stripes / map->sub_stripes, GFP_NOFS);
>>>>    	if (!raid0_allocs)
>>>>    		return -ENOMEM;
>>>>    @@ -1916,7 +1915,7 @@ int btrfs_load_block_group_zone_info(struct
>>>> btrfs_block_group *cache, bool new)
>>>>      	cache->physical_map = map;
>>>>    -	zone_info = kcalloc(map->num_stripes, sizeof(*zone_info), GFP_NOFS);
>>>> +	zone_info = kzalloc_objs(*zone_info, map->num_stripes, GFP_NOFS);
>>>>    	if (!zone_info) {
>>>>    		ret = -ENOMEM;
>>>>    		goto out;

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 897 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()
  2026-02-24  6:54     ` Qu Wenruo
@ 2026-02-24  9:04       ` Miquel Sabaté Solà
  0 siblings, 0 replies; 18+ messages in thread
From: Miquel Sabaté Solà @ 2026-02-24  9:04 UTC (permalink / raw)
  To: Qu Wenruo
  Cc: Johannes Thumshirn, dsterba@suse.com, clm@fb.com, Naohiro Aota,
	linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org,
	kees@kernel.org

[-- Attachment #1: Type: text/plain, Size: 1914 bytes --]

Qu Wenruo @ 2026-02-24 17:24 +1030:

> 在 2026/2/24 17:16, Miquel Sabaté Solà 写道:
>> Johannes Thumshirn @ 2026-02-24 06:32 GMT:
>>
>>> On 2/24/26 12:45 AM, Miquel Sabaté Solà wrote:
>>>> Commit 2932ba8d9c99 ("slab: Introduce kmalloc_obj() and family")
>>>> introduced, among many others, the kzalloc_objs() helper, which has some
>>>> benefits over kcalloc().
>>> Namely?
>> I didn't want to repeat the arguments from the quoted commit
>> 2932ba8d9c99 ("slab: Introduce kmalloc_obj() and family"). Namely:
>
> If you assume every btrfs developer is aware of all slab/mm/vfs/whatever
> subsystem development, then I'd say you're wrong.

To be fair, what I quoted comes from the git log for that commit. So I
was not assuming a subscription to any list or following of other
subsystems. If that's how I sounded, then I apologise.

>
> Thus you should mention the commit (which is not yet in our developmenet tree),
> and at least have a short reason on the benefit.

Regardless of the above, both you and Johannes bring a fair point. I
will expand the git message just so people don't have to lookup in other
places to get the full context of the change. I'll do that on v2.

Thanks!
Miquel

>
>>
>>> Internal introspection of the allocated type now becomes possible,
>>> allowing for future alignment-aware choices to be made by the
>>> allocator and future hardening work that can be type sensitive.
>> Should I put this in the commit message as well, regardless of the
>> commit explaining this being quoted?
>> There's also the argument of dropping 'sizeof' to be more ergonomic. To
>> me, though, and considering how these helpers have been applied
>> tree-wide, I see this change more as aligning us with this recent
>> tree-wide move, which also affected btrfs (see commit 69050f8d6d07
>> "treewide: Replace kmalloc with kmalloc_obj for non-scalar types").

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 897 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()
       [not found]   ` <699d43e6.170a0220.3a6e96.a235SMTPIN_ADDED_BROKEN@mx.google.com>
@ 2026-02-24 11:29     ` David Sterba
  2026-02-24 12:23       ` Miquel Sabaté Solà
  0 siblings, 1 reply; 18+ messages in thread
From: David Sterba @ 2026-02-24 11:29 UTC (permalink / raw)
  To: Miquel Sabaté Solà
  Cc: Kees Cook, dsterba, clm, naohiro.aota, linux-btrfs, linux-kernel

On Tue, Feb 24, 2026 at 07:23:25AM +0100, Miquel Sabaté Solà wrote:
> Kees Cook @ 2026-02-23 16:06 -08:
> 
> > On Tue, Feb 24, 2026 at 12:44:51AM +0100, Miquel Sabaté Solà wrote:
> >> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
> >> index 02105d68accb..1ebfed8f0a0a 100644
> >> --- a/fs/btrfs/raid56.c
> >> +++ b/fs/btrfs/raid56.c
> >> @@ -2110,8 +2110,8 @@ static int recover_sectors(struct btrfs_raid_bio *rbio)
> >>  	 * @unmap_array stores copy of pointers that does not get reordered
> >>  	 * during reconstruction so that kunmap_local works.
> >>  	 */
> >> -	pointers = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
> >> -	unmap_array = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
> >> +	pointers = kzalloc_objs(*pointers, rbio->real_stripes, GFP_NOFS);
> >> +	unmap_array = kzalloc_objs(*unmap_array, rbio->real_stripes, GFP_NOFS);
> >>  	if (!pointers || !unmap_array) {
> >>  		ret = -ENOMEM;
> >>  		goto out;
> >
> > Just as a style option, I wanted to point out (for at least the above,
> > I didn't check the rest), you can do the definition and declaration at
> > once with "auto" and put the type in the alloc:
> >
> > 	auto pointers = kzalloc_objs(void *, rbio->real_stripes, GFP_NOFS);
> >
> > But either way is fine. :) This patch looks good to me!
> 
> I personally don't mind either way, but I don't what's the policy around
> using "auto" in btrfs.

So far it hasn't been used and as with all the other syntax updates it's
up to debate and eventually start using it or not.  I'd need to see
examples where it's better than not using it, apart from macros.
In C the explicit types are everywhere and are I think always simple,
unlike in C++ where 'auto' can hide something very complex.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()
  2026-02-24 11:29     ` David Sterba
@ 2026-02-24 12:23       ` Miquel Sabaté Solà
  0 siblings, 0 replies; 18+ messages in thread
From: Miquel Sabaté Solà @ 2026-02-24 12:23 UTC (permalink / raw)
  To: David Sterba
  Cc: Kees Cook, dsterba, clm, naohiro.aota, linux-btrfs, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2242 bytes --]

David Sterba @ 2026-02-24 12:29 +01:

> On Tue, Feb 24, 2026 at 07:23:25AM +0100, Miquel Sabaté Solà wrote:
>> Kees Cook @ 2026-02-23 16:06 -08:
>>
>> > On Tue, Feb 24, 2026 at 12:44:51AM +0100, Miquel Sabaté Solà wrote:
>> >> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
>> >> index 02105d68accb..1ebfed8f0a0a 100644
>> >> --- a/fs/btrfs/raid56.c
>> >> +++ b/fs/btrfs/raid56.c
>> >> @@ -2110,8 +2110,8 @@ static int recover_sectors(struct btrfs_raid_bio *rbio)
>> >>  	 * @unmap_array stores copy of pointers that does not get reordered
>> >>  	 * during reconstruction so that kunmap_local works.
>> >>  	 */
>> >> -	pointers = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>> >> -	unmap_array = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
>> >> +	pointers = kzalloc_objs(*pointers, rbio->real_stripes, GFP_NOFS);
>> >> +	unmap_array = kzalloc_objs(*unmap_array, rbio->real_stripes, GFP_NOFS);
>> >>  	if (!pointers || !unmap_array) {
>> >>  		ret = -ENOMEM;
>> >>  		goto out;
>> >
>> > Just as a style option, I wanted to point out (for at least the above,
>> > I didn't check the rest), you can do the definition and declaration at
>> > once with "auto" and put the type in the alloc:
>> >
>> > 	auto pointers = kzalloc_objs(void *, rbio->real_stripes, GFP_NOFS);
>> >
>> > But either way is fine. :) This patch looks good to me!
>>
>> I personally don't mind either way, but I don't what's the policy around
>> using "auto" in btrfs.
>
> So far it hasn't been used and as with all the other syntax updates it's
> up to debate and eventually start using it or not.  I'd need to see
> examples where it's better than not using it, apart from macros.
> In C the explicit types are everywhere and are I think always simple,
> unlike in C++ where 'auto' can hide something very complex.

In this case, I'd say we can skip the use of 'auto', at least for these
patches. Using it wouldn't help much, and it's more coherent with the
rest of the codebase to stick with explicit typing.

Also, using 'auto' in this case would mean to remove the declaration
from the top of the function, which would break the style for this and
many other functions from the btrfs code.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 897 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()
  2026-02-24  4:42   ` Qu Wenruo
@ 2026-02-24 13:52     ` David Sterba
  0 siblings, 0 replies; 18+ messages in thread
From: David Sterba @ 2026-02-24 13:52 UTC (permalink / raw)
  To: Qu Wenruo
  Cc: Miquel Sabaté Solà, dsterba, clm, naohiro.aota,
	linux-btrfs, linux-kernel, kees

On Tue, Feb 24, 2026 at 03:12:07PM +1030, Qu Wenruo wrote:
> 在 2026/2/24 15:07, Qu Wenruo 写道:
> > 在 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@kernel.org>
> >> Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>
> >> ---
> >>   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.
> > 
> > Another thing is, we may not want to use the kzalloc version.
> > We don't want to waste CPU time just to zero out the content meanwhile 
> > we're ensured to re-assign the contents.
> > 
> > Thus kmalloc_objs() maybe better.
> 
> Sorry I only mean for some particular call sites, like this one.
> 
> Not all call sites can be converted to kmalloc version, and will need 
> proper inspection one by one.

I'd rather keep the zeroing version, we do it almost everywhere as a
precaution. The CPU time spent on that is not significant.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()
  2026-02-24  4:37 ` Qu Wenruo
                     ` (2 preceding siblings ...)
       [not found]   ` <699d4704.050a0220.1a6450.86d7SMTPIN_ADDED_BROKEN@mx.google.com>
@ 2026-02-24 14:55   ` David Laight
  2026-02-25 14:44     ` David Sterba
  3 siblings, 1 reply; 18+ messages in thread
From: David Laight @ 2026-02-24 14:55 UTC (permalink / raw)
  To: Qu Wenruo
  Cc: Miquel Sabaté Solà, dsterba, clm, naohiro.aota,
	linux-btrfs, linux-kernel, kees

On Tue, 24 Feb 2026 15:07:10 +1030
Qu Wenruo <quwenruo.btrfs@gmx.com> 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@kernel.org>
> > Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>
> > ---
> >   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.

	David



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()
  2026-02-24 14:55   ` David Laight
@ 2026-02-25 14:44     ` David Sterba
  2026-02-25 17:11       ` David Laight
  0 siblings, 1 reply; 18+ messages in thread
From: David Sterba @ 2026-02-25 14:44 UTC (permalink / raw)
  To: David Laight
  Cc: Qu Wenruo, Miquel Sabaté Solà, dsterba, clm,
	naohiro.aota, linux-btrfs, linux-kernel, kees

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@gmx.com> 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@kernel.org>
> > > Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>
> > > ---
> > >   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.

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs()
  2026-02-25 14:44     ` David Sterba
@ 2026-02-25 17:11       ` David Laight
  0 siblings, 0 replies; 18+ messages in thread
From: David Laight @ 2026-02-25 17:11 UTC (permalink / raw)
  To: David Sterba
  Cc: Qu Wenruo, Miquel Sabaté Solà, dsterba, clm,
	naohiro.aota, linux-btrfs, linux-kernel, kees

On Wed, 25 Feb 2026 15:44:46 +0100
David Sterba <dsterba@suse.cz> 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@gmx.com> 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@kernel.org>
> > > > Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>
> > > > ---
> > > >   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.


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2026-02-25 17:11 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-23 23:44 [PATCH] btrfs: replace kcalloc() calls to kzalloc_objs() Miquel Sabaté Solà
2026-02-24  0:06 ` Kees Cook
2026-02-24  6:23   ` Miquel Sabaté Solà
     [not found]   ` <699d43e6.170a0220.3a6e96.a235SMTPIN_ADDED_BROKEN@mx.google.com>
2026-02-24 11:29     ` David Sterba
2026-02-24 12:23       ` Miquel Sabaté Solà
2026-02-24  4:37 ` Qu Wenruo
2026-02-24  4:42   ` Qu Wenruo
2026-02-24 13:52     ` David Sterba
2026-02-24  6:36   ` Miquel Sabaté Solà
     [not found]   ` <699d4704.050a0220.1a6450.86d7SMTPIN_ADDED_BROKEN@mx.google.com>
2026-02-24  6:48     ` Qu Wenruo
2026-02-24  8:59       ` Miquel Sabaté Solà
2026-02-24 14:55   ` David Laight
2026-02-25 14:44     ` David Sterba
2026-02-25 17:11       ` David Laight
2026-02-24  6:32 ` Johannes Thumshirn
2026-02-24  6:46   ` Miquel Sabaté Solà
2026-02-24  6:54     ` Qu Wenruo
2026-02-24  9:04       ` Miquel Sabaté Solà

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox