Messages in this thread Patch in this message | | | Subject | Re: [PATCH] md/raid5: Convert stripe_head's "dev" to flexible array member | From | Yu Kuai <> | Date | Mon, 22 May 2023 19:49:38 +0800 |
| |
Hi, Christoph
在 2023/05/18 12:27, Christoph Hellwig 写道:
It's not related to this patch, just I think I found a problem while reviewing raid5 code, commit e82ed3a4fbb5 ("md/raid6: refactor raid5_read_one_chunk") changes the caculation of 'end_sector', 'end_sector' is compared to 'rdev->recovery_offset', so it should be offset to rdev, but this commit change it to offset to the array.
Perhaps following change will make sense:
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c index 7e2bbcfef325..8686d629e3f2 100644 --- a/drivers/md/raid5.c +++ b/drivers/md/raid5.c @@ -5516,7 +5516,7 @@ static int raid5_read_one_chunk(struct mddev *mddev, struct bio *raid_bio)
sector = raid5_compute_sector(conf, raid_bio->bi_iter.bi_sector, 0, &dd_idx, NULL); - end_sector = bio_end_sector(raid_bio); + end_sector = sector + bio_sectors(raid_bio);
rcu_read_lock(); if (r5c_big_stripe_cached(conf, sector))
Thanks, Kuai > On Wed, May 17, 2023 at 04:33:14PM -0700, Kees Cook wrote: >> sc = kmem_cache_create(conf->cache_name[conf->active_name], >> - sizeof(struct stripe_head)+(devs-1)*sizeof(struct r5dev), >> + struct_size((struct stripe_head *)0, dev, devs), >> 0, 0, NULL); >> if (!sc) >> return 1; >> @@ -2559,7 +2559,7 @@ static int resize_stripes(struct r5conf *conf, int newsize) >> >> /* Step 1 */ >> sc = kmem_cache_create(conf->cache_name[1-conf->active_name], >> - sizeof(struct stripe_head)+(newsize-1)*sizeof(struct r5dev), >> + struct_size((struct stripe_head *)0, dev, newsize), > > The constant you're casting here should be NULL, not 0. > Also given that this expression is duplicated, I'd suggest a little > helper for it… > >> - } dev[1]; /* allocated with extra space depending of RAID geometry */ >> + } dev[]; /* allocated with extra space depending of RAID geometry */ > > And this isn't extra space over the single entry anymore, so I'd > change this to: > > /* allocated depending of RAID geometry */ > . >
| |