| Message ID | 20250204231209.429356-2-mcgrof@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | enable bs > ps for block devices | expand |
On 2/5/25 00:12, Luis Chamberlain wrote: > When we read over all buffers in a folio we currently use the > buffer index on the folio and blocksize to get the offset. Simplify > this with bh_offset(). This simplifies the loop while making no > functional changes. > > Suggested-by: Matthew Wilcox <willy@infradead.org> > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> > --- > fs/buffer.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/fs/buffer.c b/fs/buffer.c > index cc8452f60251..b99560e8a142 100644 > --- a/fs/buffer.c > +++ b/fs/buffer.c > @@ -2381,7 +2381,6 @@ int block_read_full_folio(struct folio *folio, get_block_t *get_block) > lblock = div_u64(limit + blocksize - 1, blocksize); > bh = head; > nr = 0; > - i = 0; > > do { > if (buffer_uptodate(bh)) > @@ -2398,7 +2397,7 @@ int block_read_full_folio(struct folio *folio, get_block_t *get_block) > page_error = true; > } > if (!buffer_mapped(bh)) { > - folio_zero_range(folio, i * blocksize, > + folio_zero_range(folio, bh_offset(bh), > blocksize); > if (!err) > set_buffer_uptodate(bh); > @@ -2412,7 +2411,7 @@ 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 (iblock++, (bh = bh->b_this_page) != head); > > if (fully_mapped) > folio_set_mappedtodisk(folio); One wonders: shouldn't we use plugging here to make I/O more efficient? Cheers, Hannes
On Wed, Feb 05, 2025 at 05:18:20PM +0100, Hannes Reinecke wrote:
> One wonders: shouldn't we use plugging here to make I/O more efficient?
Should we plug at a higher level?
Opposite question: What if getblk() needs to do a read (ie ext2 indirect
block)?
On 2/5/25 23:03, Matthew Wilcox wrote: > On Wed, Feb 05, 2025 at 05:18:20PM +0100, Hannes Reinecke wrote: >> One wonders: shouldn't we use plugging here to make I/O more efficient? > > Should we plug at a higher level? > > Opposite question: What if getblk() needs to do a read (ie ext2 indirect > block)? Ah, that. Yes, plugging on higher level would be a good idea. (And can we check for nested plugs? _Should_ we check for nested plugs?) Cheers, Hannes
On Thu, Feb 06, 2025 at 08:17:32AM +0100, Hannes Reinecke wrote: > On 2/5/25 23:03, Matthew Wilcox wrote: > > On Wed, Feb 05, 2025 at 05:18:20PM +0100, Hannes Reinecke wrote: > > > One wonders: shouldn't we use plugging here to make I/O more efficient? > > > > Should we plug at a higher level? > > > > Opposite question: What if getblk() needs to do a read (ie ext2 indirect > > block)? > > Ah, that. Yes, plugging on higher level would be a good idea. > (And can we check for nested plugs? _Should_ we check for nested plugs?) I think given the discussion less is more for now, and if we really want this we can add it later. Thoughts? Luis
On 2/6/25 18:30, Luis Chamberlain wrote: > On Thu, Feb 06, 2025 at 08:17:32AM +0100, Hannes Reinecke wrote: >> On 2/5/25 23:03, Matthew Wilcox wrote: >>> On Wed, Feb 05, 2025 at 05:18:20PM +0100, Hannes Reinecke wrote: >>>> One wonders: shouldn't we use plugging here to make I/O more efficient? >>> >>> Should we plug at a higher level? >>> >>> Opposite question: What if getblk() needs to do a read (ie ext2 indirect >>> block)? >> >> Ah, that. Yes, plugging on higher level would be a good idea. >> (And can we check for nested plugs? _Should_ we check for nested plugs?) > > I think given the discussion less is more for now, and if we really want > this we can add it later. Thoughts? > Yeah, go for it. Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
diff --git a/fs/buffer.c b/fs/buffer.c index cc8452f60251..b99560e8a142 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -2381,7 +2381,6 @@ int block_read_full_folio(struct folio *folio, get_block_t *get_block) lblock = div_u64(limit + blocksize - 1, blocksize); bh = head; nr = 0; - i = 0; do { if (buffer_uptodate(bh)) @@ -2398,7 +2397,7 @@ int block_read_full_folio(struct folio *folio, get_block_t *get_block) page_error = true; } if (!buffer_mapped(bh)) { - folio_zero_range(folio, i * blocksize, + folio_zero_range(folio, bh_offset(bh), blocksize); if (!err) set_buffer_uptodate(bh); @@ -2412,7 +2411,7 @@ 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 (iblock++, (bh = bh->b_this_page) != head); if (fully_mapped) folio_set_mappedtodisk(folio);
When we read over all buffers in a folio we currently use the buffer index on the folio and blocksize to get the offset. Simplify this with bh_offset(). This simplifies the loop while making no functional changes. Suggested-by: Matthew Wilcox <willy@infradead.org> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> --- fs/buffer.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)