diff options
| author | Matthew Wilcox (Oracle) <willy@infradead.org> | 2024-01-01 09:38:48 +0000 |
|---|---|---|
| committer | Andrew Morton <akpm@linux-foundation.org> | 2024-01-05 10:17:43 -0800 |
| commit | bcd30d4cd937e8e15c3986358c5e601135475ce1 (patch) | |
| tree | 5627fe36447141833d470f0f480ff819dafeffe4 | |
| parent | 501a06fe8e4c185bbda371b8cedbdf1b23a633d8 (diff) | |
buffer: fix unintended successful return
If try_to_free_buffers() succeeded and then folio_alloc_buffers() failed,
grow_dev_folio() would return success. This would be incorrect; memory
allocation failure is supposed to result in a failure. It's a harmless
bug; the caller will simply go around the loop one more time and
grow_dev_folio() will correctly return a failure that time. But it was an
unintended change and looks like a more serious bug than it is.
While I'm in here, improve the commentary about why we return success even
though we failed.
Link: https://lkml.kernel.org/r/20240101093848.2017115-1-willy@infradead.org
Fixes: 6d840a18773f ("buffer: return bool from grow_dev_folio()")
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Reported-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
| -rw-r--r-- | fs/buffer.c | 17 |
1 files changed, 12 insertions, 5 deletions
diff --git a/fs/buffer.c b/fs/buffer.c index 19548369bc6c..5c29850e4781 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -1028,8 +1028,8 @@ static sector_t folio_init_buffers(struct folio *folio, * * This is used purely for blockdev mappings. * - * Returns false if we have a 'permanent' failure. Returns true if - * we succeeded, or the caller should retry. + * Returns false if we have a failure which cannot be cured by retrying + * without sleeping. Returns true if we succeeded, or the caller should retry. */ static bool grow_dev_folio(struct block_device *bdev, sector_t block, pgoff_t index, unsigned size, gfp_t gfp) @@ -1051,10 +1051,17 @@ static bool grow_dev_folio(struct block_device *bdev, sector_t block, goto unlock; } - /* Caller should retry if this call fails */ - end_block = ~0ULL; - if (!try_to_free_buffers(folio)) + /* + * Retrying may succeed; for example the folio may finish + * writeback, or buffers may be cleaned. This should not + * happen very often; maybe we have old buffers attached to + * this blockdev's page cache and we're trying to change + * the block size? + */ + if (!try_to_free_buffers(folio)) { + end_block = ~0ULL; goto unlock; + } } bh = folio_alloc_buffers(folio, size, gfp | __GFP_ACCOUNT); |