| Message ID | 20230918110510.66470-10-hare@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: update buffer_head for Large-block I/O | expand |
On Mon, Sep 18, 2023 at 01:05:01PM +0200, Hannes Reinecke wrote: > Use the correct mapping order in grow_dev_page() to ensure folios > are created with the correct order. I see why you did this, but I think it's fragile. __filemap_get_folio() will happily decrease 'order' if memory allocation fails. I think __filemap_get_folio() needs to become aware of the minimum folio order for this mapping, and then we don't need this patch. Overall, I like bits of this patchset and I like bits of Pankaj's ;-)
On 9/18/23 16:00, Matthew Wilcox wrote: > On Mon, Sep 18, 2023 at 01:05:01PM +0200, Hannes Reinecke wrote: >> Use the correct mapping order in grow_dev_page() to ensure folios >> are created with the correct order. > > I see why you did this, but I think it's fragile. __filemap_get_folio() > will happily decrease 'order' if memory allocation fails. I think > __filemap_get_folio() needs to become aware of the minimum folio > order for this mapping, and then we don't need this patch. > > Overall, I like bits of this patchset and I like bits of Pankaj's ;-) To be expected. It's basically parallel development, me and Pankaj working independently and arriving at different patchsets. Will see next week at ALPSS if we can merge them into something sensible. And 'grow_dev_page()' was really done by audit, and I'm not sure if my tests even exercised this particular codepath. So yeah, I'm with you. Cheers, Hannes
diff --git a/fs/buffer.c b/fs/buffer.c index 66895432d91f..a219b79c57ad 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -1044,6 +1044,7 @@ grow_dev_page(struct block_device *bdev, sector_t block, sector_t end_block; int ret = 0; gfp_t gfp_mask; + fgf_t fgf = FGP_LOCK | FGP_ACCESSED | FGP_CREAT; gfp_mask = mapping_gfp_constraint(inode->i_mapping, ~__GFP_FS) | gfp; @@ -1054,9 +1055,8 @@ grow_dev_page(struct block_device *bdev, sector_t block, * code knows what it's doing. */ gfp_mask |= __GFP_NOFAIL; - - folio = __filemap_get_folio(inode->i_mapping, index, - FGP_LOCK | FGP_ACCESSED | FGP_CREAT, gfp_mask); + fgf |= fgf_set_order(mapping_min_folio_order(inode->i_mapping)); + folio = __filemap_get_folio(inode->i_mapping, index, fgf, gfp_mask); bh = folio_buffers(folio); if (bh) {
Use the correct mapping order in grow_dev_page() to ensure folios are created with the correct order. Signed-off-by: Hannes Reinecke <hare@suse.de> --- fs/buffer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)