Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Xianting Tian <xianting_tian@126.com>
To:     linux-kernel@vger.kernel.org
Subject: [PATCH] mm/filemap.c: clear page error before actual read
Date:   Mon,  2 Mar 2020 10:14:09 -0500
Message-Id: <1583162049-17623-1-git-send-email-xianting_tian@126.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Mount failure issue happens under the circumstances:
Application totally forked 38 threads to mount 38 cramfs images in docker,
but several mounts failed with high probability.
Mount failed due to the checking result of the page(read from the superblock
of loop dev) is not uptodate after wait_on_page_locked(page) returned in
function cramfs_read:
wait_on_page_locked(page);
if (!PageUptodate(page)) {
	...
}

The reason of the checking result of the page not uptodate:
systemd-udevd read the loopX dev before mount, because the status of loopX
is Lo_unbound at this time, so loop_make_request directly trigger the calling
of io_end handler end_buffer_async_read, which called SetPageError(page). So It
caused the page can't be set to uptodate in function end_buffer_saync_read:
if(page_uptodate && !PageError(page)) {
	SetPageUptodate(page);
}
Then mount operation is performed, it used the same page which is just used by
systemd-udevd above, Because this page is not uptodate, it will launch a actual
read via submit_bh, then wait on this page by calling wait_on_page_locked(page).
When the I/O of the page done, io_end handler end_buffer_async_read is called,
because no one cleared the page error(during the whole read path), which is caused
by systemd-udevd, so this page is still in "PageError" staus, which is can't
be set to uptodate in function end_buffer_async_read, then caused mount failure.

But sometimes mount succeed even through systemd-udeved read loop dev just before,
The reason is systemd-udevd launched other loopX read just between step 3.1 and 3.2,
the steps as below:
1, loopX dev default status is Lo_unbound;
2, systemd-udved read loopX dev (page is set to PageError);
3, mount operation
   1) set loopX status to Lo_bound;
   ==>systemd-udevd read loopX dev<==
   2) read loopX dev(page has no error)
   3) mount succeed
As the loopX dev status is set to Lo_bound after step 3.1, so the other loopX dev read
by systemd-udevd will go through the whole I/O stack, part of the call trace as below:
   SYS_read
      vfs_read
          do_sync_read
              blkdev_aio_read
                 generic_file_aio_read
                     do_generic_file_read
                         ClearPageError(page);mapping->a_ops->readpage(filp, page)
here, mapping->a_ops_readpage() is blkdev_readpage.
We can see the ClearPageError(page) is called, then the read in step 3.2 succeed.

The fix is to add ClearPageError just before the actual read of other read path.

Signed-off-by: Xianting Tian <xianting_tian@126.com>
---
 mm/filemap.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/filemap.c b/mm/filemap.c
index 1784478..c2cf0fd 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2755,6 +2755,7 @@ static struct page *do_read_cache_page(struct address_space *mapping,
 		}
 
 filler:
+		ClearPageError(page);
 		if (filler)
 			err = filler(data, page);
 		else
-- 
1.8.3.1