| Message ID | 20150220015930.GA28726@ret.masoncoding.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
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. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;
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(-)