Whamcloud - gitweb
LU-16412 llite: check truncated page in ->readpage() 33/49433/6
authorQian Yingjin <qian@ddn.com>
Mon, 19 Dec 2022 06:57:39 +0000 (01:57 -0500)
committerOleg Drokin <green@whamcloud.com>
Tue, 31 Jan 2023 02:34:23 +0000 (02:34 +0000)
The page end offset calculation in filemap_get_read_batch() was
off by one. This bug was introduced in commit v5.11-10234-gcbd59c48ae
("mm/filemap: use head pages in generic_file_buffered_read")

When a read is submitted with end offset 1048575, it calculates
the end page index for read of 256 where it should be 255. This
results in the readpage() call for the page with index 256 is over
stripe boundary and may not be covered by a DLM extent lock.

This happens in a corner race case: filemap_get_read_batch()
batches the page with index 256 for read, but later this page is
removed from page cache due to the lock protected it being revoked,
but has a reference count due to the batch.  This results in this
page in the read path is not covered by any DLM lock.

The solution is simple. We can check whether the page was
truncated and removed from page cache in ->readpage() by the
address_sapce pointer of the page. If it was truncated, return
AOP_TRUNCATED_PAGE to the upper caller.  This will cause the
kernel to retry to batch pages and the truncated page will not
be added as it was already removed from page cache of the file.

Add sanityn/test_95 to verify it.

Test-Parameters: testlist=sanityn env=ONLY=95 clientdistro=ubuntu2204
Signed-off-by: Qian Yingjin <qian@ddn.com>
Change-Id: I192df92b1d1b79057055430cc81cb7cc760cc9ed
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/49433
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Patrick Farrell <pfarrell@whamcloud.com>
Reviewed-by: Zhenyu Xu <bobijam@hotmail.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/include/obd_support.h
lustre/llite/rw.c
lustre/llite/rw26.c
lustre/tests/sanityn.sh

index 12c8747..395118e 100644 (file)
@@ -620,6 +620,8 @@ extern char obd_jobid_var[];
 #define OBD_FAIL_LLITE_PAGE_ALLOC                  0x1418
 #define OBD_FAIL_LLITE_OPEN_DELAY                  0x1419
 #define OBD_FAIL_LLITE_XATTR_PAUSE                 0x1420
+#define OBD_FAIL_LLITE_PAGE_INVALIDATE_PAUSE       0x1421
+#define OBD_FAIL_LLITE_READPAGE_PAUSE              0x1422
 
 #define OBD_FAIL_FID_INDIR     0x1501
 #define OBD_FAIL_FID_INLMA     0x1502
index 49f174f..aaf7433 100644 (file)
@@ -1865,6 +1865,41 @@ int ll_readpage(struct file *file, struct page *vmpage)
        int result;
        ENTRY;
 
+       if (OBD_FAIL_PRECHECK(OBD_FAIL_LLITE_READPAGE_PAUSE)) {
+               unlock_page(vmpage);
+               OBD_FAIL_TIMEOUT(OBD_FAIL_LLITE_READPAGE_PAUSE, cfs_fail_val);
+               lock_page(vmpage);
+       }
+
+       /*
+        * The @vmpage got truncated.
+        * This is a kernel bug introduced since kernel 5.12:
+        * comment: cbd59c48ae2bcadc4a7599c29cf32fd3f9b78251
+        * ("mm/filemap: use head pages in generic_file_buffered_read")
+        *
+        * The page end offset calculation in filemap_get_read_batch() was off
+        * by one.  When a read is submitted with end offset 1048575, then it
+        * calculates the end page for read of 256 where it should be 255. This
+        * results in the readpage() for the page with index 256 is over stripe
+        * boundary and may not covered by a DLM extent lock.
+        *
+        * This happens in a corner race case: filemap_get_read_batch() adds
+        * the page with index 256 for read which is not in the current read
+        * I/O context, and this page is being invalidated and will be removed
+        * from page cache due to the lock protected it being revoken. This
+        * results in this page in the read path not covered by any DLM lock.
+        *
+        * The solution is simple. Check whether the page was truncated in
+        * ->readpage(). If so, just return AOP_TRUNCATED_PAGE to the upper
+        * caller. Then the kernel will retry to batch pages, and it will not
+        * add the truncated page into batches as it was removed from page
+        * cache of the file.
+        */
+       if (vmpage->mapping != inode->i_mapping) {
+               unlock_page(vmpage);
+               RETURN(AOP_TRUNCATED_PAGE);
+       }
+
        lcc = ll_cl_find(inode);
        if (lcc != NULL) {
                env = lcc->lcc_env;
index 1c5a88a..c07c321 100644 (file)
@@ -156,6 +156,13 @@ static void ll_invalidatepage(struct page *vmpage,
 
                cl_env_percpu_put(env);
        }
+
+       if (OBD_FAIL_PRECHECK(OBD_FAIL_LLITE_PAGE_INVALIDATE_PAUSE)) {
+               unlock_page(vmpage);
+               OBD_FAIL_TIMEOUT(OBD_FAIL_LLITE_PAGE_INVALIDATE_PAUSE,
+                                cfs_fail_val);
+               lock_page(vmpage);
+       }
 }
 #endif
 
index c324b60..5cabe65 100755 (executable)
@@ -5215,6 +5215,38 @@ test_94() {
 }
 run_test 94 "signal vs CP callback race"
 
+test_95() {
+       local file=$DIR/$tfile
+       local file2=$DIR2/$tfile
+       local fast_read_save
+       local pid
+
+       fast_read_save=$($LCTL get_param -n llite.*.fast_read | head -n 1)
+       [ -z "$fast_read_save" ] && skip "no fast read support"
+
+       stack_trap "$LCTL set_param llite.*.fast_read=$fast_read_save" EXIT
+       $LCTL set_param llite.*.fast_read=0
+
+       $LFS setstripe -c $OSTCOUNT $file || error "failed to setstripe $file"
+       dd if=/dev/zero of=$file bs=1M count=2 || error "failed to write $file"
+       cancel_lru_locks $OSC
+       $MULTIOP $file Oz1048576w4096c || error "failed to write $file"
+       $MULTIOP $file oz1044480r4096c || error "failed to read $file"
+
+       # OBD_FAIL_LLITE_PAGE_INVALIDATE_PAUSE  0x1421
+       $LCTL set_param fail_loc=0x80001421 fail_val=7
+       $MULTIOP $file2 Oz1048576w4096_c &
+       pid=$!
+
+       sleep 2
+       # OBD_FAIL_LLITE_READPAGE_PAUSE         0x1422
+       $LCTL set_param fail_loc=0x80001422 fail_val=10
+       $MULTIOP $file oz1044480r4096c || error "failed to read $file"
+
+       kill -USR1 $pid && wait $pid || error "wait for PID $pid failed"
+}
+run_test 95 "Check readpage() on a page that was removed from page cache"
+
 # Data-on-MDT tests
 test_100a() {
        skip "Reserved for glimpse-ahead" && return