2017-07-12 23:11:05

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v2 0/4] pNFS/NFS COMMIT fixups

v2: Fix a bug in "NFS: Fix another COMMIT race in pNFS"

Trond Myklebust (4):
NFS: Fix a COMMIT race in pNFS
NFS: Fix another COMMIT race in pNFS
pNFS/flexfiles: Handle expired layout segments in
ff_layout_initiate_commit()
Revert commit 722f0b891198 ("pNFS: Don't send COMMITs to the DSes
if...")

fs/nfs/flexfilelayout/flexfilelayout.c | 4 ++++
fs/nfs/pnfs_nfs.c | 24 +++++++++++++-----------
2 files changed, 17 insertions(+), 11 deletions(-)

--
2.13.0



2017-07-12 23:11:07

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v2 2/4] NFS: Fix another COMMIT race in pNFS

We must make sure that cinfo->ds->ncommitting is in sync with the
commit list, since it is checked as part of pnfs_commit_list().

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/pnfs_nfs.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c
index 3e6de85faf42..7ceb86627e54 100644
--- a/fs/nfs/pnfs_nfs.c
+++ b/fs/nfs/pnfs_nfs.c
@@ -187,6 +187,7 @@ static void pnfs_generic_retry_commit(struct nfs_commit_info *cinfo, int idx)
struct pnfs_ds_commit_info *fl_cinfo = cinfo->ds;
struct pnfs_commit_bucket *bucket;
struct pnfs_layout_segment *freeme;
+ struct list_head *pos;
LIST_HEAD(pages);
int i;

@@ -197,6 +198,8 @@ static void pnfs_generic_retry_commit(struct nfs_commit_info *cinfo, int idx)
continue;
freeme = bucket->clseg;
bucket->clseg = NULL;
+ list_for_each(pos, &bucket->committing)
+ cinfo->ds->ncommitting--;
list_splice_init(&bucket->committing, &pages);
spin_unlock(&cinfo->inode->i_lock);
nfs_retry_commit(&pages, freeme, cinfo, i);
@@ -247,9 +250,12 @@ void pnfs_fetch_commit_bucket_list(struct list_head *pages,
struct nfs_commit_info *cinfo)
{
struct pnfs_commit_bucket *bucket;
+ struct list_head *pos;

bucket = &cinfo->ds->buckets[data->ds_commit_index];
spin_lock(&cinfo->inode->i_lock);
+ list_for_each(pos, &bucket->committing)
+ cinfo->ds->ncommitting--;
list_splice_init(&bucket->committing, pages);
data->lseg = bucket->clseg;
bucket->clseg = NULL;
@@ -334,7 +340,6 @@ pnfs_generic_commit_pagelist(struct inode *inode, struct list_head *mds_pages,
}
}
out:
- cinfo->ds->ncommitting = 0;
return PNFS_ATTEMPTED;
}
EXPORT_SYMBOL_GPL(pnfs_generic_commit_pagelist);
--
2.13.0


2017-07-12 23:11:09

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v2 4/4] Revert commit 722f0b891198 ("pNFS: Don't send COMMITs to the DSes if...")

Doing the test without taking any locks is racy, and so really it makes
more sense to do it in the flexfiles code (which is the only case that
cares).

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/pnfs_nfs.c | 7 -------
1 file changed, 7 deletions(-)

diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c
index 7ceb86627e54..25f28fa64c57 100644
--- a/fs/nfs/pnfs_nfs.c
+++ b/fs/nfs/pnfs_nfs.c
@@ -224,13 +224,6 @@ pnfs_generic_alloc_ds_commits(struct nfs_commit_info *cinfo,
for (i = 0; i < fl_cinfo->nbuckets; i++, bucket++) {
if (list_empty(&bucket->committing))
continue;
- /*
- * If the layout segment is invalid, then let
- * pnfs_generic_retry_commit() clean up the bucket.
- */
- if (bucket->clseg && !pnfs_is_valid_lseg(bucket->clseg) &&
- !test_bit(NFS_LSEG_LAYOUTRETURN, &bucket->clseg->pls_flags))
- break;
data = nfs_commitdata_alloc(false);
if (!data)
break;
--
2.13.0


2017-07-12 23:11:08

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v2 3/4] pNFS/flexfiles: Handle expired layout segments in ff_layout_initiate_commit()

If the layout has expired due to a fencing event, then we should not
attempt to commit to the DS.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/flexfilelayout/flexfilelayout.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 23542dc44a25..d41e1f2c5991 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -1875,6 +1875,10 @@ static int ff_layout_initiate_commit(struct nfs_commit_data *data, int how)
int vers, ret;
struct nfs_fh *fh;

+ if (!lseg || !(pnfs_is_valid_lseg(lseg) ||
+ test_bit(NFS_LSEG_LAYOUTRETURN, &lseg->pls_flags)))
+ goto out_err;
+
idx = calc_ds_index_from_commit(lseg, data->ds_commit_index);
ds = nfs4_ff_layout_prepare_ds(lseg, idx, true);
if (!ds)
--
2.13.0


2017-07-12 23:11:06

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v2 1/4] NFS: Fix a COMMIT race in pNFS

We must make sure that cinfo->ds->nwritten is in sync with the
commit list, since it is checked as part of pnfs_scan_commit_lists().

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/pnfs_nfs.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c
index d40755a0984b..3e6de85faf42 100644
--- a/fs/nfs/pnfs_nfs.c
+++ b/fs/nfs/pnfs_nfs.c
@@ -159,13 +159,18 @@ void pnfs_generic_recover_commit_reqs(struct list_head *dst,
{
struct pnfs_commit_bucket *b;
struct pnfs_layout_segment *freeme;
+ int nwritten;
int i;

lockdep_assert_held(&cinfo->inode->i_lock);
restart:
for (i = 0, b = cinfo->ds->buckets; i < cinfo->ds->nbuckets; i++, b++) {
- if (pnfs_generic_transfer_commit_list(&b->written, dst,
- cinfo, 0)) {
+ nwritten = pnfs_generic_transfer_commit_list(&b->written,
+ dst, cinfo, 0);
+ if (!nwritten)
+ continue;
+ cinfo->ds->nwritten -= nwritten;
+ if (list_empty(&b->written)) {
freeme = b->wlseg;
b->wlseg = NULL;
spin_unlock(&cinfo->inode->i_lock);
@@ -174,7 +179,6 @@ void pnfs_generic_recover_commit_reqs(struct list_head *dst,
goto restart;
}
}
- cinfo->ds->nwritten = 0;
}
EXPORT_SYMBOL_GPL(pnfs_generic_recover_commit_reqs);

--
2.13.0