| 1 |
From 67f4b5dc49913abcdb5cc736e73674e2f352f81d Mon Sep 17 00:00:00 2001 |
| 2 |
From: Trond Myklebust <trond.myklebust@hammerspace.com> |
| 3 |
Date: Sat, 13 Aug 2022 08:22:25 -0400 |
| 4 |
Subject: NFS: Fix another fsync() issue after a server reboot |
| 5 |
|
| 6 |
From: Trond Myklebust <trond.myklebust@hammerspace.com> |
| 7 |
|
| 8 |
commit 67f4b5dc49913abcdb5cc736e73674e2f352f81d upstream. |
| 9 |
|
| 10 |
Currently, when the writeback code detects a server reboot, it redirties |
| 11 |
any pages that were not committed to disk, and it sets the flag |
| 12 |
NFS_CONTEXT_RESEND_WRITES in the nfs_open_context of the file descriptor |
| 13 |
that dirtied the file. While this allows the file descriptor in question |
| 14 |
to redrive its own writes, it violates the fsync() requirement that we |
| 15 |
should be synchronising all writes to disk. |
| 16 |
While the problem is infrequent, we do see corner cases where an |
| 17 |
untimely server reboot causes the fsync() call to abandon its attempt to |
| 18 |
sync data to disk and causing data corruption issues due to missed error |
| 19 |
conditions or similar. |
| 20 |
|
| 21 |
In order to tighted up the client's ability to deal with this situation |
| 22 |
without introducing livelocks, add a counter that records the number of |
| 23 |
times pages are redirtied due to a server reboot-like condition, and use |
| 24 |
that in fsync() to redrive the sync to disk. |
| 25 |
|
| 26 |
Fixes: 2197e9b06c22 ("NFS: Fix up fsync() when the server rebooted") |
| 27 |
Cc: stable@vger.kernel.org |
| 28 |
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> |
| 29 |
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| 30 |
--- |
| 31 |
fs/nfs/file.c | 15 ++++++--------- |
| 32 |
fs/nfs/inode.c | 1 + |
| 33 |
fs/nfs/write.c | 6 ++++-- |
| 34 |
include/linux/nfs_fs.h | 1 + |
| 35 |
4 files changed, 12 insertions(+), 11 deletions(-) |
| 36 |
|
| 37 |
--- a/fs/nfs/file.c |
| 38 |
+++ b/fs/nfs/file.c |
| 39 |
@@ -221,8 +221,10 @@ nfs_file_fsync_commit(struct file *file, |
| 40 |
int |
| 41 |
nfs_file_fsync(struct file *file, loff_t start, loff_t end, int datasync) |
| 42 |
{ |
| 43 |
- struct nfs_open_context *ctx = nfs_file_open_context(file); |
| 44 |
struct inode *inode = file_inode(file); |
| 45 |
+ struct nfs_inode *nfsi = NFS_I(inode); |
| 46 |
+ long save_nredirtied = atomic_long_read(&nfsi->redirtied_pages); |
| 47 |
+ long nredirtied; |
| 48 |
int ret; |
| 49 |
|
| 50 |
trace_nfs_fsync_enter(inode); |
| 51 |
@@ -237,15 +239,10 @@ nfs_file_fsync(struct file *file, loff_t |
| 52 |
ret = pnfs_sync_inode(inode, !!datasync); |
| 53 |
if (ret != 0) |
| 54 |
break; |
| 55 |
- if (!test_and_clear_bit(NFS_CONTEXT_RESEND_WRITES, &ctx->flags)) |
| 56 |
+ nredirtied = atomic_long_read(&nfsi->redirtied_pages); |
| 57 |
+ if (nredirtied == save_nredirtied) |
| 58 |
break; |
| 59 |
- /* |
| 60 |
- * If nfs_file_fsync_commit detected a server reboot, then |
| 61 |
- * resend all dirty pages that might have been covered by |
| 62 |
- * the NFS_CONTEXT_RESEND_WRITES flag |
| 63 |
- */ |
| 64 |
- start = 0; |
| 65 |
- end = LLONG_MAX; |
| 66 |
+ save_nredirtied = nredirtied; |
| 67 |
} |
| 68 |
|
| 69 |
trace_nfs_fsync_exit(inode, ret); |
| 70 |
--- a/fs/nfs/inode.c |
| 71 |
+++ b/fs/nfs/inode.c |
| 72 |
@@ -426,6 +426,7 @@ nfs_ilookup(struct super_block *sb, stru |
| 73 |
static void nfs_inode_init_regular(struct nfs_inode *nfsi) |
| 74 |
{ |
| 75 |
atomic_long_set(&nfsi->nrequests, 0); |
| 76 |
+ atomic_long_set(&nfsi->redirtied_pages, 0); |
| 77 |
INIT_LIST_HEAD(&nfsi->commit_info.list); |
| 78 |
atomic_long_set(&nfsi->commit_info.ncommit, 0); |
| 79 |
atomic_set(&nfsi->commit_info.rpcs_out, 0); |
| 80 |
--- a/fs/nfs/write.c |
| 81 |
+++ b/fs/nfs/write.c |
| 82 |
@@ -1419,10 +1419,12 @@ static void nfs_initiate_write(struct nf |
| 83 |
*/ |
| 84 |
static void nfs_redirty_request(struct nfs_page *req) |
| 85 |
{ |
| 86 |
+ struct nfs_inode *nfsi = NFS_I(page_file_mapping(req->wb_page)->host); |
| 87 |
+ |
| 88 |
/* Bump the transmission count */ |
| 89 |
req->wb_nio++; |
| 90 |
nfs_mark_request_dirty(req); |
| 91 |
- set_bit(NFS_CONTEXT_RESEND_WRITES, &nfs_req_openctx(req)->flags); |
| 92 |
+ atomic_long_inc(&nfsi->redirtied_pages); |
| 93 |
nfs_end_page_writeback(req); |
| 94 |
nfs_release_request(req); |
| 95 |
} |
| 96 |
@@ -1892,7 +1894,7 @@ static void nfs_commit_release_pages(str |
| 97 |
/* We have a mismatch. Write the page again */ |
| 98 |
dprintk_cont(" mismatch\n"); |
| 99 |
nfs_mark_request_dirty(req); |
| 100 |
- set_bit(NFS_CONTEXT_RESEND_WRITES, &nfs_req_openctx(req)->flags); |
| 101 |
+ atomic_long_inc(&NFS_I(data->inode)->redirtied_pages); |
| 102 |
next: |
| 103 |
nfs_unlock_and_release_request(req); |
| 104 |
/* Latency breaker */ |
| 105 |
--- a/include/linux/nfs_fs.h |
| 106 |
+++ b/include/linux/nfs_fs.h |
| 107 |
@@ -182,6 +182,7 @@ struct nfs_inode { |
| 108 |
/* Regular file */ |
| 109 |
struct { |
| 110 |
atomic_long_t nrequests; |
| 111 |
+ atomic_long_t redirtied_pages; |
| 112 |
struct nfs_mds_commit_info commit_info; |
| 113 |
struct mutex commit_mutex; |
| 114 |
}; |
Parent Directory
| 
Revision Log