[PATCH] Btrfs: fix allocation size calculations in alloc_btrfs_bio

[PATCH] Btrfs: fix allocation size calculations in alloc_btrfs_bio

[Chris Mason]

Since commit 8e5cfb55d3f (Btrfs: Make raid_map array be inlined in
btrfs_bio structure), the raid map array is allocated along with the
btrfs bio in alloc_btrfs_bio.  The calculation used to decide how much
we need to allocate was using the wrong parameter passed into the
allocation function.

The passed in real_stripes will be zero if a target replace operation
is not currently running.  We want to use total_stripes instead.

Signed-off-by: Chris Mason <clm@fb.com>
Reported-by: Dave Sterba <dsterba@suse.cz>

---
 fs/btrfs/volumes.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index cd4d131..8222f6f 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4903,10 +4903,17 @@ static void sort_parity_stripes(struct btrfs_bio *bbio, int num_stripes)
 static struct btrfs_bio *alloc_btrfs_bio(int total_stripes, int real_stripes)
 {
 	struct btrfs_bio *bbio = kzalloc(
+		 /* the size of the btrfs_bio */
 		sizeof(struct btrfs_bio) +
+		/* plus the variable array for the stripes */
 		sizeof(struct btrfs_bio_stripe) * (total_stripes) +
+		/* plus the variable array for the tgt dev */
 		sizeof(int) * (real_stripes) +
-		sizeof(u64) * (real_stripes),
+		/*
+		 * plus the raid_map, which includes both the tgt dev
+		 * and the stripes
+		 */
+		sizeof(u64) * (total_stripes),
 		GFP_NOFS);
 	if (!bbio)
 		return NULL;
-- 
1.8.1

Re: [PATCH] Btrfs: fix allocation size calculations in alloc_btrfs_bio

[David Sterba]

On Thu, Feb 19, 2015 at 08:59:30PM -0500, Chris Mason wrote:
> Since commit 8e5cfb55d3f (Btrfs: Make raid_map array be inlined in
> btrfs_bio structure), the raid map array is allocated along with the
> btrfs bio in alloc_btrfs_bio.  The calculation used to decide how much
> we need to allocate was using the wrong parameter passed into the
> allocation function.
> 
> The passed in real_stripes will be zero if a target replace operation
> is not currently running.  We want to use total_stripes instead.
> 
> Signed-off-by: Chris Mason <clm@fb.com>
> Reported-by: Dave Sterba <dsterba@suse.cz>

Tested-by: David Sterba <dsterba@suse.cz>

Current master + this patch with full slub_debug is now ok with
btrfs/023.

Re: [PATCH] Btrfs: fix allocation size calculations in alloc_btrfs_bio

[Chris Mason]

On Fri, Feb 20, 2015 at 9:35 AM, David Sterba <dsterba@suse.cz> wrote:
> On Thu, Feb 19, 2015 at 08:59:30PM -0500, Chris Mason wrote:
>>  Since commit 8e5cfb55d3f (Btrfs: Make raid_map array be inlined in
>>  btrfs_bio structure), the raid map array is allocated along with the
>>  btrfs bio in alloc_btrfs_bio.  The calculation used to decide how 
>> much
>>  we need to allocate was using the wrong parameter passed into the
>>  allocation function.
>> 
>>  The passed in real_stripes will be zero if a target replace 
>> operation
>>  is not currently running.  We want to use total_stripes instead.
>> 
>>  Signed-off-by: Chris Mason <clm@fb.com>
>>  Reported-by: Dave Sterba <dsterba@suse.cz>
> 
> Tested-by: David Sterba <dsterba@suse.cz>
> 
> Current master + this patch with full slub_debug is now ok with
> btrfs/023.

Thanks Dave.  It also survived all night with raid6 and stress.sh.  I'm 
sending to Linus today.

-chris

RE: [PATCH] Btrfs: fix allocation size calculations in alloc_btrfs_bio

[Zhao Lei]

Hi, Chris Mason

* From: Chris Mason [mailto:clm@fb.com]
> Sent: Friday, February 20, 2015 10:00 AM
> To: linux-btrfs; Zhao Lei; dsterba@suse.cz
> Subject: [PATCH] Btrfs: fix allocation size calculations in alloc_btrfs_bio
> 
> Since commit 8e5cfb55d3f (Btrfs: Make raid_map array be inlined in btrfs_bio
> structure), the raid map array is allocated along with the btrfs bio in
> alloc_btrfs_bio.  The calculation used to decide how much we need to allocate
> was using the wrong parameter passed into the allocation function.
> 
> The passed in real_stripes will be zero if a target replace operation is not
> currently running.  We want to use total_stripes instead.
> 
Sorry, my bad.

Thanks
Zhaolei

> Signed-off-by: Chris Mason <clm@fb.com>
> Reported-by: Dave Sterba <dsterba@suse.cz>
> 
> ---
>  fs/btrfs/volumes.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index cd4d131..8222f6f
> 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -4903,10 +4903,17 @@ static void sort_parity_stripes(struct btrfs_bio
> *bbio, int num_stripes)  static struct btrfs_bio *alloc_btrfs_bio(int
> total_stripes, int real_stripes)  {
>  	struct btrfs_bio *bbio = kzalloc(
> +		 /* the size of the btrfs_bio */
>  		sizeof(struct btrfs_bio) +
> +		/* plus the variable array for the stripes */
>  		sizeof(struct btrfs_bio_stripe) * (total_stripes) +
> +		/* plus the variable array for the tgt dev */
>  		sizeof(int) * (real_stripes) +
> -		sizeof(u64) * (real_stripes),
> +		/*
> +		 * plus the raid_map, which includes both the tgt dev
> +		 * and the stripes
> +		 */
> +		sizeof(u64) * (total_stripes),
>  		GFP_NOFS);
>  	if (!bbio)
>  		return NULL;
> --
> 1.8.1