* [PATCH 0/4] Add support for application leases to NFSv4 @ 2021-05-20 16:38 trondmy 2021-05-20 16:38 ` [PATCH 1/4] NFSv4: Fix delegation return in cases where we have to retry trondmy 0 siblings, 1 reply; 10+ messages in thread From: trondmy @ 2021-05-20 16:38 UTC (permalink / raw) To: linux-nfs From: Trond Myklebust <trond.myklebust@hammerspace.com> If the client holds a delegation, it should be able to offer all the guarantees that are needed to support application leases and delegations. The main use case here is when re-exporting NFSv4. Trond Myklebust (4): NFSv4: Fix delegation return in cases where we have to retry NFSv4: Add lease breakpoints in case of a delegation recall or return NFSv4: Add support for application leases underpinned by a delegation NFS: nfs_find_open_context() may only select open files fs/nfs/delegation.c | 94 ++++++++++++++++++++++++++++++++---------- fs/nfs/delegation.h | 1 + fs/nfs/inode.c | 4 ++ fs/nfs/nfs4_fs.h | 4 +- fs/nfs/nfs4file.c | 8 +++- fs/nfs/nfs4proc.c | 37 +++++++++++++++++ include/linux/nfs_fs.h | 1 + 7 files changed, 126 insertions(+), 23 deletions(-) -- 2.31.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/4] NFSv4: Fix delegation return in cases where we have to retry 2021-05-20 16:38 [PATCH 0/4] Add support for application leases to NFSv4 trondmy @ 2021-05-20 16:38 ` trondmy 2021-05-20 16:39 ` [PATCH 2/4] NFSv4: Add lease breakpoints in case of a delegation recall or return trondmy 2021-05-20 18:29 ` [PATCH 1/4] NFSv4: Fix delegation return in cases where we have to retry J. Bruce Fields 0 siblings, 2 replies; 10+ messages in thread From: trondmy @ 2021-05-20 16:38 UTC (permalink / raw) To: linux-nfs From: Trond Myklebust <trond.myklebust@hammerspace.com> If we're unable to immediately recover all locks because the server is unable to immediately service our reclaim calls, then we want to retry after we've finished servicing all the other asynchronous delegation returns on our queue. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/delegation.c | 71 +++++++++++++++++++++++++++++++++++---------- fs/nfs/delegation.h | 1 + fs/nfs/nfs4_fs.h | 1 + 3 files changed, 58 insertions(+), 15 deletions(-) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index e6ec6f09ac6e..7c45ac3c3b0b 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -75,6 +75,13 @@ void nfs_mark_delegation_referenced(struct nfs_delegation *delegation) set_bit(NFS_DELEGATION_REFERENCED, &delegation->flags); } +static void nfs_mark_return_delegation(struct nfs_server *server, + struct nfs_delegation *delegation) +{ + set_bit(NFS_DELEGATION_RETURN, &delegation->flags); + set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client->cl_state); +} + static bool nfs4_is_valid_delegation(const struct nfs_delegation *delegation, fmode_t flags) @@ -293,6 +300,7 @@ nfs_start_delegation_return_locked(struct nfs_inode *nfsi) goto out; spin_lock(&delegation->lock); if (!test_and_set_bit(NFS_DELEGATION_RETURNING, &delegation->flags)) { + clear_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation->flags); /* Refcount matched in nfs_end_delegation_return() */ ret = nfs_get_delegation(delegation); } @@ -314,16 +322,17 @@ nfs_start_delegation_return(struct nfs_inode *nfsi) return delegation; } -static void -nfs_abort_delegation_return(struct nfs_delegation *delegation, - struct nfs_client *clp) +static void nfs_abort_delegation_return(struct nfs_delegation *delegation, + struct nfs_client *clp, int err) { spin_lock(&delegation->lock); clear_bit(NFS_DELEGATION_RETURNING, &delegation->flags); - set_bit(NFS_DELEGATION_RETURN, &delegation->flags); + if (err == -EAGAIN) { + set_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation->flags); + set_bit(NFS4CLNT_DELEGRETURN_DELAYED, &clp->cl_state); + } spin_unlock(&delegation->lock); - set_bit(NFS4CLNT_DELEGRETURN, &clp->cl_state); } static struct nfs_delegation * @@ -539,7 +548,7 @@ static int nfs_end_delegation_return(struct inode *inode, struct nfs_delegation } while (err == 0); if (err) { - nfs_abort_delegation_return(delegation, clp); + nfs_abort_delegation_return(delegation, clp, err); goto out; } @@ -568,6 +577,7 @@ static bool nfs_delegation_need_return(struct nfs_delegation *delegation) if (ret) clear_bit(NFS_DELEGATION_RETURN_IF_CLOSED, &delegation->flags); if (test_bit(NFS_DELEGATION_RETURNING, &delegation->flags) || + test_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation->flags) || test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) ret = false; @@ -647,6 +657,38 @@ static int nfs_server_return_marked_delegations(struct nfs_server *server, return err; } +static bool nfs_server_clear_delayed_delegations(struct nfs_server *server) +{ + struct nfs_delegation *d; + bool ret = false; + + list_for_each_entry_rcu (d, &server->delegations, super_list) { + if (!test_bit(NFS_DELEGATION_RETURN_DELAYED, &d->flags)) + continue; + nfs_mark_return_delegation(server, d); + clear_bit(NFS_DELEGATION_RETURN_DELAYED, &d->flags); + ret = true; + } + return ret; +} + +static bool nfs_client_clear_delayed_delegations(struct nfs_client *clp) +{ + struct nfs_server *server; + bool ret = false; + + if (!test_and_clear_bit(NFS4CLNT_DELEGRETURN_DELAYED, &clp->cl_state)) + goto out; + rcu_read_lock(); + list_for_each_entry_rcu (server, &clp->cl_superblocks, client_link) { + if (nfs_server_clear_delayed_delegations(server)) + ret = true; + } + rcu_read_unlock(); +out: + return ret; +} + /** * nfs_client_return_marked_delegations - return previously marked delegations * @clp: nfs_client to process @@ -659,8 +701,14 @@ static int nfs_server_return_marked_delegations(struct nfs_server *server, */ int nfs_client_return_marked_delegations(struct nfs_client *clp) { - return nfs_client_for_each_server(clp, - nfs_server_return_marked_delegations, NULL); + int err = nfs_client_for_each_server( + clp, nfs_server_return_marked_delegations, NULL); + if (err) + return err; + /* If a return was delayed, sleep to prevent hard looping */ + if (nfs_client_clear_delayed_delegations(clp)) + ssleep(1); + return 0; } /** @@ -775,13 +823,6 @@ static void nfs_mark_return_if_closed_delegation(struct nfs_server *server, set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client->cl_state); } -static void nfs_mark_return_delegation(struct nfs_server *server, - struct nfs_delegation *delegation) -{ - set_bit(NFS_DELEGATION_RETURN, &delegation->flags); - set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client->cl_state); -} - static bool nfs_server_mark_return_all_delegations(struct nfs_server *server) { struct nfs_delegation *delegation; diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h index c19b4fd20781..1c378992b7c0 100644 --- a/fs/nfs/delegation.h +++ b/fs/nfs/delegation.h @@ -36,6 +36,7 @@ enum { NFS_DELEGATION_REVOKED, NFS_DELEGATION_TEST_EXPIRED, NFS_DELEGATION_INODE_FREEING, + NFS_DELEGATION_RETURN_DELAYED, }; int nfs_inode_set_delegation(struct inode *inode, const struct cred *cred, diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h index 065cb04222a1..4c44322c2643 100644 --- a/fs/nfs/nfs4_fs.h +++ b/fs/nfs/nfs4_fs.h @@ -45,6 +45,7 @@ enum nfs4_client_state { NFS4CLNT_RECALL_RUNNING, NFS4CLNT_RECALL_ANY_LAYOUT_READ, NFS4CLNT_RECALL_ANY_LAYOUT_RW, + NFS4CLNT_DELEGRETURN_DELAYED, }; #define NFS4_RENEW_TIMEOUT 0x01 -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/4] NFSv4: Add lease breakpoints in case of a delegation recall or return 2021-05-20 16:38 ` [PATCH 1/4] NFSv4: Fix delegation return in cases where we have to retry trondmy @ 2021-05-20 16:39 ` trondmy 2021-05-20 16:39 ` [PATCH 3/4] NFSv4: Add support for application leases underpinned by a delegation trondmy 2021-05-20 18:29 ` [PATCH 1/4] NFSv4: Fix delegation return in cases where we have to retry J. Bruce Fields 1 sibling, 1 reply; 10+ messages in thread From: trondmy @ 2021-05-20 16:39 UTC (permalink / raw) To: linux-nfs From: Trond Myklebust <trond.myklebust@hammerspace.com> When we add support for application level leases and knfsd delegations to the NFS client, we we want to have them safely underpinned by a "real" delegation to provide the caching guarantees. If that real delegation is recalled, then we need to ensure that the application leases/delegations are recalled too. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/delegation.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index 7c45ac3c3b0b..11118398f495 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -530,11 +530,18 @@ int nfs_inode_set_delegation(struct inode *inode, const struct cred *cred, static int nfs_end_delegation_return(struct inode *inode, struct nfs_delegation *delegation, int issync) { struct nfs_client *clp = NFS_SERVER(inode)->nfs_client; + unsigned int mode = O_WRONLY | O_RDWR; int err = 0; if (delegation == NULL) return 0; - do { + + if (!issync) + mode |= O_NONBLOCK; + /* Recall of any remaining application leases */ + err = break_lease(inode, mode); + + while (err == 0) { if (test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) break; err = nfs_delegation_claim_opens(inode, &delegation->stateid, @@ -545,7 +552,7 @@ static int nfs_end_delegation_return(struct inode *inode, struct nfs_delegation * Guard against state recovery */ err = nfs4_wait_clnt_recover(clp); - } while (err == 0); + } if (err) { nfs_abort_delegation_return(delegation, clp, err); @@ -746,13 +753,14 @@ int nfs4_inode_return_delegation(struct inode *inode) { struct nfs_inode *nfsi = NFS_I(inode); struct nfs_delegation *delegation; - int err = 0; - nfs_wb_all(inode); delegation = nfs_start_delegation_return(nfsi); + /* Synchronous recall of any application leases */ + break_lease(inode, O_WRONLY | O_RDWR); + nfs_wb_all(inode); if (delegation != NULL) - err = nfs_end_delegation_return(inode, delegation, 1); - return err; + return nfs_end_delegation_return(inode, delegation, 1); + return 0; } /** @@ -1051,6 +1059,9 @@ int nfs_async_inode_return_delegation(struct inode *inode, nfs_mark_return_delegation(server, delegation); rcu_read_unlock(); + /* If there are any application leases or delegations, recall them */ + break_lease(inode, O_WRONLY | O_RDWR | O_NONBLOCK); + nfs_delegation_run_state_manager(clp); return 0; out_enoent: -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/4] NFSv4: Add support for application leases underpinned by a delegation 2021-05-20 16:39 ` [PATCH 2/4] NFSv4: Add lease breakpoints in case of a delegation recall or return trondmy @ 2021-05-20 16:39 ` trondmy 2021-05-20 16:39 ` [PATCH 4/4] NFS: nfs_find_open_context() may only select open files trondmy 0 siblings, 1 reply; 10+ messages in thread From: trondmy @ 2021-05-20 16:39 UTC (permalink / raw) To: linux-nfs From: Trond Myklebust <trond.myklebust@hammerspace.com> If the NFSv4 client already holds a delegation for a file, then we can support application leases (i.e. fcntl(fd, F_SETLEASE,...)) because the underlying delegation guarantees that the file is not being modified on the server by another client in a way that might conflict with the lease guarantees. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/nfs4_fs.h | 3 ++- fs/nfs/nfs4file.c | 8 +++++++- fs/nfs/nfs4proc.c | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h index 4c44322c2643..6a04ba9e5e6a 100644 --- a/fs/nfs/nfs4_fs.h +++ b/fs/nfs/nfs4_fs.h @@ -322,7 +322,8 @@ extern int update_open_stateid(struct nfs4_state *state, const nfs4_stateid *open_stateid, const nfs4_stateid *deleg_stateid, fmode_t fmode); - +extern int nfs4_proc_setlease(struct file *file, long arg, + struct file_lock **lease, void **priv); extern int nfs4_proc_get_lease_time(struct nfs_client *clp, struct nfs_fsinfo *fsinfo); extern void nfs4_update_changeattr(struct inode *dir, diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c index 57b3821d975a..aafb77f9bfdf 100644 --- a/fs/nfs/nfs4file.c +++ b/fs/nfs/nfs4file.c @@ -435,6 +435,12 @@ void nfs42_ssc_unregister_ops(void) } #endif /* CONFIG_NFS_V4_2 */ +static int nfs4_setlease(struct file *file, long arg, struct file_lock **lease, + void **priv) +{ + return nfs4_proc_setlease(file, arg, lease, priv); +} + const struct file_operations nfs4_file_operations = { .read_iter = nfs_file_read, .write_iter = nfs_file_write, @@ -448,7 +454,7 @@ const struct file_operations nfs4_file_operations = { .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .check_flags = nfs_check_flags, - .setlease = simple_nosetlease, + .setlease = nfs4_setlease, #ifdef CONFIG_NFS_V4_2 .copy_file_range = nfs4_copy_file_range, .llseek = nfs4_file_llseek, diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 0cd965882232..e1abd024a492 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -7420,6 +7420,43 @@ nfs4_proc_lock(struct file *filp, int cmd, struct file_lock *request) return nfs4_retry_setlk(state, cmd, request); } +static int nfs4_delete_lease(struct file *file, void **priv) +{ + return generic_setlease(file, F_UNLCK, NULL, priv); +} + +static int nfs4_add_lease(struct file *file, long arg, struct file_lock **lease, + void **priv) +{ + struct inode *inode = file_inode(file); + fmode_t type = arg == F_RDLCK ? FMODE_READ : FMODE_WRITE; + int ret; + + /* No delegation, no lease */ + if (!nfs4_have_delegation(inode, type)) + return -ENOLCK; + ret = generic_setlease(file, arg, lease, priv); + if (ret || nfs4_have_delegation(inode, type)) + return ret; + /* We raced with a delegation return */ + nfs4_delete_lease(file, priv); + return -ENOLCK; +} + +int nfs4_proc_setlease(struct file *file, long arg, struct file_lock **lease, + void **priv) +{ + switch (arg) { + case F_RDLCK: + case F_WRLCK: + return nfs4_add_lease(file, arg, lease, priv); + case F_UNLCK: + return nfs4_delete_lease(file, priv); + default: + return -EINVAL; + } +} + int nfs4_lock_delegation_recall(struct file_lock *fl, struct nfs4_state *state, const nfs4_stateid *stateid) { struct nfs_server *server = NFS_SERVER(state->inode); -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/4] NFS: nfs_find_open_context() may only select open files 2021-05-20 16:39 ` [PATCH 3/4] NFSv4: Add support for application leases underpinned by a delegation trondmy @ 2021-05-20 16:39 ` trondmy 0 siblings, 0 replies; 10+ messages in thread From: trondmy @ 2021-05-20 16:39 UTC (permalink / raw) To: linux-nfs From: Trond Myklebust <trond.myklebust@hammerspace.com> If a file has already been closed, then it should not be selected to support further I/O. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/inode.c | 4 ++++ include/linux/nfs_fs.h | 1 + 2 files changed, 5 insertions(+) diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 529c4099f482..5ccc6b258ca5 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -1101,6 +1101,7 @@ EXPORT_SYMBOL_GPL(nfs_inode_attach_open_context); void nfs_file_set_open_context(struct file *filp, struct nfs_open_context *ctx) { filp->private_data = get_nfs_open_context(ctx); + set_bit(NFS_CONTEXT_FILE_OPEN, &ctx->flags); if (list_empty(&ctx->list)) nfs_inode_attach_open_context(ctx); } @@ -1120,6 +1121,8 @@ struct nfs_open_context *nfs_find_open_context(struct inode *inode, const struct continue; if ((pos->mode & (FMODE_READ|FMODE_WRITE)) != mode) continue; + if (!test_bit(NFS_CONTEXT_FILE_OPEN, &ctx->flags)) + continue; ctx = get_nfs_open_context(pos); if (ctx) break; @@ -1135,6 +1138,7 @@ void nfs_file_clear_open_context(struct file *filp) if (ctx) { struct inode *inode = d_inode(ctx->dentry); + clear_bit(NFS_CONTEXT_FILE_OPEN, &ctx->flags); /* * We fatal error on write before. Try to writeback * every page again. diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h index ffba254d2098..ce6474594872 100644 --- a/include/linux/nfs_fs.h +++ b/include/linux/nfs_fs.h @@ -84,6 +84,7 @@ struct nfs_open_context { #define NFS_CONTEXT_RESEND_WRITES (1) #define NFS_CONTEXT_BAD (2) #define NFS_CONTEXT_UNLOCK (3) +#define NFS_CONTEXT_FILE_OPEN (4) int error; struct list_head list; -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] NFSv4: Fix delegation return in cases where we have to retry 2021-05-20 16:38 ` [PATCH 1/4] NFSv4: Fix delegation return in cases where we have to retry trondmy 2021-05-20 16:39 ` [PATCH 2/4] NFSv4: Add lease breakpoints in case of a delegation recall or return trondmy @ 2021-05-20 18:29 ` J. Bruce Fields 2021-05-20 19:08 ` Trond Myklebust 1 sibling, 1 reply; 10+ messages in thread From: J. Bruce Fields @ 2021-05-20 18:29 UTC (permalink / raw) To: trondmy; +Cc: linux-nfs On Thu, May 20, 2021 at 12:38:59PM -0400, trondmy@kernel.org wrote: > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > If we're unable to immediately recover all locks because the server is > unable to immediately service our reclaim calls, then we want to retry > after we've finished servicing all the other asynchronous delegation > returns on our queue. So, there's a situation where the server can't service a reclaim until some other delegation is returned? I'm not seeing how that happens. --b. > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > --- > fs/nfs/delegation.c | 71 +++++++++++++++++++++++++++++++++++---------- > fs/nfs/delegation.h | 1 + > fs/nfs/nfs4_fs.h | 1 + > 3 files changed, 58 insertions(+), 15 deletions(-) > > diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c > index e6ec6f09ac6e..7c45ac3c3b0b 100644 > --- a/fs/nfs/delegation.c > +++ b/fs/nfs/delegation.c > @@ -75,6 +75,13 @@ void nfs_mark_delegation_referenced(struct nfs_delegation *delegation) > set_bit(NFS_DELEGATION_REFERENCED, &delegation->flags); > } > > +static void nfs_mark_return_delegation(struct nfs_server *server, > + struct nfs_delegation *delegation) > +{ > + set_bit(NFS_DELEGATION_RETURN, &delegation->flags); > + set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client->cl_state); > +} > + > static bool > nfs4_is_valid_delegation(const struct nfs_delegation *delegation, > fmode_t flags) > @@ -293,6 +300,7 @@ nfs_start_delegation_return_locked(struct nfs_inode *nfsi) > goto out; > spin_lock(&delegation->lock); > if (!test_and_set_bit(NFS_DELEGATION_RETURNING, &delegation->flags)) { > + clear_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation->flags); > /* Refcount matched in nfs_end_delegation_return() */ > ret = nfs_get_delegation(delegation); > } > @@ -314,16 +322,17 @@ nfs_start_delegation_return(struct nfs_inode *nfsi) > return delegation; > } > > -static void > -nfs_abort_delegation_return(struct nfs_delegation *delegation, > - struct nfs_client *clp) > +static void nfs_abort_delegation_return(struct nfs_delegation *delegation, > + struct nfs_client *clp, int err) > { > > spin_lock(&delegation->lock); > clear_bit(NFS_DELEGATION_RETURNING, &delegation->flags); > - set_bit(NFS_DELEGATION_RETURN, &delegation->flags); > + if (err == -EAGAIN) { > + set_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation->flags); > + set_bit(NFS4CLNT_DELEGRETURN_DELAYED, &clp->cl_state); > + } > spin_unlock(&delegation->lock); > - set_bit(NFS4CLNT_DELEGRETURN, &clp->cl_state); > } > > static struct nfs_delegation * > @@ -539,7 +548,7 @@ static int nfs_end_delegation_return(struct inode *inode, struct nfs_delegation > } while (err == 0); > > if (err) { > - nfs_abort_delegation_return(delegation, clp); > + nfs_abort_delegation_return(delegation, clp, err); > goto out; > } > > @@ -568,6 +577,7 @@ static bool nfs_delegation_need_return(struct nfs_delegation *delegation) > if (ret) > clear_bit(NFS_DELEGATION_RETURN_IF_CLOSED, &delegation->flags); > if (test_bit(NFS_DELEGATION_RETURNING, &delegation->flags) || > + test_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation->flags) || > test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) > ret = false; > > @@ -647,6 +657,38 @@ static int nfs_server_return_marked_delegations(struct nfs_server *server, > return err; > } > > +static bool nfs_server_clear_delayed_delegations(struct nfs_server *server) > +{ > + struct nfs_delegation *d; > + bool ret = false; > + > + list_for_each_entry_rcu (d, &server->delegations, super_list) { > + if (!test_bit(NFS_DELEGATION_RETURN_DELAYED, &d->flags)) > + continue; > + nfs_mark_return_delegation(server, d); > + clear_bit(NFS_DELEGATION_RETURN_DELAYED, &d->flags); > + ret = true; > + } > + return ret; > +} > + > +static bool nfs_client_clear_delayed_delegations(struct nfs_client *clp) > +{ > + struct nfs_server *server; > + bool ret = false; > + > + if (!test_and_clear_bit(NFS4CLNT_DELEGRETURN_DELAYED, &clp->cl_state)) > + goto out; > + rcu_read_lock(); > + list_for_each_entry_rcu (server, &clp->cl_superblocks, client_link) { > + if (nfs_server_clear_delayed_delegations(server)) > + ret = true; > + } > + rcu_read_unlock(); > +out: > + return ret; > +} > + > /** > * nfs_client_return_marked_delegations - return previously marked delegations > * @clp: nfs_client to process > @@ -659,8 +701,14 @@ static int nfs_server_return_marked_delegations(struct nfs_server *server, > */ > int nfs_client_return_marked_delegations(struct nfs_client *clp) > { > - return nfs_client_for_each_server(clp, > - nfs_server_return_marked_delegations, NULL); > + int err = nfs_client_for_each_server( > + clp, nfs_server_return_marked_delegations, NULL); > + if (err) > + return err; > + /* If a return was delayed, sleep to prevent hard looping */ > + if (nfs_client_clear_delayed_delegations(clp)) > + ssleep(1); > + return 0; > } > > /** > @@ -775,13 +823,6 @@ static void nfs_mark_return_if_closed_delegation(struct nfs_server *server, > set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client->cl_state); > } > > -static void nfs_mark_return_delegation(struct nfs_server *server, > - struct nfs_delegation *delegation) > -{ > - set_bit(NFS_DELEGATION_RETURN, &delegation->flags); > - set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client->cl_state); > -} > - > static bool nfs_server_mark_return_all_delegations(struct nfs_server *server) > { > struct nfs_delegation *delegation; > diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h > index c19b4fd20781..1c378992b7c0 100644 > --- a/fs/nfs/delegation.h > +++ b/fs/nfs/delegation.h > @@ -36,6 +36,7 @@ enum { > NFS_DELEGATION_REVOKED, > NFS_DELEGATION_TEST_EXPIRED, > NFS_DELEGATION_INODE_FREEING, > + NFS_DELEGATION_RETURN_DELAYED, > }; > > int nfs_inode_set_delegation(struct inode *inode, const struct cred *cred, > diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h > index 065cb04222a1..4c44322c2643 100644 > --- a/fs/nfs/nfs4_fs.h > +++ b/fs/nfs/nfs4_fs.h > @@ -45,6 +45,7 @@ enum nfs4_client_state { > NFS4CLNT_RECALL_RUNNING, > NFS4CLNT_RECALL_ANY_LAYOUT_READ, > NFS4CLNT_RECALL_ANY_LAYOUT_RW, > + NFS4CLNT_DELEGRETURN_DELAYED, > }; > > #define NFS4_RENEW_TIMEOUT 0x01 > -- > 2.31.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] NFSv4: Fix delegation return in cases where we have to retry 2021-05-20 18:29 ` [PATCH 1/4] NFSv4: Fix delegation return in cases where we have to retry J. Bruce Fields @ 2021-05-20 19:08 ` Trond Myklebust 2021-05-20 20:47 ` bfields 0 siblings, 1 reply; 10+ messages in thread From: Trond Myklebust @ 2021-05-20 19:08 UTC (permalink / raw) To: bfields@fieldses.org; +Cc: linux-nfs@vger.kernel.org On Thu, 2021-05-20 at 14:29 -0400, J. Bruce Fields wrote: > On Thu, May 20, 2021 at 12:38:59PM -0400, trondmy@kernel.org wrote: > > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > > > If we're unable to immediately recover all locks because the server > > is > > unable to immediately service our reclaim calls, then we want to > > retry > > after we've finished servicing all the other asynchronous > > delegation > > returns on our queue. > > So, there's a situation where the server can't service a reclaim > until > some other delegation is returned? I'm not seeing how that happens. > I can and I do... pNFS can be complicated... > --b. > > > > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > > --- > > fs/nfs/delegation.c | 71 +++++++++++++++++++++++++++++++++++------ > > ---- > > fs/nfs/delegation.h | 1 + > > fs/nfs/nfs4_fs.h | 1 + > > 3 files changed, 58 insertions(+), 15 deletions(-) > > > > diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c > > index e6ec6f09ac6e..7c45ac3c3b0b 100644 > > --- a/fs/nfs/delegation.c > > +++ b/fs/nfs/delegation.c > > @@ -75,6 +75,13 @@ void nfs_mark_delegation_referenced(struct > > nfs_delegation *delegation) > > set_bit(NFS_DELEGATION_REFERENCED, &delegation->flags); > > } > > > > +static void nfs_mark_return_delegation(struct nfs_server *server, > > + struct nfs_delegation > > *delegation) > > +{ > > + set_bit(NFS_DELEGATION_RETURN, &delegation->flags); > > + set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client- > > >cl_state); > > +} > > + > > static bool > > nfs4_is_valid_delegation(const struct nfs_delegation *delegation, > > fmode_t flags) > > @@ -293,6 +300,7 @@ nfs_start_delegation_return_locked(struct > > nfs_inode *nfsi) > > goto out; > > spin_lock(&delegation->lock); > > if (!test_and_set_bit(NFS_DELEGATION_RETURNING, > > &delegation->flags)) { > > + clear_bit(NFS_DELEGATION_RETURN_DELAYED, > > &delegation->flags); > > /* Refcount matched in nfs_end_delegation_return() > > */ > > ret = nfs_get_delegation(delegation); > > } > > @@ -314,16 +322,17 @@ nfs_start_delegation_return(struct nfs_inode > > *nfsi) > > return delegation; > > } > > > > -static void > > -nfs_abort_delegation_return(struct nfs_delegation *delegation, > > - struct nfs_client *clp) > > +static void nfs_abort_delegation_return(struct nfs_delegation > > *delegation, > > + struct nfs_client *clp, int > > err) > > { > > > > spin_lock(&delegation->lock); > > clear_bit(NFS_DELEGATION_RETURNING, &delegation->flags); > > - set_bit(NFS_DELEGATION_RETURN, &delegation->flags); > > + if (err == -EAGAIN) { > > + set_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation- > > >flags); > > + set_bit(NFS4CLNT_DELEGRETURN_DELAYED, &clp- > > >cl_state); > > + } > > spin_unlock(&delegation->lock); > > - set_bit(NFS4CLNT_DELEGRETURN, &clp->cl_state); > > } > > > > static struct nfs_delegation * > > @@ -539,7 +548,7 @@ static int nfs_end_delegation_return(struct > > inode *inode, struct nfs_delegation > > } while (err == 0); > > > > if (err) { > > - nfs_abort_delegation_return(delegation, clp); > > + nfs_abort_delegation_return(delegation, clp, err); > > goto out; > > } > > > > @@ -568,6 +577,7 @@ static bool nfs_delegation_need_return(struct > > nfs_delegation *delegation) > > if (ret) > > clear_bit(NFS_DELEGATION_RETURN_IF_CLOSED, > > &delegation->flags); > > if (test_bit(NFS_DELEGATION_RETURNING, &delegation->flags) > > || > > + test_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation- > > >flags) || > > test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) > > ret = false; > > > > @@ -647,6 +657,38 @@ static int > > nfs_server_return_marked_delegations(struct nfs_server *server, > > return err; > > } > > > > +static bool nfs_server_clear_delayed_delegations(struct nfs_server > > *server) > > +{ > > + struct nfs_delegation *d; > > + bool ret = false; > > + > > + list_for_each_entry_rcu (d, &server->delegations, > > super_list) { > > + if (!test_bit(NFS_DELEGATION_RETURN_DELAYED, &d- > > >flags)) > > + continue; > > + nfs_mark_return_delegation(server, d); > > + clear_bit(NFS_DELEGATION_RETURN_DELAYED, &d- > > >flags); > > + ret = true; > > + } > > + return ret; > > +} > > + > > +static bool nfs_client_clear_delayed_delegations(struct nfs_client > > *clp) > > +{ > > + struct nfs_server *server; > > + bool ret = false; > > + > > + if (!test_and_clear_bit(NFS4CLNT_DELEGRETURN_DELAYED, &clp- > > >cl_state)) > > + goto out; > > + rcu_read_lock(); > > + list_for_each_entry_rcu (server, &clp->cl_superblocks, > > client_link) { > > + if (nfs_server_clear_delayed_delegations(server)) > > + ret = true; > > + } > > + rcu_read_unlock(); > > +out: > > + return ret; > > +} > > + > > /** > > * nfs_client_return_marked_delegations - return previously marked > > delegations > > * @clp: nfs_client to process > > @@ -659,8 +701,14 @@ static int > > nfs_server_return_marked_delegations(struct nfs_server *server, > > */ > > int nfs_client_return_marked_delegations(struct nfs_client *clp) > > { > > - return nfs_client_for_each_server(clp, > > - nfs_server_return_marked_delegations, > > NULL); > > + int err = nfs_client_for_each_server( > > + clp, nfs_server_return_marked_delegations, NULL); > > + if (err) > > + return err; > > + /* If a return was delayed, sleep to prevent hard looping > > */ > > + if (nfs_client_clear_delayed_delegations(clp)) > > + ssleep(1); > > + return 0; > > } > > > > /** > > @@ -775,13 +823,6 @@ static void > > nfs_mark_return_if_closed_delegation(struct nfs_server *server, > > set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client- > > >cl_state); > > } > > > > -static void nfs_mark_return_delegation(struct nfs_server *server, > > - struct nfs_delegation *delegation) > > -{ > > - set_bit(NFS_DELEGATION_RETURN, &delegation->flags); > > - set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client- > > >cl_state); > > -} > > - > > static bool nfs_server_mark_return_all_delegations(struct > > nfs_server *server) > > { > > struct nfs_delegation *delegation; > > diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h > > index c19b4fd20781..1c378992b7c0 100644 > > --- a/fs/nfs/delegation.h > > +++ b/fs/nfs/delegation.h > > @@ -36,6 +36,7 @@ enum { > > NFS_DELEGATION_REVOKED, > > NFS_DELEGATION_TEST_EXPIRED, > > NFS_DELEGATION_INODE_FREEING, > > + NFS_DELEGATION_RETURN_DELAYED, > > }; > > > > int nfs_inode_set_delegation(struct inode *inode, const struct > > cred *cred, > > diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h > > index 065cb04222a1..4c44322c2643 100644 > > --- a/fs/nfs/nfs4_fs.h > > +++ b/fs/nfs/nfs4_fs.h > > @@ -45,6 +45,7 @@ enum nfs4_client_state { > > NFS4CLNT_RECALL_RUNNING, > > NFS4CLNT_RECALL_ANY_LAYOUT_READ, > > NFS4CLNT_RECALL_ANY_LAYOUT_RW, > > + NFS4CLNT_DELEGRETURN_DELAYED, > > }; > > > > #define NFS4_RENEW_TIMEOUT 0x01 > > -- > > 2.31.1 -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@hammerspace.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] NFSv4: Fix delegation return in cases where we have to retry 2021-05-20 19:08 ` Trond Myklebust @ 2021-05-20 20:47 ` bfields 2021-05-20 21:14 ` Trond Myklebust 0 siblings, 1 reply; 10+ messages in thread From: bfields @ 2021-05-20 20:47 UTC (permalink / raw) To: Trond Myklebust; +Cc: linux-nfs@vger.kernel.org On Thu, May 20, 2021 at 07:08:24PM +0000, Trond Myklebust wrote: > On Thu, 2021-05-20 at 14:29 -0400, J. Bruce Fields wrote: > > On Thu, May 20, 2021 at 12:38:59PM -0400, trondmy@kernel.org wrote: > > > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > > > > > If we're unable to immediately recover all locks because the server > > > is > > > unable to immediately service our reclaim calls, then we want to > > > retry > > > after we've finished servicing all the other asynchronous > > > delegation > > > returns on our queue. > > > > So, there's a situation where the server can't service a reclaim > > until > > some other delegation is returned? I'm not seeing how that happens. > > > > I can and I do... pNFS can be complicated... I don't doubt you, but does everyone get this but me? Is it too complicated to explain? --b. > > > --b. > > > > > > > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > > > --- > > > fs/nfs/delegation.c | 71 +++++++++++++++++++++++++++++++++++------ > > > ---- > > > fs/nfs/delegation.h | 1 + > > > fs/nfs/nfs4_fs.h | 1 + > > > 3 files changed, 58 insertions(+), 15 deletions(-) > > > > > > diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c > > > index e6ec6f09ac6e..7c45ac3c3b0b 100644 > > > --- a/fs/nfs/delegation.c > > > +++ b/fs/nfs/delegation.c > > > @@ -75,6 +75,13 @@ void nfs_mark_delegation_referenced(struct > > > nfs_delegation *delegation) > > > set_bit(NFS_DELEGATION_REFERENCED, &delegation->flags); > > > } > > > > > > +static void nfs_mark_return_delegation(struct nfs_server *server, > > > + struct nfs_delegation > > > *delegation) > > > +{ > > > + set_bit(NFS_DELEGATION_RETURN, &delegation->flags); > > > + set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client- > > > >cl_state); > > > +} > > > + > > > static bool > > > nfs4_is_valid_delegation(const struct nfs_delegation *delegation, > > > fmode_t flags) > > > @@ -293,6 +300,7 @@ nfs_start_delegation_return_locked(struct > > > nfs_inode *nfsi) > > > goto out; > > > spin_lock(&delegation->lock); > > > if (!test_and_set_bit(NFS_DELEGATION_RETURNING, > > > &delegation->flags)) { > > > + clear_bit(NFS_DELEGATION_RETURN_DELAYED, > > > &delegation->flags); > > > /* Refcount matched in nfs_end_delegation_return() > > > */ > > > ret = nfs_get_delegation(delegation); > > > } > > > @@ -314,16 +322,17 @@ nfs_start_delegation_return(struct nfs_inode > > > *nfsi) > > > return delegation; > > > } > > > > > > -static void > > > -nfs_abort_delegation_return(struct nfs_delegation *delegation, > > > - struct nfs_client *clp) > > > +static void nfs_abort_delegation_return(struct nfs_delegation > > > *delegation, > > > + struct nfs_client *clp, int > > > err) > > > { > > > > > > spin_lock(&delegation->lock); > > > clear_bit(NFS_DELEGATION_RETURNING, &delegation->flags); > > > - set_bit(NFS_DELEGATION_RETURN, &delegation->flags); > > > + if (err == -EAGAIN) { > > > + set_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation- > > > >flags); > > > + set_bit(NFS4CLNT_DELEGRETURN_DELAYED, &clp- > > > >cl_state); > > > + } > > > spin_unlock(&delegation->lock); > > > - set_bit(NFS4CLNT_DELEGRETURN, &clp->cl_state); > > > } > > > > > > static struct nfs_delegation * > > > @@ -539,7 +548,7 @@ static int nfs_end_delegation_return(struct > > > inode *inode, struct nfs_delegation > > > } while (err == 0); > > > > > > if (err) { > > > - nfs_abort_delegation_return(delegation, clp); > > > + nfs_abort_delegation_return(delegation, clp, err); > > > goto out; > > > } > > > > > > @@ -568,6 +577,7 @@ static bool nfs_delegation_need_return(struct > > > nfs_delegation *delegation) > > > if (ret) > > > clear_bit(NFS_DELEGATION_RETURN_IF_CLOSED, > > > &delegation->flags); > > > if (test_bit(NFS_DELEGATION_RETURNING, &delegation->flags) > > > || > > > + test_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation- > > > >flags) || > > > test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) > > > ret = false; > > > > > > @@ -647,6 +657,38 @@ static int > > > nfs_server_return_marked_delegations(struct nfs_server *server, > > > return err; > > > } > > > > > > +static bool nfs_server_clear_delayed_delegations(struct nfs_server > > > *server) > > > +{ > > > + struct nfs_delegation *d; > > > + bool ret = false; > > > + > > > + list_for_each_entry_rcu (d, &server->delegations, > > > super_list) { > > > + if (!test_bit(NFS_DELEGATION_RETURN_DELAYED, &d- > > > >flags)) > > > + continue; > > > + nfs_mark_return_delegation(server, d); > > > + clear_bit(NFS_DELEGATION_RETURN_DELAYED, &d- > > > >flags); > > > + ret = true; > > > + } > > > + return ret; > > > +} > > > + > > > +static bool nfs_client_clear_delayed_delegations(struct nfs_client > > > *clp) > > > +{ > > > + struct nfs_server *server; > > > + bool ret = false; > > > + > > > + if (!test_and_clear_bit(NFS4CLNT_DELEGRETURN_DELAYED, &clp- > > > >cl_state)) > > > + goto out; > > > + rcu_read_lock(); > > > + list_for_each_entry_rcu (server, &clp->cl_superblocks, > > > client_link) { > > > + if (nfs_server_clear_delayed_delegations(server)) > > > + ret = true; > > > + } > > > + rcu_read_unlock(); > > > +out: > > > + return ret; > > > +} > > > + > > > /** > > > * nfs_client_return_marked_delegations - return previously marked > > > delegations > > > * @clp: nfs_client to process > > > @@ -659,8 +701,14 @@ static int > > > nfs_server_return_marked_delegations(struct nfs_server *server, > > > */ > > > int nfs_client_return_marked_delegations(struct nfs_client *clp) > > > { > > > - return nfs_client_for_each_server(clp, > > > - nfs_server_return_marked_delegations, > > > NULL); > > > + int err = nfs_client_for_each_server( > > > + clp, nfs_server_return_marked_delegations, NULL); > > > + if (err) > > > + return err; > > > + /* If a return was delayed, sleep to prevent hard looping > > > */ > > > + if (nfs_client_clear_delayed_delegations(clp)) > > > + ssleep(1); > > > + return 0; > > > } > > > > > > /** > > > @@ -775,13 +823,6 @@ static void > > > nfs_mark_return_if_closed_delegation(struct nfs_server *server, > > > set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client- > > > >cl_state); > > > } > > > > > > -static void nfs_mark_return_delegation(struct nfs_server *server, > > > - struct nfs_delegation *delegation) > > > -{ > > > - set_bit(NFS_DELEGATION_RETURN, &delegation->flags); > > > - set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client- > > > >cl_state); > > > -} > > > - > > > static bool nfs_server_mark_return_all_delegations(struct > > > nfs_server *server) > > > { > > > struct nfs_delegation *delegation; > > > diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h > > > index c19b4fd20781..1c378992b7c0 100644 > > > --- a/fs/nfs/delegation.h > > > +++ b/fs/nfs/delegation.h > > > @@ -36,6 +36,7 @@ enum { > > > NFS_DELEGATION_REVOKED, > > > NFS_DELEGATION_TEST_EXPIRED, > > > NFS_DELEGATION_INODE_FREEING, > > > + NFS_DELEGATION_RETURN_DELAYED, > > > }; > > > > > > int nfs_inode_set_delegation(struct inode *inode, const struct > > > cred *cred, > > > diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h > > > index 065cb04222a1..4c44322c2643 100644 > > > --- a/fs/nfs/nfs4_fs.h > > > +++ b/fs/nfs/nfs4_fs.h > > > @@ -45,6 +45,7 @@ enum nfs4_client_state { > > > NFS4CLNT_RECALL_RUNNING, > > > NFS4CLNT_RECALL_ANY_LAYOUT_READ, > > > NFS4CLNT_RECALL_ANY_LAYOUT_RW, > > > + NFS4CLNT_DELEGRETURN_DELAYED, > > > }; > > > > > > #define NFS4_RENEW_TIMEOUT 0x01 > > > -- > > > 2.31.1 > > -- > Trond Myklebust > Linux NFS client maintainer, Hammerspace > trond.myklebust@hammerspace.com > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] NFSv4: Fix delegation return in cases where we have to retry 2021-05-20 20:47 ` bfields @ 2021-05-20 21:14 ` Trond Myklebust 2021-05-21 20:23 ` bfields 0 siblings, 1 reply; 10+ messages in thread From: Trond Myklebust @ 2021-05-20 21:14 UTC (permalink / raw) To: bfields@fieldses.org; +Cc: linux-nfs@vger.kernel.org On Thu, 2021-05-20 at 16:47 -0400, bfields@fieldses.org wrote: > On Thu, May 20, 2021 at 07:08:24PM +0000, Trond Myklebust wrote: > > On Thu, 2021-05-20 at 14:29 -0400, J. Bruce Fields wrote: > > > On Thu, May 20, 2021 at 12:38:59PM -0400, trondmy@kernel.org wrote: > > > > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > > > > > > > If we're unable to immediately recover all locks because the > > > > server > > > > is > > > > unable to immediately service our reclaim calls, then we want to > > > > retry > > > > after we've finished servicing all the other asynchronous > > > > delegation > > > > returns on our queue. > > > > > > So, there's a situation where the server can't service a reclaim > > > until > > > some other delegation is returned? I'm not seeing how that > > > happens. > > > > > > > I can and I do... pNFS can be complicated... > > I don't doubt you, but does everyone get this but me? > > Is it too complicated to explain? > Not if you read the code, no. In nfs_end_delegation_return(): do { if (test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) break; err = nfs_delegation_claim_opens(inode, &delegation->stateid, delegation->type); if (!issync || err != -EAGAIN) break; /* * Guard against state recovery */ err = nfs4_wait_clnt_recover(clp); } while (err == 0); So if issync is false, then we abort the delegation return and try again later. ...and when we later add a break_lease() call in this patch series, then there is another condition. > --b. > > > > > > --b. > > > > > > > > > > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > > > > --- > > > > fs/nfs/delegation.c | 71 +++++++++++++++++++++++++++++++++++---- > > > > -- > > > > ---- > > > > fs/nfs/delegation.h | 1 + > > > > fs/nfs/nfs4_fs.h | 1 + > > > > 3 files changed, 58 insertions(+), 15 deletions(-) > > > > > > > > diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c > > > > index e6ec6f09ac6e..7c45ac3c3b0b 100644 > > > > --- a/fs/nfs/delegation.c > > > > +++ b/fs/nfs/delegation.c > > > > @@ -75,6 +75,13 @@ void nfs_mark_delegation_referenced(struct > > > > nfs_delegation *delegation) > > > > set_bit(NFS_DELEGATION_REFERENCED, &delegation->flags); > > > > } > > > > > > > > +static void nfs_mark_return_delegation(struct nfs_server > > > > *server, > > > > + struct nfs_delegation > > > > *delegation) > > > > +{ > > > > + set_bit(NFS_DELEGATION_RETURN, &delegation->flags); > > > > + set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client- > > > > > cl_state); > > > > +} > > > > + > > > > static bool > > > > nfs4_is_valid_delegation(const struct nfs_delegation > > > > *delegation, > > > > fmode_t flags) > > > > @@ -293,6 +300,7 @@ nfs_start_delegation_return_locked(struct > > > > nfs_inode *nfsi) > > > > goto out; > > > > spin_lock(&delegation->lock); > > > > if (!test_and_set_bit(NFS_DELEGATION_RETURNING, > > > > &delegation->flags)) { > > > > + clear_bit(NFS_DELEGATION_RETURN_DELAYED, > > > > &delegation->flags); > > > > /* Refcount matched in > > > > nfs_end_delegation_return() > > > > */ > > > > ret = nfs_get_delegation(delegation); > > > > } > > > > @@ -314,16 +322,17 @@ nfs_start_delegation_return(struct > > > > nfs_inode > > > > *nfsi) > > > > return delegation; > > > > } > > > > > > > > -static void > > > > -nfs_abort_delegation_return(struct nfs_delegation *delegation, > > > > - struct nfs_client *clp) > > > > +static void nfs_abort_delegation_return(struct nfs_delegation > > > > *delegation, > > > > + struct nfs_client *clp, > > > > int > > > > err) > > > > { > > > > > > > > spin_lock(&delegation->lock); > > > > clear_bit(NFS_DELEGATION_RETURNING, &delegation->flags); > > > > - set_bit(NFS_DELEGATION_RETURN, &delegation->flags); > > > > + if (err == -EAGAIN) { > > > > + set_bit(NFS_DELEGATION_RETURN_DELAYED, > > > > &delegation- > > > > > flags); > > > > + set_bit(NFS4CLNT_DELEGRETURN_DELAYED, &clp- > > > > > cl_state); > > > > + } > > > > spin_unlock(&delegation->lock); > > > > - set_bit(NFS4CLNT_DELEGRETURN, &clp->cl_state); > > > > } > > > > > > > > static struct nfs_delegation * > > > > @@ -539,7 +548,7 @@ static int nfs_end_delegation_return(struct > > > > inode *inode, struct nfs_delegation > > > > } while (err == 0); > > > > > > > > if (err) { > > > > - nfs_abort_delegation_return(delegation, clp); > > > > + nfs_abort_delegation_return(delegation, clp, > > > > err); > > > > goto out; > > > > } > > > > > > > > @@ -568,6 +577,7 @@ static bool nfs_delegation_need_return(struct > > > > nfs_delegation *delegation) > > > > if (ret) > > > > clear_bit(NFS_DELEGATION_RETURN_IF_CLOSED, > > > > &delegation->flags); > > > > if (test_bit(NFS_DELEGATION_RETURNING, &delegation- > > > > >flags) > > > > > > > > > > + test_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation- > > > > > flags) || > > > > test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) > > > > ret = false; > > > > > > > > @@ -647,6 +657,38 @@ static int > > > > nfs_server_return_marked_delegations(struct nfs_server *server, > > > > return err; > > > > } > > > > > > > > +static bool nfs_server_clear_delayed_delegations(struct > > > > nfs_server > > > > *server) > > > > +{ > > > > + struct nfs_delegation *d; > > > > + bool ret = false; > > > > + > > > > + list_for_each_entry_rcu (d, &server->delegations, > > > > super_list) { > > > > + if (!test_bit(NFS_DELEGATION_RETURN_DELAYED, &d- > > > > > flags)) > > > > + continue; > > > > + nfs_mark_return_delegation(server, d); > > > > + clear_bit(NFS_DELEGATION_RETURN_DELAYED, &d- > > > > > flags); > > > > + ret = true; > > > > + } > > > > + return ret; > > > > +} > > > > + > > > > +static bool nfs_client_clear_delayed_delegations(struct > > > > nfs_client > > > > *clp) > > > > +{ > > > > + struct nfs_server *server; > > > > + bool ret = false; > > > > + > > > > + if (!test_and_clear_bit(NFS4CLNT_DELEGRETURN_DELAYED, > > > > &clp- > > > > > cl_state)) > > > > + goto out; > > > > + rcu_read_lock(); > > > > + list_for_each_entry_rcu (server, &clp->cl_superblocks, > > > > client_link) { > > > > + if (nfs_server_clear_delayed_delegations(server)) > > > > + ret = true; > > > > + } > > > > + rcu_read_unlock(); > > > > +out: > > > > + return ret; > > > > +} > > > > + > > > > /** > > > > * nfs_client_return_marked_delegations - return previously > > > > marked > > > > delegations > > > > * @clp: nfs_client to process > > > > @@ -659,8 +701,14 @@ static int > > > > nfs_server_return_marked_delegations(struct nfs_server *server, > > > > */ > > > > int nfs_client_return_marked_delegations(struct nfs_client *clp) > > > > { > > > > - return nfs_client_for_each_server(clp, > > > > - nfs_server_return_marked_delegations, > > > > NULL); > > > > + int err = nfs_client_for_each_server( > > > > + clp, nfs_server_return_marked_delegations, NULL); > > > > + if (err) > > > > + return err; > > > > + /* If a return was delayed, sleep to prevent hard looping > > > > */ > > > > + if (nfs_client_clear_delayed_delegations(clp)) > > > > + ssleep(1); > > > > + return 0; > > > > } > > > > > > > > /** > > > > @@ -775,13 +823,6 @@ static void > > > > nfs_mark_return_if_closed_delegation(struct nfs_server *server, > > > > set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client- > > > > > cl_state); > > > > } > > > > > > > > -static void nfs_mark_return_delegation(struct nfs_server > > > > *server, > > > > - struct nfs_delegation *delegation) > > > > -{ > > > > - set_bit(NFS_DELEGATION_RETURN, &delegation->flags); > > > > - set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client- > > > > > cl_state); > > > > -} > > > > - > > > > static bool nfs_server_mark_return_all_delegations(struct > > > > nfs_server *server) > > > > { > > > > struct nfs_delegation *delegation; > > > > diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h > > > > index c19b4fd20781..1c378992b7c0 100644 > > > > --- a/fs/nfs/delegation.h > > > > +++ b/fs/nfs/delegation.h > > > > @@ -36,6 +36,7 @@ enum { > > > > NFS_DELEGATION_REVOKED, > > > > NFS_DELEGATION_TEST_EXPIRED, > > > > NFS_DELEGATION_INODE_FREEING, > > > > + NFS_DELEGATION_RETURN_DELAYED, > > > > }; > > > > > > > > int nfs_inode_set_delegation(struct inode *inode, const struct > > > > cred *cred, > > > > diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h > > > > index 065cb04222a1..4c44322c2643 100644 > > > > --- a/fs/nfs/nfs4_fs.h > > > > +++ b/fs/nfs/nfs4_fs.h > > > > @@ -45,6 +45,7 @@ enum nfs4_client_state { > > > > NFS4CLNT_RECALL_RUNNING, > > > > NFS4CLNT_RECALL_ANY_LAYOUT_READ, > > > > NFS4CLNT_RECALL_ANY_LAYOUT_RW, > > > > + NFS4CLNT_DELEGRETURN_DELAYED, > > > > }; > > > > > > > > #define NFS4_RENEW_TIMEOUT 0x01 > > > > -- > > > > 2.31.1 > > > > -- > > Trond Myklebust > > Linux NFS client maintainer, Hammerspace > > trond.myklebust@hammerspace.com > > > > -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@hammerspace.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] NFSv4: Fix delegation return in cases where we have to retry 2021-05-20 21:14 ` Trond Myklebust @ 2021-05-21 20:23 ` bfields 0 siblings, 0 replies; 10+ messages in thread From: bfields @ 2021-05-21 20:23 UTC (permalink / raw) To: Trond Myklebust; +Cc: linux-nfs@vger.kernel.org On Thu, May 20, 2021 at 09:14:09PM +0000, Trond Myklebust wrote: > On Thu, 2021-05-20 at 16:47 -0400, bfields@fieldses.org wrote: > > On Thu, May 20, 2021 at 07:08:24PM +0000, Trond Myklebust wrote: > > > On Thu, 2021-05-20 at 14:29 -0400, J. Bruce Fields wrote: > > > > On Thu, May 20, 2021 at 12:38:59PM -0400, trondmy@kernel.org wrote: > > > > > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > > > > > > > > > If we're unable to immediately recover all locks because the > > > > > server > > > > > is > > > > > unable to immediately service our reclaim calls, then we want to > > > > > retry > > > > > after we've finished servicing all the other asynchronous > > > > > delegation > > > > > returns on our queue. > > > > > > > > So, there's a situation where the server can't service a reclaim > > > > until > > > > some other delegation is returned? I'm not seeing how that > > > > happens. > > > > > > > > > > I can and I do... pNFS can be complicated... > > > > I don't doubt you, but does everyone get this but me? > > > > Is it too complicated to explain? > > > > Not if you read the code, no. I tried, and couldn't follow it, at least not on a quick read. It's OK, I guess, it was mainly just curiosity. --b. > > In nfs_end_delegation_return(): > > do { > if (test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) > break; > err = nfs_delegation_claim_opens(inode, &delegation->stateid, > delegation->type); > if (!issync || err != -EAGAIN) > break; > /* > * Guard against state recovery > */ > err = nfs4_wait_clnt_recover(clp); > } while (err == 0); > > So if issync is false, then we abort the delegation return and try > again later. > > ...and when we later add a break_lease() call in this patch series, > then there is another condition. > > > --b. > > > > > > > > > --b. > > > > > > > > > > > > > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > > > > > --- > > > > > fs/nfs/delegation.c | 71 +++++++++++++++++++++++++++++++++++---- > > > > > -- > > > > > ---- > > > > > fs/nfs/delegation.h | 1 + > > > > > fs/nfs/nfs4_fs.h | 1 + > > > > > 3 files changed, 58 insertions(+), 15 deletions(-) > > > > > > > > > > diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c > > > > > index e6ec6f09ac6e..7c45ac3c3b0b 100644 > > > > > --- a/fs/nfs/delegation.c > > > > > +++ b/fs/nfs/delegation.c > > > > > @@ -75,6 +75,13 @@ void nfs_mark_delegation_referenced(struct > > > > > nfs_delegation *delegation) > > > > > set_bit(NFS_DELEGATION_REFERENCED, &delegation->flags); > > > > > } > > > > > > > > > > +static void nfs_mark_return_delegation(struct nfs_server > > > > > *server, > > > > > + struct nfs_delegation > > > > > *delegation) > > > > > +{ > > > > > + set_bit(NFS_DELEGATION_RETURN, &delegation->flags); > > > > > + set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client- > > > > > > cl_state); > > > > > +} > > > > > + > > > > > static bool > > > > > nfs4_is_valid_delegation(const struct nfs_delegation > > > > > *delegation, > > > > > fmode_t flags) > > > > > @@ -293,6 +300,7 @@ nfs_start_delegation_return_locked(struct > > > > > nfs_inode *nfsi) > > > > > goto out; > > > > > spin_lock(&delegation->lock); > > > > > if (!test_and_set_bit(NFS_DELEGATION_RETURNING, > > > > > &delegation->flags)) { > > > > > + clear_bit(NFS_DELEGATION_RETURN_DELAYED, > > > > > &delegation->flags); > > > > > /* Refcount matched in > > > > > nfs_end_delegation_return() > > > > > */ > > > > > ret = nfs_get_delegation(delegation); > > > > > } > > > > > @@ -314,16 +322,17 @@ nfs_start_delegation_return(struct > > > > > nfs_inode > > > > > *nfsi) > > > > > return delegation; > > > > > } > > > > > > > > > > -static void > > > > > -nfs_abort_delegation_return(struct nfs_delegation *delegation, > > > > > - struct nfs_client *clp) > > > > > +static void nfs_abort_delegation_return(struct nfs_delegation > > > > > *delegation, > > > > > + struct nfs_client *clp, > > > > > int > > > > > err) > > > > > { > > > > > > > > > > spin_lock(&delegation->lock); > > > > > clear_bit(NFS_DELEGATION_RETURNING, &delegation->flags); > > > > > - set_bit(NFS_DELEGATION_RETURN, &delegation->flags); > > > > > + if (err == -EAGAIN) { > > > > > + set_bit(NFS_DELEGATION_RETURN_DELAYED, > > > > > &delegation- > > > > > > flags); > > > > > + set_bit(NFS4CLNT_DELEGRETURN_DELAYED, &clp- > > > > > > cl_state); > > > > > + } > > > > > spin_unlock(&delegation->lock); > > > > > - set_bit(NFS4CLNT_DELEGRETURN, &clp->cl_state); > > > > > } > > > > > > > > > > static struct nfs_delegation * > > > > > @@ -539,7 +548,7 @@ static int nfs_end_delegation_return(struct > > > > > inode *inode, struct nfs_delegation > > > > > } while (err == 0); > > > > > > > > > > if (err) { > > > > > - nfs_abort_delegation_return(delegation, clp); > > > > > + nfs_abort_delegation_return(delegation, clp, > > > > > err); > > > > > goto out; > > > > > } > > > > > > > > > > @@ -568,6 +577,7 @@ static bool nfs_delegation_need_return(struct > > > > > nfs_delegation *delegation) > > > > > if (ret) > > > > > clear_bit(NFS_DELEGATION_RETURN_IF_CLOSED, > > > > > &delegation->flags); > > > > > if (test_bit(NFS_DELEGATION_RETURNING, &delegation- > > > > > >flags) > > > > > > > > > > > > + test_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation- > > > > > > flags) || > > > > > test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) > > > > > ret = false; > > > > > > > > > > @@ -647,6 +657,38 @@ static int > > > > > nfs_server_return_marked_delegations(struct nfs_server *server, > > > > > return err; > > > > > } > > > > > > > > > > +static bool nfs_server_clear_delayed_delegations(struct > > > > > nfs_server > > > > > *server) > > > > > +{ > > > > > + struct nfs_delegation *d; > > > > > + bool ret = false; > > > > > + > > > > > + list_for_each_entry_rcu (d, &server->delegations, > > > > > super_list) { > > > > > + if (!test_bit(NFS_DELEGATION_RETURN_DELAYED, &d- > > > > > > flags)) > > > > > + continue; > > > > > + nfs_mark_return_delegation(server, d); > > > > > + clear_bit(NFS_DELEGATION_RETURN_DELAYED, &d- > > > > > > flags); > > > > > + ret = true; > > > > > + } > > > > > + return ret; > > > > > +} > > > > > + > > > > > +static bool nfs_client_clear_delayed_delegations(struct > > > > > nfs_client > > > > > *clp) > > > > > +{ > > > > > + struct nfs_server *server; > > > > > + bool ret = false; > > > > > + > > > > > + if (!test_and_clear_bit(NFS4CLNT_DELEGRETURN_DELAYED, > > > > > &clp- > > > > > > cl_state)) > > > > > + goto out; > > > > > + rcu_read_lock(); > > > > > + list_for_each_entry_rcu (server, &clp->cl_superblocks, > > > > > client_link) { > > > > > + if (nfs_server_clear_delayed_delegations(server)) > > > > > + ret = true; > > > > > + } > > > > > + rcu_read_unlock(); > > > > > +out: > > > > > + return ret; > > > > > +} > > > > > + > > > > > /** > > > > > * nfs_client_return_marked_delegations - return previously > > > > > marked > > > > > delegations > > > > > * @clp: nfs_client to process > > > > > @@ -659,8 +701,14 @@ static int > > > > > nfs_server_return_marked_delegations(struct nfs_server *server, > > > > > */ > > > > > int nfs_client_return_marked_delegations(struct nfs_client *clp) > > > > > { > > > > > - return nfs_client_for_each_server(clp, > > > > > - nfs_server_return_marked_delegations, > > > > > NULL); > > > > > + int err = nfs_client_for_each_server( > > > > > + clp, nfs_server_return_marked_delegations, NULL); > > > > > + if (err) > > > > > + return err; > > > > > + /* If a return was delayed, sleep to prevent hard looping > > > > > */ > > > > > + if (nfs_client_clear_delayed_delegations(clp)) > > > > > + ssleep(1); > > > > > + return 0; > > > > > } > > > > > > > > > > /** > > > > > @@ -775,13 +823,6 @@ static void > > > > > nfs_mark_return_if_closed_delegation(struct nfs_server *server, > > > > > set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client- > > > > > > cl_state); > > > > > } > > > > > > > > > > -static void nfs_mark_return_delegation(struct nfs_server > > > > > *server, > > > > > - struct nfs_delegation *delegation) > > > > > -{ > > > > > - set_bit(NFS_DELEGATION_RETURN, &delegation->flags); > > > > > - set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client- > > > > > > cl_state); > > > > > -} > > > > > - > > > > > static bool nfs_server_mark_return_all_delegations(struct > > > > > nfs_server *server) > > > > > { > > > > > struct nfs_delegation *delegation; > > > > > diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h > > > > > index c19b4fd20781..1c378992b7c0 100644 > > > > > --- a/fs/nfs/delegation.h > > > > > +++ b/fs/nfs/delegation.h > > > > > @@ -36,6 +36,7 @@ enum { > > > > > NFS_DELEGATION_REVOKED, > > > > > NFS_DELEGATION_TEST_EXPIRED, > > > > > NFS_DELEGATION_INODE_FREEING, > > > > > + NFS_DELEGATION_RETURN_DELAYED, > > > > > }; > > > > > > > > > > int nfs_inode_set_delegation(struct inode *inode, const struct > > > > > cred *cred, > > > > > diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h > > > > > index 065cb04222a1..4c44322c2643 100644 > > > > > --- a/fs/nfs/nfs4_fs.h > > > > > +++ b/fs/nfs/nfs4_fs.h > > > > > @@ -45,6 +45,7 @@ enum nfs4_client_state { > > > > > NFS4CLNT_RECALL_RUNNING, > > > > > NFS4CLNT_RECALL_ANY_LAYOUT_READ, > > > > > NFS4CLNT_RECALL_ANY_LAYOUT_RW, > > > > > + NFS4CLNT_DELEGRETURN_DELAYED, > > > > > }; > > > > > > > > > > #define NFS4_RENEW_TIMEOUT 0x01 > > > > > -- > > > > > 2.31.1 > > > > > > -- > > > Trond Myklebust > > > Linux NFS client maintainer, Hammerspace > > > trond.myklebust@hammerspace.com > > > > > > > > -- > Trond Myklebust > Linux NFS client maintainer, Hammerspace > trond.myklebust@hammerspace.com > > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-05-21 20:23 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-05-20 16:38 [PATCH 0/4] Add support for application leases to NFSv4 trondmy 2021-05-20 16:38 ` [PATCH 1/4] NFSv4: Fix delegation return in cases where we have to retry trondmy 2021-05-20 16:39 ` [PATCH 2/4] NFSv4: Add lease breakpoints in case of a delegation recall or return trondmy 2021-05-20 16:39 ` [PATCH 3/4] NFSv4: Add support for application leases underpinned by a delegation trondmy 2021-05-20 16:39 ` [PATCH 4/4] NFS: nfs_find_open_context() may only select open files trondmy 2021-05-20 18:29 ` [PATCH 1/4] NFSv4: Fix delegation return in cases where we have to retry J. Bruce Fields 2021-05-20 19:08 ` Trond Myklebust 2021-05-20 20:47 ` bfields 2021-05-20 21:14 ` Trond Myklebust 2021-05-21 20:23 ` bfields
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox