Commit bcd30d4c authored by Matthew Wilcox (Oracle)'s avatar Matthew Wilcox (Oracle) Committed by Andrew Morton
Browse files

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: 6d840a18 ("buffer: return bool from grow_dev_folio()")
Signed-off-by: default avatarMatthew Wilcox (Oracle) <willy@infradead.org>
Reported-by: default avatarRyusuke Konishi <konishi.ryusuke@gmail.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
parent 501a06fe

fs/buffer.c

+12 −5
Original line number Diff line number Diff line
@@ -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,11 +1051,18 @@ static bool grow_dev_folio(struct block_device *bdev, sector_t block, 
			goto unlock;

		}



		/* Caller should retry if this call fails */

		/*

		 * 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;

		if (!try_to_free_buffers(folio))

			goto unlock;

		}

	}



	bh = folio_alloc_buffers(folio, size, gfp | __GFP_ACCOUNT);

	if (!bh)