| Message ID | a2f049f315b3c218d909c854ab117779d8842abe.1648807440.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: add subpage support for RAID56 | expand |
Hi Qu, On Fri, 1 Apr 2022, Qu Wenruo wrote: > The new members are all related to number of sectors, but the existing > number of pages members are kept as is: > > - nr_sectors > Total sectors of the full stripe including P/Q. > > - stripe_nsectors > The sectors of a single stripe. > > Signed-off-by: Qu Wenruo <wqu@suse.com> Thanks for your patch, which is now commit f1e779cdb7f165f0 ("btrfs: raid56: introduce new cached members for btrfs_raid_bio") in next-20220411. > --- a/fs/btrfs/raid56.c > +++ b/fs/btrfs/raid56.c > @@ -958,18 +964,25 @@ static struct btrfs_raid_bio *alloc_rbio(struct btrfs_fs_info *fs_info, > const unsigned int real_stripes = bioc->num_stripes - bioc->num_tgtdevs; > const unsigned int num_pages = DIV_ROUND_UP(stripe_len, PAGE_SIZE) * > real_stripes; > + const unsigned int num_sectors = DIV_ROUND_UP(stripe_len * real_stripes, > + fs_info->sectorsize); noreply@ellerman.id.au reports for m68k builds: ERROR: modpost: "__udivdi3" [fs/btrfs/btrfs.ko] undefined! As this is a 64-by-32 division, you should not use a plain division, but e.g. a shift by fs_info->sectorsize_bits. > const unsigned int stripe_npages = DIV_ROUND_UP(stripe_len, PAGE_SIZE); > + const unsigned int stripe_nsectors = DIV_ROUND_UP(stripe_len, > + fs_info->sectorsize); Likewise. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 2022/4/11 18:29, Geert Uytterhoeven wrote: > Hi Qu, > > On Fri, 1 Apr 2022, Qu Wenruo wrote: >> The new members are all related to number of sectors, but the existing >> number of pages members are kept as is: >> >> - nr_sectors >> Total sectors of the full stripe including P/Q. >> >> - stripe_nsectors >> The sectors of a single stripe. >> >> Signed-off-by: Qu Wenruo <wqu@suse.com> > > Thanks for your patch, which is now commit f1e779cdb7f165f0 > ("btrfs: raid56: introduce new cached members for btrfs_raid_bio") in > next-20220411. > >> --- a/fs/btrfs/raid56.c >> +++ b/fs/btrfs/raid56.c >> @@ -958,18 +964,25 @@ static struct btrfs_raid_bio *alloc_rbio(struct >> btrfs_fs_info *fs_info, >> const unsigned int real_stripes = bioc->num_stripes - >> bioc->num_tgtdevs; >> const unsigned int num_pages = DIV_ROUND_UP(stripe_len, PAGE_SIZE) * >> real_stripes; >> + const unsigned int num_sectors = DIV_ROUND_UP(stripe_len * >> real_stripes, >> + fs_info->sectorsize); > > noreply@ellerman.id.au reports for m68k builds: > > ERROR: modpost: "__udivdi3" [fs/btrfs/btrfs.ko] undefined! > > As this is a 64-by-32 division, you should not use a plain division, > but e.g. a shift by fs_info->sectorsize_bits. > >> const unsigned int stripe_npages = DIV_ROUND_UP(stripe_len, >> PAGE_SIZE); >> + const unsigned int stripe_nsectors = DIV_ROUND_UP(stripe_len, >> + fs_info->sectorsize); All my bad, in fact when I crafting this exact line, the 64-diveded-by-32 problem is already in my mind. But my initial assumption that stripe_len should be u32, prevents me to do a proper check. In fact, stripe_len should never be u64. U32 is definitely enough, and we should avoid abusing u64 for those members. I'll update the patchset, with one new patch to address the abuse of u64 specifically. Thank you very much for pointing this out, Qu > > Likewise. > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. > But > when I'm talking to journalists I just say "programmer" or something > like that. > -- Linus Torvalds
diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c index 2553e1bb8bbf..1bad7d3a0331 100644 --- a/fs/btrfs/raid56.c +++ b/fs/btrfs/raid56.c @@ -115,6 +115,9 @@ struct btrfs_raid_bio { /* How many pages there are for the full stripe including P/Q */ u16 nr_pages; + /* How many sectors there are for the full stripe including P/Q */ + u16 nr_sectors; + /* Number of data stripes (no p/q) */ u8 nr_data; @@ -124,6 +127,9 @@ struct btrfs_raid_bio { /* How many pages there are for each stripe */ u8 stripe_npages; + /* How many sectors there are for each stripe */ + u8 stripe_nsectors; + /* First bad stripe, -1 means no corruption */ s8 faila; @@ -172,7 +178,7 @@ struct btrfs_raid_bio { /* allocated with real_stripes-many pointers for finish_*() calls */ void **finish_pointers; - /* allocated with stripe_npages-many bits for finish_*() calls */ + /* allocated with stripe_nsectors-many bits for finish_*() calls */ unsigned long *finish_pbitmap; }; @@ -958,18 +964,25 @@ static struct btrfs_raid_bio *alloc_rbio(struct btrfs_fs_info *fs_info, const unsigned int real_stripes = bioc->num_stripes - bioc->num_tgtdevs; const unsigned int num_pages = DIV_ROUND_UP(stripe_len, PAGE_SIZE) * real_stripes; + const unsigned int num_sectors = DIV_ROUND_UP(stripe_len * real_stripes, + fs_info->sectorsize); const unsigned int stripe_npages = DIV_ROUND_UP(stripe_len, PAGE_SIZE); + const unsigned int stripe_nsectors = DIV_ROUND_UP(stripe_len, + fs_info->sectorsize); struct btrfs_raid_bio *rbio; int nr_data = 0; void *p; + /* PAGE_SIZE must tbe aligned to sectorsize for subpage support */ + ASSERT(IS_ALIGNED(PAGE_SIZE, fs_info->sectorsize)); + rbio = kzalloc(sizeof(*rbio) + sizeof(*rbio->stripe_pages) * num_pages + sizeof(*rbio->bio_pages) * num_pages + sizeof(*rbio->finish_pointers) * real_stripes + - sizeof(*rbio->dbitmap) * BITS_TO_LONGS(stripe_npages) + + sizeof(*rbio->dbitmap) * BITS_TO_LONGS(stripe_nsectors) + sizeof(*rbio->finish_pbitmap) * - BITS_TO_LONGS(stripe_npages), + BITS_TO_LONGS(stripe_nsectors), GFP_NOFS); if (!rbio) return ERR_PTR(-ENOMEM); @@ -982,8 +995,10 @@ static struct btrfs_raid_bio *alloc_rbio(struct btrfs_fs_info *fs_info, rbio->bioc = bioc; rbio->stripe_len = stripe_len; rbio->nr_pages = num_pages; + rbio->nr_sectors = num_sectors; rbio->real_stripes = real_stripes; rbio->stripe_npages = stripe_npages; + rbio->stripe_nsectors = stripe_nsectors; rbio->faila = -1; rbio->failb = -1; refcount_set(&rbio->refs, 1); @@ -1002,8 +1017,8 @@ static struct btrfs_raid_bio *alloc_rbio(struct btrfs_fs_info *fs_info, CONSUME_ALLOC(rbio->stripe_pages, num_pages); CONSUME_ALLOC(rbio->bio_pages, num_pages); CONSUME_ALLOC(rbio->finish_pointers, real_stripes); - CONSUME_ALLOC(rbio->dbitmap, BITS_TO_LONGS(stripe_npages)); - CONSUME_ALLOC(rbio->finish_pbitmap, BITS_TO_LONGS(stripe_npages)); + CONSUME_ALLOC(rbio->dbitmap, BITS_TO_LONGS(stripe_nsectors)); + CONSUME_ALLOC(rbio->finish_pbitmap, BITS_TO_LONGS(stripe_nsectors)); #undef CONSUME_ALLOC if (bioc->map_type & BTRFS_BLOCK_GROUP_RAID5)
The new members are all related to number of sectors, but the existing number of pages members are kept as is: - nr_sectors Total sectors of the full stripe including P/Q. - stripe_nsectors The sectors of a single stripe. Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/raid56.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-)