enable bs > ps for block devices
Subsystems (4)
cover enable bs > ps for block devices
From: Hannes Reinecke <[email protected]>
Hi all,
based on the patch series from Pankaj '[PATCH v5 00/11] enable bs > ps in XFS' it's now quite simple to enable support for block devices with block sizes larger than page size even without having to disable CONFIG_BUFFER_HEAD. The patchset really is just two rather trivial patches to fs/mpage, and two patches to remove hardcoded restrictions on the block size.
As usual, comments and reviews are welcome.
Hannes Reinecke (4):
fs/mpage: use blocks_per_folio instead of blocks_per_page fs/mpage: avoid negative shift for large blocksize block/bdev: lift restrictions on supported blocksize block/bdev: enable large folio support for large logical block sizes
Pankaj Raghav (1):
nvme: enable logical block size > PAGE_SIZE
block/bdev.c | 10 ++++++--- drivers/nvme/host/core.c | 7 +++---- fs/mpage.c | 44 ++++++++++++++++++++-------------------- 3 files changed, 32 insertions(+), 29 deletions(-)
Signature · 1 line
2.35.3
PATCH 1/5 fs/mpage: use blocks_per_folio instead of blocks_per_page
From: Hannes Reinecke <[email protected]>
Convert mpage to folios and associate the number of blocks with a folio and not a page.
--- fs/mpage.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-)
fs/mpage.c +20 -20
@@ -114,7 +114,7 @@ static void map_buffer_to_folio(struct folio *folio, struct buffer_head *bh,* don't make any buffers if there is only one buffer on * the folio and the folio just needs to be set up to date */ -if (inode->i_blkbits ==PAGE_SHIFT && +if (inode->i_blkbits ==folio_shift (folio) &&buffer_uptodate (bh)) {folio_mark_uptodate (folio);return ;
@@ -160,7 +160,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)struct folio *folio = args->folio;struct inode *inode = folio->mapping->host;const unsigned blkbits = inode->i_blkbits; -const unsigned blocks_per_page =PAGE_SIZE >> blkbits; +const unsigned blocks_per_folio =folio_size (folio) >> blkbits;const unsigned blocksize =1 << blkbits;struct buffer_head *map_bh = &args->map_bh;sector_t block_in_file;
@@ -168,7 +168,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)sector_t last_block_in_file;sector_t first_block;unsigned page_block; -unsigned first_hole = blocks_per_page; +unsigned first_hole = blocks_per_folio;struct block_device *bdev =NULL ;int length;int fully_mapped =1 ;
@@ -189,7 +189,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) goto confused; block_in_file = (sector_t )folio->index << (PAGE_SHIFT - blkbits); - last_block = block_in_file + args->nr_pages * blocks_per_page; + last_block = block_in_file + args->nr_pages * blocks_per_folio; last_block_in_file = (i_size_read (inode) + blocksize -1 ) >> blkbits;if (last_block > last_block_in_file) last_block = last_block_in_file;
@@ -211,7 +211,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)clear_buffer_mapped (map_bh);break ; } -if (page_block == blocks_per_page) +if (page_block == blocks_per_folio)break ; page_block++; block_in_file++;
@@ -223,7 +223,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)* Then do more get_blocks calls until we are done with this folio. */ map_bh->b_folio = folio; -while (page_block < blocks_per_page) { +while (page_block < blocks_per_folio) { map_bh->b_state =0 ; map_bh->b_size =0 ;
@@ -236,7 +236,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)if (!buffer_mapped (map_bh)) { fully_mapped =0 ; -if (first_hole == blocks_per_page) +if (first_hole == blocks_per_folio) first_hole = page_block; page_block++; block_in_file++;
@@ -254,7 +254,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) goto confused; } -if (first_hole != blocks_per_page) +if (first_hole != blocks_per_folio) goto confused;/* hole -> non-hole */ /* Contiguous blocks? */
@@ -267,7 +267,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)if (relative_block == nblocks) {clear_buffer_mapped (map_bh);break ; - }else if (page_block == blocks_per_page) + }else if (page_block == blocks_per_folio)break ; page_block++; block_in_file++;
@@ -275,7 +275,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) bdev = map_bh->b_bdev; } -if (first_hole != blocks_per_page) { +if (first_hole != blocks_per_folio) {folio_zero_segment (folio, first_hole << blkbits,PAGE_SIZE );if (first_hole ==0 ) {folio_mark_uptodate (folio);
@@ -310,10 +310,10 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) relative_block = block_in_file - args->first_logical_block; nblocks = map_bh->b_size >> blkbits;if ((buffer_boundary (map_bh) && relative_block == nblocks) || - (first_hole != blocks_per_page)) + (first_hole != blocks_per_folio)) args->bio =mpage_bio_submit_read (args->bio);else - args->last_block_in_bio = first_block + blocks_per_page -1 ; + args->last_block_in_bio = first_block + blocks_per_folio -1 ; out:return args->bio;
@@ -392,7 +392,7 @@ int mpage_read_folio(struct folio *folio, get_block_t get_block) {struct mpage_readpage_args args = { .folio = folio, - .nr_pages =1 , + .nr_pages =folio_nr_pages (folio), .get_block = get_block, };
@@ -463,12 +463,12 @@ static int __mpage_writepage(struct folio *folio, struct writeback_control *wbc,struct address_space *mapping = folio->mapping;struct inode *inode = mapping->host;const unsigned blkbits = inode->i_blkbits; -const unsigned blocks_per_page =PAGE_SIZE >> blkbits; +const unsigned blocks_per_folio =folio_size (folio) >> blkbits;sector_t last_block;sector_t block_in_file;sector_t first_block;unsigned page_block; -unsigned first_unmapped = blocks_per_page; +unsigned first_unmapped = blocks_per_folio;struct block_device *bdev =NULL ;int boundary =0 ;sector_t boundary_block =0 ;
@@ -493,12 +493,12 @@ static int __mpage_writepage(struct folio *folio, struct writeback_control *wbc,*/ if (buffer_dirty (bh)) goto confused; -if (first_unmapped == blocks_per_page) +if (first_unmapped == blocks_per_folio) first_unmapped = page_block;continue ; } -if (first_unmapped != blocks_per_page) +if (first_unmapped != blocks_per_folio) goto confused;/* hole -> non-hole */ if (!buffer_dirty (bh) || !buffer_uptodate (bh))
@@ -543,7 +543,7 @@ static int __mpage_writepage(struct folio *folio, struct writeback_control *wbc, goto page_is_mapped; last_block = (i_size -1 ) >> blkbits; map_bh.b_folio = folio; -for (page_block =0 ; page_block < blocks_per_page; ) { +for (page_block =0 ; page_block < blocks_per_folio; ) { map_bh.b_state =0 ; map_bh.b_size =1 << blkbits;
@@ -625,14 +625,14 @@ static int __mpage_writepage(struct folio *folio, struct writeback_control *wbc,BUG_ON (folio_test_writeback (folio));folio_start_writeback (folio);folio_unlock (folio); -if (boundary || (first_unmapped != blocks_per_page)) { +if (boundary || (first_unmapped != blocks_per_folio)) { bio =mpage_bio_submit_write (bio);if (boundary_block) {write_boundary_block (boundary_bdev, boundary_block,1 << blkbits); } }else { - mpd->last_block_in_bio = first_block + blocks_per_page -1 ; + mpd->last_block_in_bio = first_block + blocks_per_folio -1 ; } goto out;
Signature · 1 line
2.35.3
Re: [PATCH 1/5] fs/mpage: use blocks_per_folio instead of blocks_per_page
fs/mpage.c static void map_buffer_to_folio(struct folio *folio, struct buffer_head *bh, · line 117
Quoted 7 lines · On Fri, May 10, 2024 at 12:29:02PM +0200, [email protected] wrote:
+++
b/fs/mpage.cfs/mpage.c +1
-1@@ -114,7 +114,7 @@ static void map_buffer_to_folio(struct folio *folio, struct buffer_head *bh,* don't make any buffers if there is only one buffer on * the folio and the folio just needs to be set up to date */ - if (inode->i_blkbits == PAGE_SHIFT && + if (inode->i_blkbits == folio_shift(folio) &&
yes
Quoted 10 lines
fs/mpage.c +1
-1@@ -160,7 +160,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)struct folio *folio = args->folio;struct inode *inode = folio->mapping->host;const unsigned blkbits = inode->i_blkbits; -const unsigned blocks_per_page =PAGE_SIZE >> blkbits; +const unsigned blocks_per_folio =folio_size (folio) >> blkbits;const unsigned blocksize =1 << blkbits;struct buffer_head *map_bh = &args->map_bh;sector_t block_in_file;
fs/mpage.c static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) · line 171
Quoted 5 lines
fs/mpage.c +1
-1@@ -168,7 +168,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)sector_t last_block_in_file;sector_t first_block;unsigned page_block; -unsigned first_hole = blocks_per_page; +unsigned first_hole = blocks_per_folio;
yes
fs/mpage.c static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) · line 192
Quoted 6 lines
fs/mpage.c +1
-1@@ -189,7 +189,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) goto confused; block_in_file = (sector_t )folio->index << (PAGE_SHIFT - blkbits); - last_block = block_in_file + args->nr_pages * blocks_per_page; + last_block = block_in_file + args->nr_pages * blocks_per_folio;
no. args->nr_pages really is the number of pages, so last_block is block_in_file + nr_pages * blocks_per_page. except that blocks_per_page might now be 0.
so i think this needs to be rewritten as:
last_block = block_in_file + (args->nr_pages * PAGE_SIZE) >> blkbits;
or have i confused myself?
fs/mpage.c static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) · line 279
Quoted 7 lines
fs/mpage.c +1
-1@@ -275,7 +275,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) bdev = map_bh->b_bdev; } -if (first_hole != blocks_per_page) { +if (first_hole != blocks_per_folio) {folio_zero_segment (folio, first_hole << blkbits,PAGE_SIZE );
... doesn't that need to be folio_size(folio)?
there may be other problems, but let's settle these questions first.
Re: [PATCH 1/5] fs/mpage: use blocks_per_folio instead of blocks_per_page
Quoted 40 lines · Matthew Wilcox
fs/mpage.c static void map_buffer_to_folio(struct folio *folio, struct buffer_head *bh, · line 117
Quoted 7 lines · On Fri, May 10, 2024 at 12:29:02PM +0200, [email protected] wrote:
+++
b/fs/mpage.cfs/mpage.c +1
-1@@ -114,7 +114,7 @@ static void map_buffer_to_folio(struct folio *folio, struct buffer_head *bh,* don't make any buffers if there is only one buffer on * the folio and the folio just needs to be set up to date */ - if (inode->i_blkbits == PAGE_SHIFT && + if (inode->i_blkbits == folio_shift(folio) &&yes
Quoted 10 lines
fs/mpage.c +1
-1@@ -160,7 +160,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)struct folio *folio = args->folio;struct inode *inode = folio->mapping->host;const unsigned blkbits = inode->i_blkbits; -const unsigned blocks_per_page =PAGE_SIZE >> blkbits; +const unsigned blocks_per_folio =folio_size (folio) >> blkbits;const unsigned blocksize =1 << blkbits;struct buffer_head *map_bh = &args->map_bh;sector_t block_in_file;fs/mpage.c static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) · line 171
Quoted 5 lines
fs/mpage.c +1
-1@@ -168,7 +168,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)sector_t last_block_in_file;sector_t first_block;unsigned page_block; -unsigned first_hole = blocks_per_page; +unsigned first_hole = blocks_per_folio;yes
fs/mpage.c static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) · line 192
Quoted 6 lines
fs/mpage.c +1
-1@@ -189,7 +189,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) goto confused; block_in_file = (sector_t )folio->index << (PAGE_SHIFT - blkbits); - last_block = block_in_file + args->nr_pages * blocks_per_page; + last_block = block_in_file + args->nr_pages * blocks_per_folio;no.
args->nr_pagesreally is the number of pages, solast_blockisblock_in_file+nr_pages*blocks_per_page. except thatblocks_per_pagemight now be 0.
Quoted 6 lines
so i think this needs to be rewritten as:
last_block = block_in_file + (args->nr_pages * PAGE_SIZE) >> blkbits;or have i confused myself?
Aw. No. MIssed that line.
Will be fixing it up.
Quoted 4 lines
Quoted 4 lines
fs/mpage.c +0
-0@@ -275,7 +275,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) bdev = map_bh->b_bdev; }
Quoted 8 lines
fs/mpage.c static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) · line 279
Quoted 3 lines
fs/mpage.c +1
-1-if (first_hole != blocks_per_page) { +if (first_hole != blocks_per_folio) {folio_zero_segment (folio, first_hole << blkbits,PAGE_SIZE );... doesn't that need to be
folio_size(folio)?there may be other problems, but let's settle these questions first.
Thanks for the review.
I'll be sure to pester you next week :-)
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
PATCH 2/5 fs/mpage: avoid negative shift for large blocksize
In reply to [email protected]From: Hannes Reinecke <[email protected]>
For large blocksizes the number of block bits is larger than PAGE_SHIFT, so use shift to calculate the sector number from the page cache index.
--- fs/mpage.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
fs/mpage.c +2 -2
@@ -188,7 +188,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)if (folio_buffers (folio)) goto confused; - block_in_file = (sector_t )folio->index << (PAGE_SHIFT - blkbits); + block_in_file = (sector_t)(((loff_t )folio->index <<PAGE_SHIFT ) >> blkbits); last_block = block_in_file + args->nr_pages * blocks_per_folio; last_block_in_file = (i_size_read (inode) + blocksize -1 ) >> blkbits;if (last_block > last_block_in_file)
@@ -534,7 +534,7 @@ static int __mpage_writepage(struct folio *folio, struct writeback_control *wbc,* The page has no buffers: map it to disk */ BUG_ON (!folio_test_uptodate (folio)); - block_in_file = (sector_t )folio->index << (PAGE_SHIFT - blkbits); + block_in_file = (sector_t)(((loff_t )folio->index <<PAGE_SHIFT ) >> blkbits);/* * Whole page beyond EOF? Skip allocating blocks to avoid leaking * space.
Signature · 1 line
2.35.3
Re: [PATCH 2/5] fs/mpage: avoid negative shift for large blocksize
Review
Quoted 2 lines · On Fri, May 10, 2024 at 12:29:03PM +0200, [email protected] wrote:
(quoted diff) +1
-1- block_in_file = (sector_t)folio->index << (PAGE_SHIFT - blkbits); + block_in_file = (sector_t)(((loff_t)folio->index << PAGE_SHIFT) >> blkbits);
block_in_file = folio_pos(folio) >> blkbits;
Re: [PATCH 2/5] fs/mpage: avoid negative shift for large blocksize
Quoted 7 lines · Matthew Wilcox
Review
Quoted 2 lines · On Fri, May 10, 2024 at 12:29:03PM +0200, [email protected] wrote:
(quoted diff) +1
-1- block_in_file = (sector_t)folio->index << (PAGE_SHIFT - blkbits); + block_in_file = (sector_t)(((loff_t)folio->index << PAGE_SHIFT) >> blkbits);block_in_file = folio_pos(folio) >> blkbits;
I know there was a better way. Will be fixing it up.
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
PATCH 3/5 block/bdev: lift restrictions on supported blocksize
In reply to [email protected]From: Hannes Reinecke <[email protected]>
We now can support blocksizes larger than PAGE_SIZE, so lift the restriction.
--- block/bdev.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
block/bdev.c +3 -3
@@ -146,8 +146,8 @@ static void set_init_blocksize(struct block_device *bdev)int set_blocksize (struct block_device *bdev,int size) { -/* Size must be a power of two, and between 512 and PAGE_SIZE */ -if (size >PAGE_SIZE || size <512 || !is_power_of_2 (size)) +/* Size must be a power of two, and larger than 512 */ +if (size <512 || !is_power_of_2 (size))return -EINVAL ;/* Size cannot be smaller than the size supported by the device */
@@ -170,7 +170,7 @@ int sb_set_blocksize(struct super_block *sb, int size)if (set_blocksize (sb->s_bdev, size))return 0 ;/* If we get here, we know size is power of two -* and it's value is between 512 and PAGE_SIZE */ +* and it's value is larger than 512 */ sb->s_blocksize = size; sb->s_blocksize_bits =blksize_bits (size);return sb->s_blocksize;
Signature · 1 line
2.35.3
Re: [PATCH 3/5] block/bdev: lift restrictions on supported blocksize
Quoted 4 lines · On Fri, May 10, 2024 at 12:29:04PM +0200, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
We now can support blocksizes larger than
PAGE_SIZE, so lift the restriction.
Do we need to make this conditional on a CONFIG option? Not all architectures support THP yet ... which means they don't support large folios. We should fix that, but it is well below the cutoff line on my todo list.
Re: [PATCH 3/5] block/bdev: lift restrictions on supported blocksize
Quoted 10 lines · Matthew Wilcox
Quoted 4 lines · On Fri, May 10, 2024 at 12:29:04PM +0200, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
We now can support blocksizes larger than
PAGE_SIZE, so lift the restriction.Do we need to make this conditional on a CONFIG option? Not all architectures support THP yet ... which means they don't support large folios. We should fix that, but it is well below the cutoff line on my todo list.
Well. We will only ever trigger this if we _really_ encounter a block device with a block size larger than PAGE_SIZE. If which there currently are none (note to self: check NFS).
And the overall idea is that the logical blocksize is mandatory, ie the device _cannot_ handle requests with shorter blocksizes. We might be setting min_order == max_order, but again this can be discussed once we get min_order working :-)
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Re: [PATCH 3/5] block/bdev: lift restrictions on supported blocksize
In reply to Matthew WilcoxQuoted 1 line · Matthew Wilcox
Not all architectures support THP yet
I noted to Hannes the existing page cache limit, should we have a helper which also then uses CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE to fallback to PAGE_SIZE otherwise ?
Luis
Re: [PATCH 3/5] block/bdev: lift restrictions on supported blocksize
In reply to [email protected]Quoted 15 lines · On Fri, May 10, 2024 at 12:29:04PM +0200, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
We now can support blocksizes larger than
PAGE_SIZE, so lift the restriction.Signed-off-by Hannes Reinecke <[email protected]>--- block/bdev.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)block/bdev.c +0
-0
block/bdev.c static void set_init_blocksize(struct block_device *bdev) · line 150
Quoted 7 lines
block/bdev.c +2
-2@@ -146,8 +146,8 @@ static void set_init_blocksize(struct block_device *bdev)int set_blocksize (struct block_device *bdev,int size) { -/* Size must be a power of two, and between 512 and PAGE_SIZE */ -if (size >PAGE_SIZE || size <512 || !is_power_of_2 (size)) +/* Size must be a power of two, and larger than 512 */ +if (size <512 || !is_power_of_2 (size))
That needs to just be replaced with:
1 << (PAGE_SHIFT + MAX_PAGECACHE_ORDER)
We're not magically shooting to the sky all of a sudden. The large folio world still has a limit. While that's 2 MiB today, yes, we can go higher later, but even then, we still have a cap.
Luis
PATCH 4/5 block/bdev: enable large folio support for large logical block sizes
In reply to [email protected]From: Hannes Reinecke <[email protected]>
Call mapping_set_folio_min_order() when modifying the logical block size to ensure folios are allocated with the correct size.
--- block/bdev.c | 4 ++++ 1 file changed, 4 insertions(+)
block/bdev.c +4 -0
@@ -142,6 +142,8 @@ static void set_init_blocksize(struct block_device *bdev) bsize <<=1 ; } bdev->bd_inode->i_blkbits =blksize_bits (bsize); +mapping_set_folio_min_order (bdev->bd_inode->i_mapping, +get_order (bsize)); }int set_blocksize (struct block_device *bdev,int size)
@@ -158,6 +160,8 @@ int set_blocksize(struct block_device *bdev, int size)if (bdev->bd_inode->i_blkbits !=blksize_bits (size)) {sync_blockdev (bdev); bdev->bd_inode->i_blkbits =blksize_bits (size); +mapping_set_folio_min_order (bdev->bd_inode->i_mapping, +get_order (size));kill_bdev (bdev); }return 0 ;
Signature · 1 line
2.35.3
Re: [PATCH 4/5] block/bdev: enable large folio support for large logical block sizes
Quoted 4 lines · On Fri, May 10, 2024 at 12:29:05PM +0200, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
Call
mapping_set_folio_min_order()when modifying the logical block size to ensure folios are allocated with the correct size.
This makes me nervous. It lets the pagecache allocate folios larger than min_order (all the way up to PMD_ORDER). Filesystems may not cope well with seeing tail pages.
I'd _like_ to be able to do this. But I think for now we need to call mapping_set_folio_order_range(mapping, order, order);
Re: [PATCH 4/5] block/bdev: enable large folio support for large logical block sizes
Quoted 6 lines · Matthew Wilcox
Quoted 4 lines · On Fri, May 10, 2024 at 12:29:05PM +0200, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
Call
mapping_set_folio_min_order()when modifying the logical block size to ensure folios are allocated with the correct size.
Quoted 7 lines
This makes me nervous. It lets the pagecache allocate folios larger than
min_order(all the way up toPMD_ORDER). Filesystems may not cope well with seeing tail pages.I'd _like_ to be able to do this. But I think for now we need to call
mapping_set_folio_order_range(mapping, order, order);
(Misrouted reply to the previous mail; airports make me jumpy ...)
The whole point is that these devices cannot support smaller allocations. We might restrict ourselves to min_order == max_order, but we should be focussing on getting min_order working first ...
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
kernel test robot
build botRe: [PATCH 4/5] block/bdev: enable large folio support for large logical block sizes
In reply to [email protected]kernel test robot noticed the following build errors:
Hi,
kernel test robot noticed the following build errors:
[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on akpm-mm/mm-everything linus/master v6.9-rc7] [cannot apply to next-20240510]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/hare-kernel-org/fs-mpage-use-blocks_per_folio-instead-of-blocks_per_page/20240510-183146 base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next patch link: https://lore.kernel.org/r/20240510102906.51844-5-hare%40kernel.org patch subject: [PATCH 4/5] block/bdev: enable large folio support for large logical block sizes config: arc-allnoconfig (https://download.01.org/0day-ci/archive/20240511/[email protected]/config) compiler: arc-elf-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240511/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
block/bdev.c: In function 'set_init_blocksize':
>> block/bdev.c:145:9: error: implicit declaration of function 'mapping_set_folio_min_order' [-Werror=implicit-function-declaration]
145 | mapping_set_folio_min_order(bdev->bd_inode->i_mapping,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~cc1: some warnings being treated as errors
vim +/mapping_set_folio_min_order +145 block/bdev.c
133
134 static void set_init_blocksize(struct block_device *bdev)
135 {
136 unsigned int bsize = bdev_logical_block_size(bdev);
137 loff_t size = i_size_read(bdev->bd_inode);
138
139 while (bsize < PAGE_SIZE) {
140 if (size & bsize)
141 break;
142 bsize <<= 1;
143 }
144 bdev->bd_inode->i_blkbits = blksize_bits(bsize);
> 145 mapping_set_folio_min_order(bdev->bd_inode->i_mapping,
146 get_order(bsize));
147 }
148
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
kernel test robot
build botRe: [PATCH 4/5] block/bdev: enable large folio support for large logical block sizes
In reply to [email protected]kernel test robot noticed the following build errors:
Hi,
kernel test robot noticed the following build errors:
[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on akpm-mm/mm-everything linus/master v6.9-rc7] [cannot apply to next-20240510]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/hare-kernel-org/fs-mpage-use-blocks_per_folio-instead-of-blocks_per_page/20240510-183146 base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next patch link: https://lore.kernel.org/r/20240510102906.51844-5-hare%40kernel.org patch subject: [PATCH 4/5] block/bdev: enable large folio support for large logical block sizes config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20240511/[email protected]/config) compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240511/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
>> block/bdev.c:145:2: error: call to undeclared function 'mapping_set_folio_min_order'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
145 | mapping_set_folio_min_order(bdev->bd_inode->i_mapping,
| ^
block/bdev.c:163:3: error: call to undeclared function 'mapping_set_folio_min_order'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
163 | mapping_set_folio_min_order(bdev->bd_inode->i_mapping,
| ^2 errors generated.
vim +/mapping_set_folio_min_order +145 block/bdev.c
133
134 static void set_init_blocksize(struct block_device *bdev)
135 {
136 unsigned int bsize = bdev_logical_block_size(bdev);
137 loff_t size = i_size_read(bdev->bd_inode);
138
139 while (bsize < PAGE_SIZE) {
140 if (size & bsize)
141 break;
142 bsize <<= 1;
143 }
144 bdev->bd_inode->i_blkbits = blksize_bits(bsize);
> 145 mapping_set_folio_min_order(bdev->bd_inode->i_mapping,
146 get_order(bsize));
147 }
148
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
PATCH 5/5 nvme: enable logical block size > PAGE_SIZE
In reply to [email protected]From: Pankaj Raghav <[email protected]>
Don't set the capacity to zero for when logical block size > PAGE_SIZE as the block device with iomap aops support allocating block cache with a minimum folio order.
--- drivers/nvme/host/core.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/nvme/host/core.c +3 -4
@@ -1963,11 +1963,10 @@ static bool nvme_update_disk_info(struct nvme_ns *ns, struct nvme_id_ns *id,bool valid = true;/* -* The block layer can't support LBA sizes larger than the page size -* or smaller than a sector size yet, so catch this early and don't -* allow block I/O. +* The block layer can't support LBA sizes smaller than a sector size, +* so catch this early and don't allow block I/O. */ -if (head->lba_shift >PAGE_SHIFT || head->lba_shift <SECTOR_SHIFT ) { +if (head->lba_shift <SECTOR_SHIFT ) { bs = (1 <<9 ); valid = false; }
Signature · 1 line
2.35.3
Re: [PATCH 5/5] nvme: enable logical block size > PAGE_SIZE
Quoted 5 lines · On Fri, May 10, 2024 at 12:29:06PM +0200, [email protected] wrote:
From: Pankaj Raghav <[email protected]>
Don't set the capacity to zero for when logical block size >
PAGE_SIZEas the block device with iomap aops support allocating block cache with a minimum folio order.
It feels like this should be something the block layer does rather than something an individual block driver does. Is there similar code to rip out of sd.c? Or other block drivers?
Re: [PATCH 5/5] nvme: enable logical block size > PAGE_SIZE
Quoted 10 lines · Matthew Wilcox
Quoted 5 lines · On Fri, May 10, 2024 at 12:29:06PM +0200, [email protected] wrote:
From: Pankaj Raghav <[email protected]>
Don't set the capacity to zero for when logical block size >
PAGE_SIZEas the block device with iomap aops support allocating block cache with a minimum folio order.It feels like this should be something the block layer does rather than something an individual block driver does. Is there similar code to rip out of
sd.c? Or other block drivers?
Correct, this restriction should never have been there. I'll be checking sd.c, too.
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Re: [PATCH 5/5] nvme: enable logical block size > PAGE_SIZE
In reply to Matthew WilcoxQuoted 10 lines · Matthew Wilcox
Quoted 5 lines · On Fri, May 10, 2024 at 12:29:06PM +0200, [email protected] wrote:
From: Pankaj Raghav <[email protected]>
Don't set the capacity to zero for when logical block size >
PAGE_SIZEas the block device with iomap aops support allocating block cache with a minimum folio order.It feels like this should be something the block layer does rather than something an individual block driver does. Is there similar code to rip out of
sd.c? Or other block drivers?
Each block driver must be tested and set a max if its below what the page cache supports.
Luis
Re: [PATCH 5/5] nvme: enable logical block size > PAGE_SIZE
In reply to [email protected]Quoted 18 lines · On Fri, May 10, 2024 at 12:29:06PM +0200, [email protected] wrote:
From: Pankaj Raghav <[email protected]>
Don't set the capacity to zero for when logical block size >
PAGE_SIZEas the block device with iomap aops support allocating block cache with a minimum folio order.Signed-off-by Pankaj Raghav <[email protected]>Signed-off-by Hannes Reinecke <[email protected]>--- drivers/nvme/host/core.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)drivers/nvme/host/core.c +0
-0@@ -1963,11 +1963,10 @@ static bool nvme_update_disk_info(struct nvme_ns *ns, struct nvme_id_ns *id,bool valid = true;
drivers/nvme/host/core.c static bool nvme_update_disk_info(struct nvme_ns *ns, struct nvme_id_ns *id, · line 1969
Quoted 10 lines
drivers/nvme/host/core.c +3
-4/* -* The block layer can't support LBA sizes larger than the page size -* or smaller than a sector size yet, so catch this early and don't -* allow block I/O. +* The block layer can't support LBA sizes smaller than a sector size, +* so catch this early and don't allow block I/O. */ -if (head->lba_shift >PAGE_SHIFT || head->lba_shift <SECTOR_SHIFT ) { +if (head->lba_shift <SECTOR_SHIFT ) {
We can't just do this, we need to consider the actual nvme cap (test it, and if it crashes and below what the page cache supports, then we have to go below) and so to make the enablment easier. So we could just move this to helper [0]. Then when the bdev cache patch goes through the check for CONFIG_BUFFER_HEAD can be removed, if this goes first.
We crash if we go above 1 MiB today, we should be able to go up to 2 MiB but that requires some review to see what stupid thing is getting in the way.
Luis
Re: [PATCH 5/5] nvme: enable logical block size > PAGE_SIZE
Quoted 11 lines · Luis Chamberlain
We can't just do this, we need to consider the actual nvme cap (test it, and if it crashes and below what the page cache supports, then we have to go below) and so to make the enablment easier. So we could just move this to helper [0]. Then when the bdev cache patch goes through the check for
CONFIG_BUFFER_HEADcan be removed, if this goes first.We crash if we go above 1 MiB today, we should be able to go up to 2 MiB but that requires some review to see what stupid thing is getting in the way.
This is overengineered garbage. What's the crash?
Re: [PATCH 5/5] nvme: enable logical block size > PAGE_SIZE
Quoted 7 lines · Matthew Wilcox
Quoted 6 lines · Luis Chamberlain
We can't just do this, we need to consider the actual nvme cap (test it, and if it crashes and below what the page cache supports, then we have to go below) and so to make the enablment easier. So we could just move this to helper [0]. Then when the bdev cache patch goes through the check for
CONFIG_BUFFER_HEADcan be removed, if this goes first.
Quoted 7 lines
Quoted 5 lines
We crash if we go above 1 MiB today, we should be able to go up to 2 MiB but that requires some review to see what stupid thing is getting in the way.
This is overengineered garbage. What's the crash?
I had only tested it with iomap, I had not tested it with buffer-heads, and so it would require re-testing. It's Saturday 5pm, I should be doing something else other than being on my computer.
Luis
Re: [PATCH 5/5] nvme: enable logical block size > PAGE_SIZE
Quoted 12 lines · Luis Chamberlain
Quoted 11 lines · Matthew Wilcox
Quoted 10 lines · Luis Chamberlain
We can't just do this, we need to consider the actual nvme cap (test it, and if it crashes and below what the page cache supports, then we have to go below) and so to make the enablment easier. So we could just move this to helper [0]. Then when the bdev cache patch goes through the check for
CONFIG_BUFFER_HEADcan be removed, if this goes first.We crash if we go above 1 MiB today, we should be able to go up to 2 MiB but that requires some review to see what stupid thing is getting in the way.
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
This is overengineered garbage. What's the crash?
I had only tested it with iomap, I had not tested it with buffer-heads, and so it would require re-testing. It's Saturday 5pm, I should be doing something else other than being on my computer.
It crashes because we forgot two things in this series below, so the change below its us to enable to *at least* boot up to 64k LBA format on NVMe.
One can reproduce this with kdevops with:
make defconfig-lbs-xfs-bdev-nvme
make bringup
make linux
I've added another defconfig which bumps the LBA format up to 512 KiB to see if bootup blows up, that has another defconfig:
make lbs-xfs-bdev-large-nvme
make bringup
make linux
That at least booted. Note that the above defconfigs use this thread's message ID, so it applies this series on top of the min order branch. The patch below is just needed.
I'll try next going above 512 KiB.
fs/buffer.c +0 -2
@@ -2360,8 +2360,6 @@ int block_read_full_folio(struct folio *folio, get_block_t *get_block)if (IS_ENABLED (CONFIG_FS_VERITY ) &&IS_VERITY (inode)) limit = inode->i_sb->s_maxbytes; -VM_BUG_ON_FOLIO (folio_test_large (folio), folio); - head =folio_create_buffers (folio, inode,0 ); blocksize = head->b_size;
fs/mpage.c +0 -1
@@ -178,7 +178,6 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)gfp_t gfp =mapping_gfp_constraint (folio->mapping,GFP_KERNEL );/* MAX_BUF_PER_PAGE, for example */ -VM_BUG_ON_FOLIO (folio_test_large (folio), folio);if (args->is_readahead) { opf |=REQ_RAHEAD ;
Re: [PATCH 5/5] nvme: enable logical block size > PAGE_SIZE
Quoted 1 line · Luis Chamberlain
I'll try next going above 512 KiB.
At 1 MiB NVMe LBA format we crash with the BUG_ON(sectors <= 0) on bio_split().
[ 13.401651] ------------[ cut here ]------------ [ 13.403298] kernel BUG at block/bio.c:1626! [ 13.404708] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI [ 13.406390] CPU: 5 PID: 87 Comm: kworker/u38:1 Not tainted 6.9.0-rc6+ #2 [ 13.408480] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 [ 13.411304] Workqueue: nvme-wq nvme_scan_work [nvme_core] [ 13.412928] RIP: 0010:bio_split (block/bio.c:1626 (discriminator 1)) [ 13.414148] Code: 5b 4c 89 e0 5d 41 5c 41 5d c3 cc cc cc cc c7 43 28 00 00 00 00 eb db 0f 0b 45 31 e4 5b 5d 4c 89 e0 41 5c 41 5d c3 cc cc cc cc <0f> 0b 0f 0b 4c 89 e7 e8 bf ee ff ff eb e1 66 66 2e 0f 1f 84 00 00 All code ======== 0: 5b pop %rbx 1: 4c 89 e0 mov %r12,%rax 4: 5d pop %rbp 5: 41 5c pop %r12 7: 41 5d pop %r13 9: c3 ret a: cc int3 b: cc int3 c: cc int3 d: cc int3 e: c7 43 28 00 00 00 00 movl $0x0,0x28(%rbx) 15: eb db jmp 0xfffffffffffffff2 17: 0f 0b ud2 19: 45 31 e4 xor %r12d,%r12d 1c: 5b pop %rbx 1d: 5d pop %rbp 1e: 4c 89 e0 mov %r12,%rax 21: 41 5c pop %r12 23: 41 5d pop %r13 25: c3 ret 26: cc int3 27: cc int3 28: cc int3 29: cc int3 2a:* 0f 0b ud2 <-- trapping instruction 2c: 0f 0b ud2 2e: 4c 89 e7 mov %r12,%rdi 31: e8 bf ee ff ff call 0xffffffffffffeef5 36: eb e1 jmp 0x19 38: 66 data16 39: 66 data16 3a: 2e cs 3b: 0f .byte 0xf 3c: 1f (bad) 3d: 84 00 test %al,(%rax) ...
Code starting with the faulting instruction
===========================================
0: 0f 0b ud2 2: 0f 0b ud2 4: 4c 89 e7 mov %r12,%rdi 7: e8 bf ee ff ff call 0xffffffffffffeecb c: eb e1 jmp 0xffffffffffffffef e: 66 data16 f: 66 data16 10: 2e cs 11: 0f .byte 0xf 12: 1f (bad) 13: 84 00 test %al,(%rax) ... [ 13.420639] RSP: 0018:ffffc056407bb8e0 EFLAGS: 00010246 [ 13.422140] RAX: 0000000000000001 RBX: ffff9d97c165fa80 RCX: ffff9d97e65ce860 [ 13.424128] RDX: 0000000000000c00 RSI: 0000000000000000 RDI: ffff9d97c165fa80 [ 13.426123] RBP: 0000000000000000 R08: 0000000000000080 R09: 0000000000000000 [ 13.428372] R10: ffff9d97c165fa80 R11: ffff9d97c165faf8 R12: ffff9d97df3f7250 [ 13.430636] R13: 0000000000000000 R14: 0000000000000001 R15: 0000000000000000 [ 13.432510] FS: 0000000000000000(0000) GS:ffff9d983bd40000(0000) knlGS:0000000000000000 [ 13.434539] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 13.435987] CR2: 000055b34c8d6188 CR3: 000000011dfb8002 CR4: 0000000000770ef0 [ 13.437664] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 13.439354] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400 [ 13.440911] PKRU: 55555554 [ 13.441605] Call Trace: [ 13.442250] <TASK> [ 13.442836] ? die (arch/x86/kernel/dumpstack.c:421 arch/x86/kernel/dumpstack.c:434 arch/x86/kernel/dumpstack.c:447) [ 13.443585] ? do_trap (arch/x86/kernel/traps.c:114 arch/x86/kernel/traps.c:155) [ 13.444356] ? bio_split (block/bio.c:1626 (discriminator 1)) [ 13.445127] ? do_error_trap (./arch/x86/include/asm/traps.h:58 arch/x86/kernel/traps.c:176) [ 13.445975] ? bio_split (block/bio.c:1626 (discriminator 1)) [ 13.446760] ? exc_invalid_op (arch/x86/kernel/traps.c:267) [ 13.447635] ? bio_split (block/bio.c:1626 (discriminator 1)) [ 13.448325] ? asm_exc_invalid_op (./arch/x86/include/asm/idtentry.h:621) [ 13.449137] ? bio_split (block/bio.c:1626 (discriminator 1)) [ 13.449819] __bio_split_to_limits (block/blk-merge.c:366) [ 13.450675] blk_mq_submit_bio (block/blk-mq.c:2973) [ 13.451490] ? kmem_cache_alloc (./include/linux/kmemleak.h:42 mm/slub.c:3802 mm/slub.c:3845 mm/slub.c:3852) [ 13.452253] ? __mod_node_page_state (mm/vmstat.c:403 (discriminator 2)) [ 13.453050] submit_bio_noacct_nocheck (./include/linux/bio.h:639 block/blk-core.c:701 block/blk-core.c:729) [ 13.453897] ? submit_bio_noacct (block/blk-core.c:758 (discriminator 1)) [ 13.454658] block_read_full_folio (fs/buffer.c:2429 (discriminator 1)) [ 13.455467] ? __pfx_blkdev_get_block (block/fops.c:409) [ 13.456240] ? __pfx_blkdev_read_folio (block/fops.c:437) [ 13.457014] ? __pfx_blkdev_read_folio (block/fops.c:437) [ 13.457792] filemap_read_folio (mm/filemap.c:2335) [ 13.458484] do_read_cache_folio (mm/filemap.c:3763) [ 13.459220] ? __pfx_adfspart_check_ICS (block/partitions/acorn.c:351) [ 13.459967] read_part_sector (./include/linux/pagemap.h:970 block/partitions/core.c:715) [ 13.460602] adfspart_check_ICS (block/partitions/acorn.c:361) [ 13.461269] ? snprintf (lib/vsprintf.c:2963) [ 13.461844] ? __pfx_adfspart_check_ICS (block/partitions/acorn.c:351) [ 13.462601] bdev_disk_changed (block/partitions/core.c:138 block/partitions/core.c:582 block/partitions/core.c:686 block/partitions/core.c:635) [ 13.463268] blkdev_get_whole (block/bdev.c:684) [ 13.463871] bdev_open (block/bdev.c:893) [ 13.464386] bdev_file_open_by_dev (block/bdev.c:995 block/bdev.c:969) [ 13.465006] disk_scan_partitions (block/genhd.c:369 (discriminator 1)) [ 13.465621] device_add_disk (block/genhd.c:512) [ 13.466199] nvme_scan_ns (drivers/nvme/host/core.c:3807 (discriminator 1) drivers/nvme/host/core.c:3961 (discriminator 1)) nvme_core [ 13.466885] nvme_scan_work (drivers/nvme/host/core.c:4015 drivers/nvme/host/core.c:4105) nvme_core [ 13.467598] process_one_work (kernel/workqueue.c:3254) [ 13.468186] worker_thread (kernel/workqueue.c:3329 (discriminator 2) kernel/workqueue.c:3416 (discriminator 2)) [ 13.468723] ? _raw_spin_lock_irqsave (./arch/x86/include/asm/atomic.h:115 (discriminator 4) ./include/linux/atomic/atomic-arch-fallback.h:2170 (discriminator 4) ./include/linux/atomic/atomic-instrumented.h:1302 (discriminator 4) ./include/asm-generic/qspinlock.h:111 (discriminator 4) ./include/linux/spinlock.h:187 (discriminator 4) ./include/linux/spinlock_api_smp.h:111 (discriminator 4) kernel/locking/spinlock.c:162 (discriminator 4)) [ 13.469343] ? __pfx_worker_thread (kernel/workqueue.c:3362) [ 13.469933] kthread (kernel/kthread.c:388) [ 13.470395] ? __pfx_kthread (kernel/kthread.c:341) [ 13.470928] ret_from_fork (arch/x86/kernel/process.c:147) [ 13.471457] ? __pfx_kthread (kernel/kthread.c:341) [ 13.472008] ret_from_fork_asm (arch/x86/entry/entry_64.S:257) [ 13.472540] </TASK> [ 13.472865] Modules linked in: crc32c_intel psmouse nvme nvme_core t10_pi crc64_rocksoft crc64 virtio_pci virtio_pci_legacy_dev virtio_pci_modern_dev virtio virtio_ring [ 13.474629] ---[ end trace 0000000000000000 ]--- [ 13.475229] RIP: 0010:bio_split (block/bio.c:1626 (discriminator 1)) [ 13.475742] Code: 5b 4c 89 e0 5d 41 5c 41 5d c3 cc cc cc cc c7 43 28 00 00 00 00 eb db 0f 0b 45 31 e4 5b 5d 4c 89 e0 41 5c 41 5d c3 cc cc cc cc <0f> 0b 0f 0b 4c 89 e7 e8 bf ee ff ff eb e1 66 66 2e 0f 1f 84 00 00 All code ======== 0: 5b pop %rbx 1: 4c 89 e0 mov %r12,%rax 4: 5d pop %rbp 5: 41 5c pop %r12 7: 41 5d pop %r13 9: c3 ret a: cc int3 b: cc int3 c: cc int3 d: cc int3 e: c7 43 28 00 00 00 00 movl $0x0,0x28(%rbx) 15: eb db jmp 0xfffffffffffffff2 17: 0f 0b ud2 19: 45 31 e4 xor %r12d,%r12d 1c: 5b pop %rbx 1d: 5d pop %rbp 1e: 4c 89 e0 mov %r12,%rax 21: 41 5c pop %r12 23: 41 5d pop %r13 25: c3 ret 26: cc int3 27: cc int3 28: cc int3 29: cc int3 2a:* 0f 0b ud2 <-- trapping instruction 2c: 0f 0b ud2 2e: 4c 89 e7 mov %r12,%rdi 31: e8 bf ee ff ff call 0xffffffffffffeef5 36: eb e1 jmp 0x19 38: 66 data16 39: 66 data16 3a: 2e cs 3b: 0f .byte 0xf 3c: 1f (bad) 3d: 84 00 test %al,(%rax) ...
Code starting with the faulting instruction
===========================================
0: 0f 0b ud2 2: 0f 0b ud2 4: 4c 89 e7 mov %r12,%rdi 7: e8 bf ee ff ff call 0xffffffffffffeecb c: eb e1 jmp 0xffffffffffffffef e: 66 data16 f: 66 data16 10: 2e cs 11: 0f .byte 0xf 12: 1f (bad) 13: 84 00 test %al,(%rax) ... [ 13.477749] RSP: 0018:ffffc056407bb8e0 EFLAGS: 00010246 [ 13.478405] RAX: 0000000000000001 RBX: ffff9d97c165fa80 RCX: ffff9d97e65ce860 [ 13.479230] RDX: 0000000000000c00 RSI: 0000000000000000 RDI: ffff9d97c165fa80 [ 13.480026] RBP: 0000000000000000 R08: 0000000000000080 R09: 0000000000000000 [ 13.480802] R10: ffff9d97c165fa80 R11: ffff9d97c165faf8 R12: ffff9d97df3f7250 [ 13.481568] R13: 0000000000000000 R14: 0000000000000001 R15: 0000000000000000 [ 13.482337] FS: 0000000000000000(0000) GS:ffff9d983bd40000(0000) knlGS:0000000000000000 [ 13.483218] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 13.483853] CR2: 000055b34c8d6188 CR3: 000000011dfb8002 CR4: 0000000000770ef0 [ 13.484584] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 13.485327] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400 [ 13.486070] PKRU: 55555554
Re: [PATCH 5/5] nvme: enable logical block size > PAGE_SIZE
Quoted 185 lines · Luis Chamberlain
Quoted 1 line · Luis Chamberlain
I'll try next going above 512 KiB.
At 1 MiB NVMe LBA format we crash with the
BUG_ON(sectors <= 0)onbio_split().[ 13.401651] ------------[ cut here ]------------ [ 13.403298] kernel BUG at block/bio.c:1626! [ 13.404708] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI [ 13.406390] CPU: 5 PID: 87 Comm: kworker/u38:1 Not tainted 6.9.0-rc6+ #2 [ 13.408480] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 [ 13.411304] Workqueue: nvme-wq nvme_scan_work [nvme_core] [ 13.412928] RIP: 0010:bio_split (block/bio.c:1626 (discriminator 1)) [ 13.414148] Code: 5b 4c 89 e0 5d 41 5c 41 5d c3 cc cc cc cc c7 43 28 00 00 00 00 eb db 0f 0b 45 31 e4 5b 5d 4c 89 e0 41 5c 41 5d c3 cc cc cc cc <0f> 0b 0f 0b 4c 89 e7 e8 bf ee ff ff eb e1 66 66 2e 0f 1f 84 00 00 All code ======== 0: 5b pop %rbx 1: 4c 89 e0 mov %r12,%rax 4: 5d pop %rbp 5: 41 5c pop %r12 7: 41 5d pop %r13 9: c3 ret a: cc int3 b: cc int3 c: cc int3 d: cc int3 e: c7 43 28 00 00 00 00 movl $0x0,0x28(%rbx) 15: eb db jmp 0xfffffffffffffff2 17: 0f 0b ud2 19: 45 31 e4 xor %r12d,%r12d 1c: 5b pop %rbx 1d: 5d pop %rbp 1e: 4c 89 e0 mov %r12,%rax 21: 41 5c pop %r12 23: 41 5d pop %r13 25: c3 ret 26: cc int3 27: cc int3 28: cc int3 29: cc int3 2a:* 0f 0b ud2 <-- trapping instruction 2c: 0f 0b ud2 2e: 4c 89 e7 mov %r12,%rdi 31: e8 bf ee ff ff call 0xffffffffffffeef5 36: eb e1 jmp 0x19 38: 66 data16 39: 66 data16 3a: 2e cs 3b: 0f .byte 0xf 3c: 1f (bad) 3d: 84 00 test %al,(%rax) ...Code starting with the faulting instruction
===========================================0: 0f 0b ud2 2: 0f 0b ud2 4: 4c 89 e7 mov %r12,%rdi 7: e8 bf ee ff ff call 0xffffffffffffeecb c: eb e1 jmp 0xffffffffffffffef e: 66 data16 f: 66 data16 10: 2e cs 11: 0f .byte 0xf 12: 1f (bad) 13: 84 00 test %al,(%rax) ... [ 13.420639] RSP: 0018:ffffc056407bb8e0 EFLAGS: 00010246 [ 13.422140] RAX: 0000000000000001 RBX: ffff9d97c165fa80 RCX: ffff9d97e65ce860 [ 13.424128] RDX: 0000000000000c00 RSI: 0000000000000000 RDI: ffff9d97c165fa80 [ 13.426123] RBP: 0000000000000000 R08: 0000000000000080 R09: 0000000000000000 [ 13.428372] R10: ffff9d97c165fa80 R11: ffff9d97c165faf8 R12: ffff9d97df3f7250 [ 13.430636] R13: 0000000000000000 R14: 0000000000000001 R15: 0000000000000000 [ 13.432510] FS: 0000000000000000(0000) GS:ffff9d983bd40000(0000) knlGS:0000000000000000 [ 13.434539] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 13.435987] CR2: 000055b34c8d6188 CR3: 000000011dfb8002 CR4: 0000000000770ef0 [ 13.437664] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 13.439354] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400 [ 13.440911] PKRU: 55555554 [ 13.441605] Call Trace: [ 13.442250] <TASK> [ 13.442836] ? die (arch/x86/kernel/dumpstack.c:421 arch/x86/kernel/dumpstack.c:434 arch/x86/kernel/dumpstack.c:447) [ 13.443585] ? do_trap (arch/x86/kernel/traps.c:114 arch/x86/kernel/traps.c:155) [ 13.444356] ? bio_split (block/bio.c:1626 (discriminator 1)) [ 13.445127] ? do_error_trap (./arch/x86/include/asm/traps.h:58 arch/x86/kernel/traps.c:176) [ 13.445975] ? bio_split (block/bio.c:1626 (discriminator 1)) [ 13.446760] ? exc_invalid_op (arch/x86/kernel/traps.c:267) [ 13.447635] ? bio_split (block/bio.c:1626 (discriminator 1)) [ 13.448325] ? asm_exc_invalid_op (./arch/x86/include/asm/idtentry.h:621) [ 13.449137] ? bio_split (block/bio.c:1626 (discriminator 1)) [ 13.449819] __bio_split_to_limits (block/blk-merge.c:366) [ 13.450675] blk_mq_submit_bio (block/blk-mq.c:2973) [ 13.451490] ? kmem_cache_alloc (./include/linux/kmemleak.h:42 mm/slub.c:3802 mm/slub.c:3845 mm/slub.c:3852) [ 13.452253] ? __mod_node_page_state (mm/vmstat.c:403 (discriminator 2)) [ 13.453050] submit_bio_noacct_nocheck (./include/linux/bio.h:639 block/blk-core.c:701 block/blk-core.c:729) [ 13.453897] ? submit_bio_noacct (block/blk-core.c:758 (discriminator 1)) [ 13.454658] block_read_full_folio (fs/buffer.c:2429 (discriminator 1)) [ 13.455467] ? __pfx_blkdev_get_block (block/fops.c:409) [ 13.456240] ? __pfx_blkdev_read_folio (block/fops.c:437) [ 13.457014] ? __pfx_blkdev_read_folio (block/fops.c:437) [ 13.457792] filemap_read_folio (mm/filemap.c:2335) [ 13.458484] do_read_cache_folio (mm/filemap.c:3763) [ 13.459220] ? __pfx_adfspart_check_ICS (block/partitions/acorn.c:351) [ 13.459967] read_part_sector (./include/linux/pagemap.h:970 block/partitions/core.c:715) [ 13.460602] adfspart_check_ICS (block/partitions/acorn.c:361) [ 13.461269] ? snprintf (lib/vsprintf.c:2963) [ 13.461844] ? __pfx_adfspart_check_ICS (block/partitions/acorn.c:351) [ 13.462601] bdev_disk_changed (block/partitions/core.c:138 block/partitions/core.c:582 block/partitions/core.c:686 block/partitions/core.c:635) [ 13.463268] blkdev_get_whole (block/bdev.c:684) [ 13.463871] bdev_open (block/bdev.c:893) [ 13.464386] bdev_file_open_by_dev (block/bdev.c:995 block/bdev.c:969) [ 13.465006] disk_scan_partitions (block/genhd.c:369 (discriminator 1)) [ 13.465621] device_add_disk (block/genhd.c:512) [ 13.466199] nvme_scan_ns (drivers/nvme/host/core.c:3807 (discriminator 1) drivers/nvme/host/core.c:3961 (discriminator 1)) nvme_core [ 13.466885] nvme_scan_work (drivers/nvme/host/core.c:4015 drivers/nvme/host/core.c:4105) nvme_core [ 13.467598] process_one_work (kernel/workqueue.c:3254) [ 13.468186] worker_thread (kernel/workqueue.c:3329 (discriminator 2) kernel/workqueue.c:3416 (discriminator 2)) [ 13.468723] ? _raw_spin_lock_irqsave (./arch/x86/include/asm/atomic.h:115 (discriminator 4) ./include/linux/atomic/atomic-arch-fallback.h:2170 (discriminator 4) ./include/linux/atomic/atomic-instrumented.h:1302 (discriminator 4) ./include/asm-generic/qspinlock.h:111 (discriminator 4) ./include/linux/spinlock.h:187 (discriminator 4) ./include/linux/spinlock_api_smp.h:111 (discriminator 4) kernel/locking/spinlock.c:162 (discriminator 4)) [ 13.469343] ? __pfx_worker_thread (kernel/workqueue.c:3362) [ 13.469933] kthread (kernel/kthread.c:388) [ 13.470395] ? __pfx_kthread (kernel/kthread.c:341) [ 13.470928] ret_from_fork (arch/x86/kernel/process.c:147) [ 13.471457] ? __pfx_kthread (kernel/kthread.c:341) [ 13.472008] ret_from_fork_asm (arch/x86/entry/entry_64.S:257) [ 13.472540] </TASK> [ 13.472865] Modules linked in: crc32c_intel psmouse nvme nvme_core t10_pi crc64_rocksoft crc64 virtio_pci virtio_pci_legacy_dev virtio_pci_modern_dev virtio virtio_ring [ 13.474629] ---[ end trace 0000000000000000 ]--- [ 13.475229] RIP: 0010:bio_split (block/bio.c:1626 (discriminator 1)) [ 13.475742] Code: 5b 4c 89 e0 5d 41 5c 41 5d c3 cc cc cc cc c7 43 28 00 00 00 00 eb db 0f 0b 45 31 e4 5b 5d 4c 89 e0 41 5c 41 5d c3 cc cc cc cc <0f> 0b 0f 0b 4c 89 e7 e8 bf ee ff ff eb e1 66 66 2e 0f 1f 84 00 00 All code ======== 0: 5b pop %rbx 1: 4c 89 e0 mov %r12,%rax 4: 5d pop %rbp 5: 41 5c pop %r12 7: 41 5d pop %r13 9: c3 ret a: cc int3 b: cc int3 c: cc int3 d: cc int3 e: c7 43 28 00 00 00 00 movl $0x0,0x28(%rbx) 15: eb db jmp 0xfffffffffffffff2 17: 0f 0b ud2 19: 45 31 e4 xor %r12d,%r12d 1c: 5b pop %rbx 1d: 5d pop %rbp 1e: 4c 89 e0 mov %r12,%rax 21: 41 5c pop %r12 23: 41 5d pop %r13 25: c3 ret 26: cc int3 27: cc int3 28: cc int3 29: cc int3 2a:* 0f 0b ud2 <-- trapping instruction 2c: 0f 0b ud2 2e: 4c 89 e7 mov %r12,%rdi 31: e8 bf ee ff ff call 0xffffffffffffeef5 36: eb e1 jmp 0x19 38: 66 data16 39: 66 data16 3a: 2e cs 3b: 0f .byte 0xf 3c: 1f (bad) 3d: 84 00 test %al,(%rax) ...Code starting with the faulting instruction
===========================================0: 0f 0b ud2 2: 0f 0b ud2 4: 4c 89 e7 mov %r12,%rdi 7: e8 bf ee ff ff call 0xffffffffffffeecb c: eb e1 jmp 0xffffffffffffffef e: 66 data16 f: 66 data16 10: 2e cs 11: 0f .byte 0xf 12: 1f (bad) 13: 84 00 test %al,(%rax) ... [ 13.477749] RSP: 0018:ffffc056407bb8e0 EFLAGS: 00010246 [ 13.478405] RAX: 0000000000000001 RBX: ffff9d97c165fa80 RCX: ffff9d97e65ce860 [ 13.479230] RDX: 0000000000000c00 RSI: 0000000000000000 RDI: ffff9d97c165fa80 [ 13.480026] RBP: 0000000000000000 R08: 0000000000000080 R09: 0000000000000000
Quoted 9 lines
[ 13.480802] R10: ffff9d97c165fa80 R11: ffff9d97c165faf8 R12: ffff9d97df3f7250 [ 13.481568] R13: 0000000000000000 R14: 0000000000000001 R15: 0000000000000000 [ 13.482337] FS: 0000000000000000(0000) GS:ffff9d983bd40000(0000) knlGS:0000000000000000 [ 13.483218] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 13.483853] CR2: 000055b34c8d6188 CR3: 000000011dfb8002 CR4: 0000000000770ef0 [ 13.484584] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 13.485327] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400 [ 13.486070] PKRU: 55555554
Ah. MAX_BUFS_PER_PAGE getting in the way.
Can you test with the attached patch?
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
From 550f8f3b556cfd1f9190e3895511cf46659521d1 Mon Sep 17 00:00:00 2001
From: Hannes Reinecke <[email protected]>
Date: Wed, 14 Feb 2024 09:20:21 +0100
Subject: [PATCH] fs/buffer: restart block_read_full_folio() to avoid array
overflow
block_read_full_folio() uses an on-stack array to hold any buffer_heads which should be updated. The array is sized for the number of buffer_heads per PAGE_SIZE, which of course will overflow for large folios. So instead of increasing the size of the array (and thereby incurring a possible stack overflow for really large folios) stop the iteration when the array is filled up, submit these buffer_heads, and restart the iteration with the remaining buffer_heads.
--- fs/buffer.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)
fs/buffer.c +17 -2
@@ -2349,7 +2349,7 @@ int block_read_full_folio(struct folio *folio, get_block_t *get_block) {struct inode *inode = folio->mapping->host;sector_t iblock, lblock; -struct buffer_head *bh, *head, *arr[MAX_BUF_PER_PAGE ]; +struct buffer_head *bh, *head, *restart_bh =NULL , *arr[MAX_BUF_PER_PAGE ];size_t blocksize;int nr, i;int fully_mapped =1 ;
@@ -2366,6 +2366,7 @@ int block_read_full_folio(struct folio *folio, get_block_t *get_block) iblock =div_u64 (folio_pos (folio), blocksize); lblock =div_u64 (limit + blocksize -1 , blocksize); bh = head; +restart: nr =0 ; i =0 ;
@@ -2400,7 +2401,12 @@ int block_read_full_folio(struct folio *folio, get_block_t *get_block)continue ; } arr[nr++] = bh; - }while (i++, iblock++, (bh = bh->b_this_page) != head); + }while (i++, iblock++, (bh = bh->b_this_page) != head && nr <MAX_BUF_PER_PAGE ); + +if (nr ==MAX_BUF_PER_PAGE && bh != head) + restart_bh = bh; +else + restart_bh =NULL ;if (fully_mapped)folio_set_mappedtodisk (folio);
@@ -2433,6 +2439,15 @@ int block_read_full_folio(struct folio *folio, get_block_t *get_block)else submit_bh (REQ_OP_READ , bh); } +/* +* Found more buffers than 'arr' could hold, +* restart to submit the remaining ones. +*/ +if (restart_bh) { + bh = restart_bh; + goto restart; + } +return 0 ; }EXPORT_SYMBOL (block_read_full_folio);
Signature · 1 line
2.35.3
Re: [PATCH 5/5] nvme: enable logical block size > PAGE_SIZE
Quoted 11 lines · Hannes Reinecke
Quoted 7 lines · Luis Chamberlain
Quoted 1 line · Luis Chamberlain
I'll try next going above 512 KiB.
At 1 MiB NVMe LBA format we crash with the
BUG_ON(sectors <= 0)onbio_split().[ 13.401651] ------------[ cut here ]------------ [ 13.403298] kernel BUG at block/bio.c:1626!Ah.
MAX_BUFS_PER_PAGEgetting in the way.Can you test with the attached patch?
Nope same crash:
I've enabled you to easily test with with NVMe on libvirt with kdevops, please test.
Luis
[ 14.972734] ------------[ cut here ]------------ [ 14.974731] kernel BUG at block/bio.c:1626! [ 14.976906] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI [ 14.978899] CPU: 3 PID: 59 Comm: kworker/u36:0 Not tainted 6.9.0-rc6+ #4 [ 14.981005] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 [ 14.983782] Workqueue: nvme-wq nvme_scan_work [nvme_core] [ 14.985431] RIP: 0010:bio_split+0xd5/0xf0 [ 14.986627] Code: 5b 4c 89 e0 5d 41 5c 41 5d c3 cc cc cc cc c7 43 28 00 00 00 00 eb db 0f 0b 45 31 e4 5b 5d 4c 89 e0 41 5c 41 5d c3 cc cc cc cc <0f> 0b 0f 0b 4c 89 e7 e8 bf ee ff ff eb e1 66 66 2e 0f 1f 84 00 00 [ 14.992063] RSP: 0018:ffffbecc002378d0 EFLAGS: 00010246 [ 14.993416] RAX: 0000000000000001 RBX: ffff9e2fe8583e40 RCX: ffff9e2fdcb73060 [ 14.995181] RDX: 0000000000000c00 RSI: 0000000000000000 RDI: ffff9e2fe8583e40 [ 14.996960] RBP: 0000000000000000 R08: 0000000000000080 R09: 0000000000000000 [ 14.998715] R10: ffff9e2fe8583e40 R11: ffff9e2fe8583eb8 R12: ffff9e2fe884b750 [ 15.000510] R13: 0000000000000000 R14: 0000000000000001 R15: 0000000000000000 [ 15.002128] FS: 0000000000000000(0000) GS:ffff9e303bcc0000(0000) knlGS:0000000000000000 [ 15.003956] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 15.005294] CR2: 0000561b2b5ce478 CR3: 0000000102484002 CR4: 0000000000770ef0 [ 15.006921] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 15.008509] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400 [ 15.010001] PKRU: 55555554 [ 15.010672] Call Trace: [ 15.011297] <TASK> [ 15.011868] ? die+0x32/0x80 [ 15.012572] ? do_trap+0xd9/0x100 [ 15.013306] ? bio_split+0xd5/0xf0 [ 15.014051] ? do_error_trap+0x6a/0x90 [ 15.014854] ? bio_split+0xd5/0xf0 [ 15.015597] ? exc_invalid_op+0x4c/0x60 [ 15.016419] ? bio_split+0xd5/0xf0 [ 15.017113] ? asm_exc_invalid_op+0x16/0x20 [ 15.017932] ? bio_split+0xd5/0xf0 [ 15.018624] __bio_split_to_limits+0x90/0x2d0 [ 15.019474] blk_mq_submit_bio+0x111/0x6a0 [ 15.020280] ? kmem_cache_alloc+0x254/0x2e0 [ 15.021040] submit_bio_noacct_nocheck+0x2f1/0x3d0 [ 15.021893] ? submit_bio_noacct+0x42/0x5b0 [ 15.022658] block_read_full_folio+0x2b7/0x350 [ 15.023457] ? __pfx_blkdev_get_block+0x10/0x10 [ 15.024284] ? __pfx_blkdev_read_folio+0x10/0x10 [ 15.025073] ? __pfx_blkdev_read_folio+0x10/0x10 [ 15.025851] filemap_read_folio+0x32/0xb0 [ 15.026540] do_read_cache_folio+0x108/0x200 [ 15.027271] ? __pfx_adfspart_check_ICS+0x10/0x10 [ 15.028066] read_part_sector+0x32/0xe0 [ 15.028701] adfspart_check_ICS+0x32/0x480 [ 15.029334] ? snprintf+0x49/0x70 [ 15.029875] ? __pfx_adfspart_check_ICS+0x10/0x10 [ 15.030592] bdev_disk_changed+0x2a2/0x6e0 [ 15.031226] blkdev_get_whole+0x5f/0xa0 [ 15.031827] bdev_open+0x201/0x3c0 [ 15.032360] bdev_file_open_by_dev+0xb5/0x110 [ 15.032990] disk_scan_partitions+0x65/0xe0 [ 15.033598] device_add_disk+0x3e0/0x3f0 [ 15.034172] nvme_scan_ns+0x5f0/0xe50 [nvme_core] [ 15.034862] nvme_scan_work+0x26f/0x5a0 [nvme_core] [ 15.035568] process_one_work+0x189/0x3b0 [ 15.036168] worker_thread+0x273/0x390 [ 15.036713] ? __pfx_worker_thread+0x10/0x10 [ 15.037312] kthread+0xda/0x110 [ 15.037779] ? __pfx_kthread+0x10/0x10 [ 15.038316] ret_from_fork+0x2d/0x50 [ 15.038829] ? __pfx_kthread+0x10/0x10 [ 15.039364] ret_from_fork_asm+0x1a/0x30 [ 15.039924] </TASK>
Re: [PATCH 5/5] nvme: enable logical block size > PAGE_SIZE
Quoted 76 lines · Luis Chamberlain
Quoted 11 lines · Hannes Reinecke
Quoted 7 lines · Luis Chamberlain
Quoted 1 line · Luis Chamberlain
I'll try next going above 512 KiB.
At 1 MiB NVMe LBA format we crash with the
BUG_ON(sectors <= 0)onbio_split().[ 13.401651] ------------[ cut here ]------------ [ 13.403298] kernel BUG at block/bio.c:1626!Ah.
MAX_BUFS_PER_PAGEgetting in the way.Can you test with the attached patch?
Nope same crash:
I've enabled you to easily test with with NVMe on libvirt with kdevops, please test.
Luis
[ 14.972734] ------------[ cut here ]------------ [ 14.974731] kernel BUG at block/bio.c:1626! [ 14.976906] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI [ 14.978899] CPU: 3 PID: 59 Comm: kworker/u36:0 Not tainted 6.9.0-rc6+ #4 [ 14.981005] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 [ 14.983782] Workqueue: nvme-wq nvme_scan_work [nvme_core] [ 14.985431] RIP: 0010:bio_split+0xd5/0xf0 [ 14.986627] Code: 5b 4c 89 e0 5d 41 5c 41 5d c3 cc cc cc cc c7 43 28 00 00 00 00 eb db 0f 0b 45 31 e4 5b 5d 4c 89 e0 41 5c 41 5d c3 cc cc cc cc <0f> 0b 0f 0b 4c 89 e7 e8 bf ee ff ff eb e1 66 66 2e 0f 1f 84 00 00 [ 14.992063] RSP: 0018:ffffbecc002378d0 EFLAGS: 00010246 [ 14.993416] RAX: 0000000000000001 RBX: ffff9e2fe8583e40 RCX: ffff9e2fdcb73060 [ 14.995181] RDX: 0000000000000c00 RSI: 0000000000000000 RDI: ffff9e2fe8583e40 [ 14.996960] RBP: 0000000000000000 R08: 0000000000000080 R09: 0000000000000000 [ 14.998715] R10: ffff9e2fe8583e40 R11: ffff9e2fe8583eb8 R12: ffff9e2fe884b750 [ 15.000510] R13: 0000000000000000 R14: 0000000000000001 R15: 0000000000000000 [ 15.002128] FS: 0000000000000000(0000) GS:ffff9e303bcc0000(0000) knlGS:0000000000000000 [ 15.003956] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 15.005294] CR2: 0000561b2b5ce478 CR3: 0000000102484002 CR4: 0000000000770ef0 [ 15.006921] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 15.008509] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400 [ 15.010001] PKRU: 55555554 [ 15.010672] Call Trace: [ 15.011297] <TASK> [ 15.011868] ? die+0x32/0x80 [ 15.012572] ? do_trap+0xd9/0x100 [ 15.013306] ? bio_split+0xd5/0xf0 [ 15.014051] ? do_error_trap+0x6a/0x90 [ 15.014854] ? bio_split+0xd5/0xf0 [ 15.015597] ? exc_invalid_op+0x4c/0x60 [ 15.016419] ? bio_split+0xd5/0xf0 [ 15.017113] ? asm_exc_invalid_op+0x16/0x20 [ 15.017932] ? bio_split+0xd5/0xf0 [ 15.018624] __bio_split_to_limits+0x90/0x2d0 [ 15.019474] blk_mq_submit_bio+0x111/0x6a0 [ 15.020280] ? kmem_cache_alloc+0x254/0x2e0 [ 15.021040] submit_bio_noacct_nocheck+0x2f1/0x3d0 [ 15.021893] ? submit_bio_noacct+0x42/0x5b0 [ 15.022658] block_read_full_folio+0x2b7/0x350 [ 15.023457] ? __pfx_blkdev_get_block+0x10/0x10 [ 15.024284] ? __pfx_blkdev_read_folio+0x10/0x10 [ 15.025073] ? __pfx_blkdev_read_folio+0x10/0x10 [ 15.025851] filemap_read_folio+0x32/0xb0 [ 15.026540] do_read_cache_folio+0x108/0x200 [ 15.027271] ? __pfx_adfspart_check_ICS+0x10/0x10 [ 15.028066] read_part_sector+0x32/0xe0 [ 15.028701] adfspart_check_ICS+0x32/0x480 [ 15.029334] ? snprintf+0x49/0x70 [ 15.029875] ? __pfx_adfspart_check_ICS+0x10/0x10 [ 15.030592] bdev_disk_changed+0x2a2/0x6e0 [ 15.031226] blkdev_get_whole+0x5f/0xa0 [ 15.031827] bdev_open+0x201/0x3c0 [ 15.032360] bdev_file_open_by_dev+0xb5/0x110 [ 15.032990] disk_scan_partitions+0x65/0xe0 [ 15.033598] device_add_disk+0x3e0/0x3f0 [ 15.034172] nvme_scan_ns+0x5f0/0xe50 [nvme_core] [ 15.034862] nvme_scan_work+0x26f/0x5a0 [nvme_core] [ 15.035568] process_one_work+0x189/0x3b0
Quoted 9 lines
[ 15.036168] worker_thread+0x273/0x390 [ 15.036713] ? __pfx_worker_thread+0x10/0x10 [ 15.037312] kthread+0xda/0x110 [ 15.037779] ? __pfx_kthread+0x10/0x10 [ 15.038316] ret_from_fork+0x2d/0x50 [ 15.038829] ? __pfx_kthread+0x10/0x10 [ 15.039364] ret_from_fork_asm+0x1a/0x30 [ 15.039924] </TASK>
Ah. So this should fix it:
block/blk-merge.c +1 -1
@@ -289,7 +289,7 @@ struct bio *bio_split_rw(struct bio *bio, const struct queue_limits *lim,if (nsegs < lim->max_segments && bytes + bv.bv_len <= max_bytes && - bv.bv_offset + bv.bv_len <=PAGE_SIZE ) { + bv.bv_offset + bv.bv_len <= lim->max_segment_size) { nsegs++; bytes += bv.bv_len; }else {
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Re: [PATCH 5/5] nvme: enable logical block size > PAGE_SIZE
In reply to Luis ChamberlainQuoted 76 lines · Luis Chamberlain
Quoted 11 lines · Hannes Reinecke
Quoted 7 lines · Luis Chamberlain
Quoted 1 line · Luis Chamberlain
I'll try next going above 512 KiB.
At 1 MiB NVMe LBA format we crash with the
BUG_ON(sectors <= 0)onbio_split().[ 13.401651] ------------[ cut here ]------------ [ 13.403298] kernel BUG at block/bio.c:1626!Ah.
MAX_BUFS_PER_PAGEgetting in the way.Can you test with the attached patch?
Nope same crash:
I've enabled you to easily test with with NVMe on libvirt with kdevops, please test.
Luis
[ 14.972734] ------------[ cut here ]------------ [ 14.974731] kernel BUG at block/bio.c:1626! [ 14.976906] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI [ 14.978899] CPU: 3 PID: 59 Comm: kworker/u36:0 Not tainted 6.9.0-rc6+ #4 [ 14.981005] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 [ 14.983782] Workqueue: nvme-wq nvme_scan_work [nvme_core] [ 14.985431] RIP: 0010:bio_split+0xd5/0xf0 [ 14.986627] Code: 5b 4c 89 e0 5d 41 5c 41 5d c3 cc cc cc cc c7 43 28 00 00 00 00 eb db 0f 0b 45 31 e4 5b 5d 4c 89 e0 41 5c 41 5d c3 cc cc cc cc <0f> 0b 0f 0b 4c 89 e7 e8 bf ee ff ff eb e1 66 66 2e 0f 1f 84 00 00 [ 14.992063] RSP: 0018:ffffbecc002378d0 EFLAGS: 00010246 [ 14.993416] RAX: 0000000000000001 RBX: ffff9e2fe8583e40 RCX: ffff9e2fdcb73060 [ 14.995181] RDX: 0000000000000c00 RSI: 0000000000000000 RDI: ffff9e2fe8583e40 [ 14.996960] RBP: 0000000000000000 R08: 0000000000000080 R09: 0000000000000000 [ 14.998715] R10: ffff9e2fe8583e40 R11: ffff9e2fe8583eb8 R12: ffff9e2fe884b750 [ 15.000510] R13: 0000000000000000 R14: 0000000000000001 R15: 0000000000000000 [ 15.002128] FS: 0000000000000000(0000) GS:ffff9e303bcc0000(0000) knlGS:0000000000000000 [ 15.003956] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 15.005294] CR2: 0000561b2b5ce478 CR3: 0000000102484002 CR4: 0000000000770ef0 [ 15.006921] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 15.008509] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400 [ 15.010001] PKRU: 55555554 [ 15.010672] Call Trace: [ 15.011297] <TASK> [ 15.011868] ? die+0x32/0x80 [ 15.012572] ? do_trap+0xd9/0x100 [ 15.013306] ? bio_split+0xd5/0xf0 [ 15.014051] ? do_error_trap+0x6a/0x90 [ 15.014854] ? bio_split+0xd5/0xf0 [ 15.015597] ? exc_invalid_op+0x4c/0x60 [ 15.016419] ? bio_split+0xd5/0xf0 [ 15.017113] ? asm_exc_invalid_op+0x16/0x20 [ 15.017932] ? bio_split+0xd5/0xf0 [ 15.018624] __bio_split_to_limits+0x90/0x2d0 [ 15.019474] blk_mq_submit_bio+0x111/0x6a0 [ 15.020280] ? kmem_cache_alloc+0x254/0x2e0 [ 15.021040] submit_bio_noacct_nocheck+0x2f1/0x3d0 [ 15.021893] ? submit_bio_noacct+0x42/0x5b0 [ 15.022658] block_read_full_folio+0x2b7/0x350 [ 15.023457] ? __pfx_blkdev_get_block+0x10/0x10 [ 15.024284] ? __pfx_blkdev_read_folio+0x10/0x10 [ 15.025073] ? __pfx_blkdev_read_folio+0x10/0x10 [ 15.025851] filemap_read_folio+0x32/0xb0 [ 15.026540] do_read_cache_folio+0x108/0x200 [ 15.027271] ? __pfx_adfspart_check_ICS+0x10/0x10 [ 15.028066] read_part_sector+0x32/0xe0 [ 15.028701] adfspart_check_ICS+0x32/0x480 [ 15.029334] ? snprintf+0x49/0x70 [ 15.029875] ? __pfx_adfspart_check_ICS+0x10/0x10 [ 15.030592] bdev_disk_changed+0x2a2/0x6e0 [ 15.031226] blkdev_get_whole+0x5f/0xa0 [ 15.031827] bdev_open+0x201/0x3c0 [ 15.032360] bdev_file_open_by_dev+0xb5/0x110 [ 15.032990] disk_scan_partitions+0x65/0xe0 [ 15.033598] device_add_disk+0x3e0/0x3f0 [ 15.034172] nvme_scan_ns+0x5f0/0xe50 [nvme_core] [ 15.034862] nvme_scan_work+0x26f/0x5a0 [nvme_core] [ 15.035568] process_one_work+0x189/0x3b0
Quoted 9 lines
[ 15.036168] worker_thread+0x273/0x390 [ 15.036713] ? __pfx_worker_thread+0x10/0x10 [ 15.037312] kthread+0xda/0x110 [ 15.037779] ? __pfx_kthread+0x10/0x10 [ 15.038316] ret_from_fork+0x2d/0x50 [ 15.038829] ? __pfx_kthread+0x10/0x10 [ 15.039364] ret_from_fork_asm+0x1a/0x30 [ 15.039924] </TASK>
So, finally nailed it down.
Problem was that qemu is setting a default mdts of '7', which obviously
is too small for 1M sector sizes.
And the next problem was that we don't validate the 'max_hw_sectors' setting against the 'logical_sector_size' setting, letting the block layer happily accept limit which have an hw_sector_size smaller than
the logical block size.
For that I've meanwhile sent a patch, but if you want continue testing
you need to the 'mdts=0' for the qemu nvme emulation.
We _might_ want to send a patch for qemu to set mdts to '0' as the
default value, but that probably is too much hassle.
Cheers,
Hannes
Re: [PATCH 5/5] nvme: enable logical block size > PAGE_SIZE
In reply to Hannes ReineckeQuoted 7 lines · Hannes Reinecke
Quoted 4 lines · Luis Chamberlain
Quoted 1 line · Luis Chamberlain
I'll try next going above 512 KiB.
At 1 MiB NVMe LBA format we crash with the
BUG_ON(sectors <= 0)onbio_split().Ah.
MAX_BUFS_PER_PAGEgetting in the way.
That doesn't make sense. We will need something like this patch for filesystems which use large folios with buffer_heads. But for a 1MB block size device, we should have one buffer_head per folio.
Re: [PATCH 5/5] nvme: enable logical block size > PAGE_SIZE
In reply to Luis ChamberlainQuoted 63 lines · Luis Chamberlain
Quoted 12 lines · Luis Chamberlain
Quoted 11 lines · Matthew Wilcox
Quoted 10 lines · Luis Chamberlain
We can't just do this, we need to consider the actual nvme cap (test it, and if it crashes and below what the page cache supports, then we have to go below) and so to make the enablment easier. So we could just move this to helper [0]. Then when the bdev cache patch goes through the check for
CONFIG_BUFFER_HEADcan be removed, if this goes first.We crash if we go above 1 MiB today, we should be able to go up to 2 MiB but that requires some review to see what stupid thing is getting in the way.
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
This is overengineered garbage. What's the crash?
I had only tested it with iomap, I had not tested it with buffer-heads, and so it would require re-testing. It's Saturday 5pm, I should be doing something else other than being on my computer.
It crashes because we forgot two things in this series below, so the change below its us to enable to *at least* boot up to 64k LBA format on NVMe.
One can reproduce this with kdevops with:
make defconfig-lbs-xfs-bdev-nvme
make bringup
make linuxI've added another defconfig which bumps the LBA format up to 512 KiB to see if bootup blows up, that has another defconfig:
make lbs-xfs-bdev-large-nvme
make bringup
make linuxThat at least booted. Note that the above defconfigs use this thread's message ID, so it applies this series on top of the min order branch. The patch below is just needed.
I'll try next going above 512 KiB.
fs/buffer.c +0
-2@@ -2360,8 +2360,6 @@ int block_read_full_folio(struct folio *folio, get_block_t *get_block)if (IS_ENABLED (CONFIG_FS_VERITY ) &&IS_VERITY (inode)) limit = inode->i_sb->s_maxbytes; -VM_BUG_ON_FOLIO (folio_test_large (folio), folio); - head =folio_create_buffers (folio, inode,0 ); blocksize = head->b_size;fs/mpage.c +0
-0
fs/mpage.c static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) · line 183
Quoted 7 lines
fs/mpage.c +0
-1@@ -178,7 +178,6 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)gfp_t gfp =mapping_gfp_constraint (folio->mapping,GFP_KERNEL );/* MAX_BUF_PER_PAGE, for example */ -VM_BUG_ON_FOLIO (folio_test_large (folio), folio);if (args->is_readahead) { opf |=REQ_RAHEAD ;
Thanks. Will be including that in the next round.
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Re: [PATCH 5/5] nvme: enable logical block size > PAGE_SIZE
Quoted 59 lines · Hannes Reinecke
Quoted 58 lines · Luis Chamberlain
Quoted 12 lines · Luis Chamberlain
Quoted 11 lines · Matthew Wilcox
Quoted 10 lines · Luis Chamberlain
We can't just do this, we need to consider the actual nvme cap (test it, and if it crashes and below what the page cache supports, then we have to go below) and so to make the enablment easier. So we could just move this to helper [0]. Then when the bdev cache patch goes through the check for
CONFIG_BUFFER_HEADcan be removed, if this goes first.We crash if we go above 1 MiB today, we should be able to go up to 2 MiB but that requires some review to see what stupid thing is getting in the way.
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
This is overengineered garbage. What's the crash?
I had only tested it with iomap, I had not tested it with buffer-heads, and so it would require re-testing. It's Saturday 5pm, I should be doing something else other than being on my computer.
It crashes because we forgot two things in this series below, so the change below its us to enable to *at least* boot up to 64k LBA format on NVMe.
One can reproduce this with kdevops with:
make defconfig-lbs-xfs-bdev-nvme
make bringup
make linuxI've added another defconfig which bumps the LBA format up to 512 KiB to see if bootup blows up, that has another defconfig:
make lbs-xfs-bdev-large-nvme
make bringup
make linuxThat at least booted. Note that the above defconfigs use this thread's message ID, so it applies this series on top of the min order branch. The patch below is just needed.
I'll try next going above 512 KiB.
fs/buffer.c +0
-2@@ -2360,8 +2360,6 @@ int block_read_full_folio(struct folio *folio, get_block_t *get_block)if (IS_ENABLED (CONFIG_FS_VERITY ) &&IS_VERITY (inode)) limit = inode->i_sb->s_maxbytes; -VM_BUG_ON_FOLIO (folio_test_large (folio), folio); - head =folio_create_buffers (folio, inode,0 ); blocksize = head->b_size;fs/mpage.c +0
-0
Quoted 8 lines
fs/mpage.c static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) · line 182
Quoted 7 lines
fs/mpage.c +0
-1@@ -178,7 +178,6 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)gfp_t gfp =mapping_gfp_constraint (folio->mapping,GFP_KERNEL );/* MAX_BUF_PER_PAGE, for example */ -VM_BUG_ON_FOLIO (folio_test_large (folio), folio);if (args->is_readahead) { opf |=REQ_RAHEAD ;Thanks. Will be including that in the next round.
I gave it a spin with the above changes, results:
https://github.com/linux-kdevops/kdevops/commit/f2641efe7d2037892e0477fdc8daf66af59c1f01
https://github.com/linux-kdevops/kdevops-results-archive/commit/c3ac880e909a30aa2f9b24231d0f7297dfdf4e8e
Luis
Quoted 11 lines · On Fri, May 10, 2024 at 12:29:01PM +0200, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
Hi all,
based on the patch series from Pankaj '[PATCH v5 00/11] enable bs > ps in XFS' it's now quite simple to enable support for block devices with block sizes larger than page size even without having to disable
CONFIG_BUFFER_HEAD. The patchset really is just two rather trivial patches to fs/mpage, and two patches to remove hardcoded restrictions on the block size.As usual, comments and reviews are welcome.
What's the test plan?
Luis
Quoted 18 lines · On Fri, May 10, 2024 at 12:29:01PM +0200, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
Hi all,
based on the patch series from Pankaj '[PATCH v5 00/11] enable bs > ps in XFS' it's now quite simple to enable support for block devices with block sizes larger than page size even without having to disable
CONFIG_BUFFER_HEAD. The patchset really is just two rather trivial patches to fs/mpage, and two patches to remove hardcoded restrictions on the block size.As usual, comments and reviews are welcome.
Hannes Reinecke (4):
fs/mpage: useblocks_per_folioinstead ofblocks_per_pagefs/mpage: avoid negative shift for large blocksize block/bdev: lift restrictions on supported blocksize block/bdev: enable large folio support for large logical block sizes
Quoted 6 lines
Pankaj Raghav (1):
nvme: enable logical block size >PAGE_SIZEblock/bdev.c | 10 ++++++--- drivers/nvme/host/core.c | 7 +++---- fs/mpage.c | 44 ++++++++++++++++++++--------------------
What about fs/buffer.c? Do we need some changes there?
One obvious place I see is:
fs/buffer.c +1 -1
@@ -1121,7 +1121,7 @@ __getblk_slow(struct block_device *bdev, sector_t block, {/* Size must be multiple of hard sectorsize */ if (unlikely (size & (bdev_logical_block_size (bdev)-1 ) || - (size <512 || size >PAGE_SIZE ))) { + (size <512 ))) {printk (KERN_ERR "getblk(): invalid block size %d requested\n" , size); printk(KERN_ERR "logical block size: %d\n",