Commit 81f4c03b authored by Matthew Wilcox (Oracle)'s avatar Matthew Wilcox (Oracle)
Browse files

filemap: Drop the refcount while waiting for page lock 



Commit bd8a1f36 ("mm/filemap: support readpage splitting a page")
changed the read_iter path to drop the refcount while waiting for the
page lock.  However, it missed the same pattern in read_mapping_page()
and friends.  Use the same pattern in do_read_cache_folio() that is
used in filemap_update_page().

Signed-off-by: default avatarMatthew Wilcox (Oracle) <willy@infradead.org>
Reviewed-by: default avatarWilliam Kucharski <william.kucharski@oracle.com>
parent 539a3322

mm/filemap.c

+5 −38
Original line number Diff line number Diff line
@@ -3460,45 +3460,12 @@ static struct folio *do_read_cache_folio(struct address_space *mapping, 
	if (folio_test_uptodate(folio))

		goto out;



	/*

	 * Page is not up to date and may be locked due to one of the following

	 * case a: Page is being filled and the page lock is held

	 * case b: Read/write error clearing the page uptodate status

	 * case c: Truncation in progress (page locked)

	 * case d: Reclaim in progress

	 *

	 * Case a, the page will be up to date when the page is unlocked.

	 *    There is no need to serialise on the page lock here as the page

	 *    is pinned so the lock gives no additional protection. Even if the

	 *    page is truncated, the data is still valid if PageUptodate as

	 *    it's a race vs truncate race.

	 * Case b, the page will not be up to date

	 * Case c, the page may be truncated but in itself, the data may still

	 *    be valid after IO completes as it's a read vs truncate race. The

	 *    operation must restart if the page is not uptodate on unlock but

	 *    otherwise serialising on page lock to stabilise the mapping gives

	 *    no additional guarantees to the caller as the page lock is

	 *    released before return.

	 * Case d, similar to truncation. If reclaim holds the page lock, it

	 *    will be a race with remove_mapping that determines if the mapping

	 *    is valid on unlock but otherwise the data is valid and there is

	 *    no need to serialise with page lock.

	 *

	 * As the page lock gives no additional guarantee, we optimistically

	 * wait on the page to be unlocked and check if it's up to date and

	 * use the page if it is. Otherwise, the page lock is required to

	 * distinguish between the different cases. The motivation is that we

	 * avoid spurious serialisations and wakeups when multiple processes

	 * wait on the same page for IO to complete.

	 */

	folio_wait_locked(folio);

	if (folio_test_uptodate(folio))

		goto out;



	/* Distinguish between all the cases under the safety of the lock */

	folio_lock(folio);

	if (!folio_trylock(folio)) {

		folio_put_wait_locked(folio, TASK_UNINTERRUPTIBLE);

		goto repeat;

	}



	/* Case c or d, restart the operation */

	/* Folio was truncated from mapping */

	if (!folio->mapping) {

		folio_unlock(folio);

		folio_put(folio);