filemap: Drop the refcount while waiting for page lock

Commit bd8a1f3655a7 ("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: Matthew Wilcox (Oracle) <willy@infradead.org>
Reviewed-by: William Kucharski <william.kucharski@oracle.com>

diff --git a/mm/filemap.c b/mm/filemap.c
index f98e084ffb31bbcf16380d65db8cf8afde720104..38f16acb893689d51910c65ab674d36396894b78 100644 (file)
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -3460,45 +3460,12 @@ filler:
        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);