* [PATCH RFC v17 01/11] fs/lock: add helper locks_owner_has_blockers to check for blockers
2022-03-17 7:43 [PATCH RFC v17 0/11] NFSD: Initial implementation of NFSv4 Courteous Server Dai Ngo
@ 2022-03-17 7:43 ` Dai Ngo
2022-03-17 15:04 ` Chuck Lever III
2022-03-17 17:39 ` Jeff Layton
2022-03-17 7:43 ` [PATCH RFC v17 02/11] NFSD: Add courtesy client state, macro and spinlock to support courteous server Dai Ngo
` (9 subsequent siblings)
10 siblings, 2 replies; 21+ messages in thread
From: Dai Ngo @ 2022-03-17 7:43 UTC (permalink / raw)
To: chuck.lever, bfields; +Cc: jlayton, viro, linux-nfs, linux-fsdevel
Add helper locks_owner_has_blockers to check if there is any blockers
for a given lockowner.
Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
fs/locks.c | 28 ++++++++++++++++++++++++++++
include/linux/fs.h | 7 +++++++
2 files changed, 35 insertions(+)
diff --git a/fs/locks.c b/fs/locks.c
index 050acf8b5110..53864eb99dc5 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -300,6 +300,34 @@ void locks_release_private(struct file_lock *fl)
}
EXPORT_SYMBOL_GPL(locks_release_private);
+/**
+ * locks_owner_has_blockers - Check for blocking lock requests
+ * @flctx: file lock context
+ * @owner: lock owner
+ *
+ * Return values:
+ * %true: @owner has at least one blocker
+ * %false: @owner has no blockers
+ */
+bool locks_owner_has_blockers(struct file_lock_context *flctx,
+ fl_owner_t owner)
+{
+ struct file_lock *fl;
+
+ spin_lock(&flctx->flc_lock);
+ list_for_each_entry(fl, &flctx->flc_posix, fl_list) {
+ if (fl->fl_owner != owner)
+ continue;
+ if (!list_empty(&fl->fl_blocked_requests)) {
+ spin_unlock(&flctx->flc_lock);
+ return true;
+ }
+ }
+ spin_unlock(&flctx->flc_lock);
+ return false;
+}
+EXPORT_SYMBOL_GPL(locks_owner_has_blockers);
+
/* Free a lock which is not in use. */
void locks_free_lock(struct file_lock *fl)
{
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 831b20430d6e..2057a9df790f 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1200,6 +1200,8 @@ extern void lease_unregister_notifier(struct notifier_block *);
struct files_struct;
extern void show_fd_locks(struct seq_file *f,
struct file *filp, struct files_struct *files);
+extern bool locks_owner_has_blockers(struct file_lock_context *flctx,
+ fl_owner_t owner);
#else /* !CONFIG_FILE_LOCKING */
static inline int fcntl_getlk(struct file *file, unsigned int cmd,
struct flock __user *user)
@@ -1335,6 +1337,11 @@ static inline int lease_modify(struct file_lock *fl, int arg,
struct files_struct;
static inline void show_fd_locks(struct seq_file *f,
struct file *filp, struct files_struct *files) {}
+static inline bool locks_owner_has_blockers(struct file_lock_context *flctx,
+ fl_owner_t owner)
+{
+ return false;
+}
#endif /* !CONFIG_FILE_LOCKING */
static inline struct inode *file_inode(const struct file *f)
--
2.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH RFC v17 01/11] fs/lock: add helper locks_owner_has_blockers to check for blockers
2022-03-17 7:43 ` [PATCH RFC v17 01/11] fs/lock: add helper locks_owner_has_blockers to check for blockers Dai Ngo
@ 2022-03-17 15:04 ` Chuck Lever III
2022-03-17 17:39 ` Jeff Layton
1 sibling, 0 replies; 21+ messages in thread
From: Chuck Lever III @ 2022-03-17 15:04 UTC (permalink / raw)
To: Jeff Layton
Cc: Bruce Fields, Dai Ngo, Al Viro, Linux NFS Mailing List, linux-fsdevel
> On Mar 17, 2022, at 3:43 AM, Dai Ngo <dai.ngo@oracle.com> wrote:
>
> Add helper locks_owner_has_blockers to check if there is any blockers
> for a given lockowner.
>
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
Hi Jeff, can we get an R-b or Acked from you for this one?
> ---
> fs/locks.c | 28 ++++++++++++++++++++++++++++
> include/linux/fs.h | 7 +++++++
> 2 files changed, 35 insertions(+)
>
> diff --git a/fs/locks.c b/fs/locks.c
> index 050acf8b5110..53864eb99dc5 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -300,6 +300,34 @@ void locks_release_private(struct file_lock *fl)
> }
> EXPORT_SYMBOL_GPL(locks_release_private);
>
> +/**
> + * locks_owner_has_blockers - Check for blocking lock requests
> + * @flctx: file lock context
> + * @owner: lock owner
> + *
> + * Return values:
> + * %true: @owner has at least one blocker
> + * %false: @owner has no blockers
> + */
> +bool locks_owner_has_blockers(struct file_lock_context *flctx,
> + fl_owner_t owner)
> +{
> + struct file_lock *fl;
> +
> + spin_lock(&flctx->flc_lock);
> + list_for_each_entry(fl, &flctx->flc_posix, fl_list) {
> + if (fl->fl_owner != owner)
> + continue;
> + if (!list_empty(&fl->fl_blocked_requests)) {
> + spin_unlock(&flctx->flc_lock);
> + return true;
> + }
> + }
> + spin_unlock(&flctx->flc_lock);
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(locks_owner_has_blockers);
> +
> /* Free a lock which is not in use. */
> void locks_free_lock(struct file_lock *fl)
> {
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 831b20430d6e..2057a9df790f 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1200,6 +1200,8 @@ extern void lease_unregister_notifier(struct notifier_block *);
> struct files_struct;
> extern void show_fd_locks(struct seq_file *f,
> struct file *filp, struct files_struct *files);
> +extern bool locks_owner_has_blockers(struct file_lock_context *flctx,
> + fl_owner_t owner);
> #else /* !CONFIG_FILE_LOCKING */
> static inline int fcntl_getlk(struct file *file, unsigned int cmd,
> struct flock __user *user)
> @@ -1335,6 +1337,11 @@ static inline int lease_modify(struct file_lock *fl, int arg,
> struct files_struct;
> static inline void show_fd_locks(struct seq_file *f,
> struct file *filp, struct files_struct *files) {}
> +static inline bool locks_owner_has_blockers(struct file_lock_context *flctx,
> + fl_owner_t owner)
> +{
> + return false;
> +}
> #endif /* !CONFIG_FILE_LOCKING */
>
> static inline struct inode *file_inode(const struct file *f)
> --
> 2.9.5
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH RFC v17 01/11] fs/lock: add helper locks_owner_has_blockers to check for blockers
2022-03-17 7:43 ` [PATCH RFC v17 01/11] fs/lock: add helper locks_owner_has_blockers to check for blockers Dai Ngo
2022-03-17 15:04 ` Chuck Lever III
@ 2022-03-17 17:39 ` Jeff Layton
2022-03-17 17:46 ` Chuck Lever III
1 sibling, 1 reply; 21+ messages in thread
From: Jeff Layton @ 2022-03-17 17:39 UTC (permalink / raw)
To: Dai Ngo, chuck.lever, bfields; +Cc: viro, linux-nfs, linux-fsdevel
On Thu, 2022-03-17 at 00:43 -0700, Dai Ngo wrote:
> Add helper locks_owner_has_blockers to check if there is any blockers
> for a given lockowner.
>
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
> fs/locks.c | 28 ++++++++++++++++++++++++++++
> include/linux/fs.h | 7 +++++++
> 2 files changed, 35 insertions(+)
>
> diff --git a/fs/locks.c b/fs/locks.c
> index 050acf8b5110..53864eb99dc5 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -300,6 +300,34 @@ void locks_release_private(struct file_lock *fl)
> }
> EXPORT_SYMBOL_GPL(locks_release_private);
>
> +/**
> + * locks_owner_has_blockers - Check for blocking lock requests
> + * @flctx: file lock context
> + * @owner: lock owner
> + *
> + * Return values:
> + * %true: @owner has at least one blocker
> + * %false: @owner has no blockers
> + */
> +bool locks_owner_has_blockers(struct file_lock_context *flctx,
> + fl_owner_t owner)
> +{
> + struct file_lock *fl;
> +
> + spin_lock(&flctx->flc_lock);
> + list_for_each_entry(fl, &flctx->flc_posix, fl_list) {
> + if (fl->fl_owner != owner)
> + continue;
> + if (!list_empty(&fl->fl_blocked_requests)) {
> + spin_unlock(&flctx->flc_lock);
> + return true;
> + }
> + }
> + spin_unlock(&flctx->flc_lock);
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(locks_owner_has_blockers);
> +
> /* Free a lock which is not in use. */
> void locks_free_lock(struct file_lock *fl)
> {
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 831b20430d6e..2057a9df790f 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1200,6 +1200,8 @@ extern void lease_unregister_notifier(struct notifier_block *);
> struct files_struct;
> extern void show_fd_locks(struct seq_file *f,
> struct file *filp, struct files_struct *files);
> +extern bool locks_owner_has_blockers(struct file_lock_context *flctx,
> + fl_owner_t owner);
> #else /* !CONFIG_FILE_LOCKING */
> static inline int fcntl_getlk(struct file *file, unsigned int cmd,
> struct flock __user *user)
> @@ -1335,6 +1337,11 @@ static inline int lease_modify(struct file_lock *fl, int arg,
> struct files_struct;
> static inline void show_fd_locks(struct seq_file *f,
> struct file *filp, struct files_struct *files) {}
> +static inline bool locks_owner_has_blockers(struct file_lock_context *flctx,
> + fl_owner_t owner)
> +{
> + return false;
> +}
> #endif /* !CONFIG_FILE_LOCKING */
>
> static inline struct inode *file_inode(const struct file *f)
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH RFC v17 01/11] fs/lock: add helper locks_owner_has_blockers to check for blockers
2022-03-17 17:39 ` Jeff Layton
@ 2022-03-17 17:46 ` Chuck Lever III
0 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever III @ 2022-03-17 17:46 UTC (permalink / raw)
To: Jeff Layton
Cc: Dai Ngo, Bruce Fields, Al Viro, Linux NFS Mailing List, linux-fsdevel
> On Mar 17, 2022, at 1:39 PM, Jeff Layton <jlayton@kernel.org> wrote:
>
> On Thu, 2022-03-17 at 00:43 -0700, Dai Ngo wrote:
>> Add helper locks_owner_has_blockers to check if there is any blockers
>> for a given lockowner.
>>
>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
>> ---
>> fs/locks.c | 28 ++++++++++++++++++++++++++++
>> include/linux/fs.h | 7 +++++++
>> 2 files changed, 35 insertions(+)
>>
>> diff --git a/fs/locks.c b/fs/locks.c
>> index 050acf8b5110..53864eb99dc5 100644
>> --- a/fs/locks.c
>> +++ b/fs/locks.c
>> @@ -300,6 +300,34 @@ void locks_release_private(struct file_lock *fl)
>> }
>> EXPORT_SYMBOL_GPL(locks_release_private);
>>
>> +/**
>> + * locks_owner_has_blockers - Check for blocking lock requests
>> + * @flctx: file lock context
>> + * @owner: lock owner
>> + *
>> + * Return values:
>> + * %true: @owner has at least one blocker
>> + * %false: @owner has no blockers
>> + */
>> +bool locks_owner_has_blockers(struct file_lock_context *flctx,
>> + fl_owner_t owner)
>> +{
>> + struct file_lock *fl;
>> +
>> + spin_lock(&flctx->flc_lock);
>> + list_for_each_entry(fl, &flctx->flc_posix, fl_list) {
>> + if (fl->fl_owner != owner)
>> + continue;
>> + if (!list_empty(&fl->fl_blocked_requests)) {
>> + spin_unlock(&flctx->flc_lock);
>> + return true;
>> + }
>> + }
>> + spin_unlock(&flctx->flc_lock);
>> + return false;
>> +}
>> +EXPORT_SYMBOL_GPL(locks_owner_has_blockers);
>> +
>> /* Free a lock which is not in use. */
>> void locks_free_lock(struct file_lock *fl)
>> {
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index 831b20430d6e..2057a9df790f 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -1200,6 +1200,8 @@ extern void lease_unregister_notifier(struct notifier_block *);
>> struct files_struct;
>> extern void show_fd_locks(struct seq_file *f,
>> struct file *filp, struct files_struct *files);
>> +extern bool locks_owner_has_blockers(struct file_lock_context *flctx,
>> + fl_owner_t owner);
>> #else /* !CONFIG_FILE_LOCKING */
>> static inline int fcntl_getlk(struct file *file, unsigned int cmd,
>> struct flock __user *user)
>> @@ -1335,6 +1337,11 @@ static inline int lease_modify(struct file_lock *fl, int arg,
>> struct files_struct;
>> static inline void show_fd_locks(struct seq_file *f,
>> struct file *filp, struct files_struct *files) {}
>> +static inline bool locks_owner_has_blockers(struct file_lock_context *flctx,
>> + fl_owner_t owner)
>> +{
>> + return false;
>> +}
>> #endif /* !CONFIG_FILE_LOCKING */
>>
>> static inline struct inode *file_inode(const struct file *f)
>
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
Thanks!
--
Chuck Lever
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH RFC v17 02/11] NFSD: Add courtesy client state, macro and spinlock to support courteous server
2022-03-17 7:43 [PATCH RFC v17 0/11] NFSD: Initial implementation of NFSv4 Courteous Server Dai Ngo
2022-03-17 7:43 ` [PATCH RFC v17 01/11] fs/lock: add helper locks_owner_has_blockers to check for blockers Dai Ngo
@ 2022-03-17 7:43 ` Dai Ngo
2022-03-17 7:43 ` [PATCH RFC v17 03/11] NFSD: Add lm_lock_expired call out Dai Ngo
` (8 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Dai Ngo @ 2022-03-17 7:43 UTC (permalink / raw)
To: chuck.lever, bfields; +Cc: jlayton, viro, linux-nfs, linux-fsdevel
Update nfs4_client to add:
. cl_cs_client_state: courtesy client state
. cl_cs_lock: spinlock to synchronize access to cl_cs_client_state
. cl_cs_list: list used by laundromat to process courtesy clients
Modify alloc_client to initialize these fields.
Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
fs/nfsd/nfs4state.c | 2 ++
fs/nfsd/nfsd.h | 1 +
fs/nfsd/state.h | 33 +++++++++++++++++++++++++++++++++
3 files changed, 36 insertions(+)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 234e852fcdfa..a65d59510681 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2009,12 +2009,14 @@ static struct nfs4_client *alloc_client(struct xdr_netobj name)
INIT_LIST_HEAD(&clp->cl_delegations);
INIT_LIST_HEAD(&clp->cl_lru);
INIT_LIST_HEAD(&clp->cl_revoked);
+ INIT_LIST_HEAD(&clp->cl_cs_list);
#ifdef CONFIG_NFSD_PNFS
INIT_LIST_HEAD(&clp->cl_lo_states);
#endif
INIT_LIST_HEAD(&clp->async_copies);
spin_lock_init(&clp->async_lock);
spin_lock_init(&clp->cl_lock);
+ spin_lock_init(&clp->cl_cs_lock);
rpc_init_wait_queue(&clp->cl_cb_waitq, "Backchannel slot table");
return clp;
err_no_hashtbl:
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 4fc1fd639527..23996c6ca75e 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -336,6 +336,7 @@ void nfsd_lockd_shutdown(void);
#define COMPOUND_ERR_SLACK_SPACE 16 /* OP_SETATTR */
#define NFSD_LAUNDROMAT_MINTIMEOUT 1 /* seconds */
+#define NFSD_COURTESY_CLIENT_TIMEOUT (24 * 60 * 60) /* seconds */
/*
* The following attributes are currently not supported by the NFSv4 server:
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 95457cfd37fc..40e390abc842 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -283,6 +283,35 @@ struct nfsd4_sessionid {
#define HEXDIR_LEN 33 /* hex version of 16 byte md5 of cl_name plus '\0' */
/*
+ * CLIENT_ CLIENT_ CLIENT_
+ * COURTESY EXPIRED RECONNECTED Meaning Where set
+ * -----------------------------------------------------------------------------
+ * | false | false | false | Confirmed, active | Default |
+ * |---------------------------------------------------------------------------|
+ * | true | false | false | Courtesy state. | nfs4_get_client_reaplist |
+ * | | | | Lease/lock/share | |
+ * | | | | reservation conflict | |
+ * | | | | can cause Courtesy | |
+ * | | | | client to be expired | |
+ * |---------------------------------------------------------------------------|
+ * | false | true | false | Courtesy client to be| nfs4_laundromat |
+ * | | | | expired by Laundromat| nfsd4_lm_lock_expired |
+ * | | | | due to conflict | nfsd4_discard_courtesy_clnt |
+ * | | | | | nfsd4_expire_courtesy_clnt |
+ * |---------------------------------------------------------------------------|
+ * | false | false | true | Courtesy client | nfsd4_courtesy_clnt_expired|
+ * | | | | reconnected, | |
+ * | | | | becoming active | |
+ * -----------------------------------------------------------------------------
+ */
+
+enum courtesy_client_state {
+ NFSD4_CLIENT_COURTESY = 1,
+ NFSD4_CLIENT_EXPIRED,
+ NFSD4_CLIENT_RECONNECTED,
+};
+
+/*
* struct nfs4_client - one per client. Clientids live here.
*
* The initial object created by an NFS client using SETCLIENTID (for NFSv4.0)
@@ -385,6 +414,10 @@ struct nfs4_client {
struct list_head async_copies; /* list of async copies */
spinlock_t async_lock; /* lock for async copies */
atomic_t cl_cb_inflight; /* Outstanding callbacks */
+
+ enum courtesy_client_state cl_cs_client_state;
+ spinlock_t cl_cs_lock;
+ struct list_head cl_cs_list;
};
/* struct nfs4_client_reset
--
2.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH RFC v17 03/11] NFSD: Add lm_lock_expired call out
2022-03-17 7:43 [PATCH RFC v17 0/11] NFSD: Initial implementation of NFSv4 Courteous Server Dai Ngo
2022-03-17 7:43 ` [PATCH RFC v17 01/11] fs/lock: add helper locks_owner_has_blockers to check for blockers Dai Ngo
2022-03-17 7:43 ` [PATCH RFC v17 02/11] NFSD: Add courtesy client state, macro and spinlock to support courteous server Dai Ngo
@ 2022-03-17 7:43 ` Dai Ngo
2022-03-17 7:43 ` [PATCH RFC v17 04/11] NFSD: Update nfsd_breaker_owns_lease() to handle courtesy clients Dai Ngo
` (7 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Dai Ngo @ 2022-03-17 7:43 UTC (permalink / raw)
To: chuck.lever, bfields; +Cc: jlayton, viro, linux-nfs, linux-fsdevel
Add callout function nfsd4_lm_lock_expired for lm_lock_expired.
If lock request has conflict with courtesy client then expire the
courtesy client and return no conflict to caller.
Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
fs/nfsd/nfs4state.c | 22 ++++++++++++++++++++++
fs/nfsd/state.h | 14 ++++++++++++++
2 files changed, 36 insertions(+)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a65d59510681..80772662236b 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6578,10 +6578,32 @@ nfsd4_lm_notify(struct file_lock *fl)
}
}
+/**
+ * nfsd4_lm_lock_expired - check if lock conflict can be resolved.
+ *
+ * @fl: pointer to file_lock with a potential conflict
+ * Return values:
+ * %false: real conflict, lock conflict can not be resolved.
+ * %true: no conflict, lock conflict was resolved.
+ *
+ * Note that this function is called while the flc_lock is held.
+ */
+static bool
+nfsd4_lm_lock_expired(struct file_lock *fl)
+{
+ struct nfs4_lockowner *lo;
+
+ if (!fl)
+ return false;
+ lo = (struct nfs4_lockowner *)fl->fl_owner;
+ return nfsd4_expire_courtesy_clnt(lo->lo_owner.so_client);
+}
+
static const struct lock_manager_operations nfsd_posix_mng_ops = {
.lm_notify = nfsd4_lm_notify,
.lm_get_owner = nfsd4_lm_get_owner,
.lm_put_owner = nfsd4_lm_put_owner,
+ .lm_lock_expired = nfsd4_lm_lock_expired,
};
static inline void
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 40e390abc842..d156ae3ab46c 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -735,4 +735,18 @@ extern void nfsd4_client_record_remove(struct nfs4_client *clp);
extern int nfsd4_client_record_check(struct nfs4_client *clp);
extern void nfsd4_record_grace_done(struct nfsd_net *nn);
+static inline bool
+nfsd4_expire_courtesy_clnt(struct nfs4_client *clp)
+{
+ bool rc = false;
+
+ spin_lock(&clp->cl_cs_lock);
+ if (clp->cl_cs_client_state == NFSD4_CLIENT_COURTESY)
+ clp->cl_cs_client_state = NFSD4_CLIENT_EXPIRED;
+ if (clp->cl_cs_client_state == NFSD4_CLIENT_EXPIRED)
+ rc = true;
+ spin_unlock(&clp->cl_cs_lock);
+ return rc;
+}
+
#endif /* NFSD4_STATE_H */
--
2.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH RFC v17 04/11] NFSD: Update nfsd_breaker_owns_lease() to handle courtesy clients
2022-03-17 7:43 [PATCH RFC v17 0/11] NFSD: Initial implementation of NFSv4 Courteous Server Dai Ngo
` (2 preceding siblings ...)
2022-03-17 7:43 ` [PATCH RFC v17 03/11] NFSD: Add lm_lock_expired call out Dai Ngo
@ 2022-03-17 7:43 ` Dai Ngo
2022-03-17 7:43 ` [PATCH RFC v17 05/11] NFSD: Update nfs4_get_vfs_file() to handle courtesy client Dai Ngo
` (6 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Dai Ngo @ 2022-03-17 7:43 UTC (permalink / raw)
To: chuck.lever, bfields; +Cc: jlayton, viro, linux-nfs, linux-fsdevel
Update nfsd_breaker_owns_lease() to handle delegation conflict with
courtesy clients by calling nfsd4_expire_courtesy_clnt.
Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
fs/nfsd/nfs4state.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 80772662236b..f20c75890594 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4727,6 +4727,9 @@ static bool nfsd_breaker_owns_lease(struct file_lock *fl)
struct svc_rqst *rqst;
struct nfs4_client *clp;
+ if (nfsd4_expire_courtesy_clnt(dl->dl_stid.sc_client))
+ return true;
+
if (!i_am_nfsd())
return false;
rqst = kthread_data(current);
--
2.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH RFC v17 05/11] NFSD: Update nfs4_get_vfs_file() to handle courtesy client
2022-03-17 7:43 [PATCH RFC v17 0/11] NFSD: Initial implementation of NFSv4 Courteous Server Dai Ngo
` (3 preceding siblings ...)
2022-03-17 7:43 ` [PATCH RFC v17 04/11] NFSD: Update nfsd_breaker_owns_lease() to handle courtesy clients Dai Ngo
@ 2022-03-17 7:43 ` Dai Ngo
2022-03-21 21:44 ` J. Bruce Fields
2022-03-17 7:43 ` [PATCH RFC v17 06/11] NFSD: Update find_clp_in_name_tree() " Dai Ngo
` (5 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Dai Ngo @ 2022-03-17 7:43 UTC (permalink / raw)
To: chuck.lever, bfields; +Cc: jlayton, viro, linux-nfs, linux-fsdevel
Update nfs4_get_vfs_file and nfs4_upgrade_open to handle share
reservation conflict with courtesy client.
Update nfs4_get_vfs_file and nfs4_upgrade_open to handle share
reservation conflict with courtesy client.
When we have deny/access conflict we walk the fi_stateids of the
file in question, looking for open stateid and check the deny/access
of that stateid against the one from the open request. If there is
a conflict then we check if the client that owns that stateid is
a courtesy client. If it is then we set the client state to
CLIENT_EXPIRED and allow the open request to continue. We have
to scan all the stateid's of the file since the conflict can be
caused by multiple open stateid's.
Client with CLIENT_EXPIRED is expired by the laundromat.
Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
fs/nfsd/nfs4state.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 91 insertions(+), 8 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index f20c75890594..c6b5e05c9c34 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4950,9 +4950,75 @@ nfsd4_truncate(struct svc_rqst *rqstp, struct svc_fh *fh,
return nfsd_setattr(rqstp, fh, &iattr, 0, (time64_t)0);
}
+static bool
+nfs4_check_access_deny_bmap(struct nfs4_ol_stateid *stp, u32 access,
+ bool share_access)
+{
+ if (share_access) {
+ if (!stp->st_deny_bmap)
+ return false;
+
+ if ((stp->st_deny_bmap & (1 << NFS4_SHARE_DENY_BOTH)) ||
+ (access & NFS4_SHARE_ACCESS_READ &&
+ stp->st_deny_bmap & (1 << NFS4_SHARE_DENY_READ)) ||
+ (access & NFS4_SHARE_ACCESS_WRITE &&
+ stp->st_deny_bmap & (1 << NFS4_SHARE_DENY_WRITE))) {
+ return true;
+ }
+ return false;
+ }
+ if ((access & NFS4_SHARE_DENY_BOTH) ||
+ (access & NFS4_SHARE_DENY_READ &&
+ stp->st_access_bmap & (1 << NFS4_SHARE_ACCESS_READ)) ||
+ (access & NFS4_SHARE_DENY_WRITE &&
+ stp->st_access_bmap & (1 << NFS4_SHARE_ACCESS_WRITE))) {
+ return true;
+ }
+ return false;
+}
+
+/*
+ * Check whether courtesy clients have conflicting access
+ *
+ * access: is op_share_access if share_access is true.
+ * Check if access mode, op_share_access, would conflict with
+ * the current deny mode of the file 'fp'.
+ * access: is op_share_deny if share_access is false.
+ * Check if the deny mode, op_share_deny, would conflict with
+ * current access of the file 'fp'.
+ * stp: skip checking this entry.
+ * new_stp: normal open, not open upgrade.
+ *
+ * Function returns:
+ * true - access/deny mode conflict with normal client.
+ * false - no conflict or conflict with courtesy client(s) is resolved.
+ */
+static bool
+nfs4_resolve_deny_conflicts_locked(struct nfs4_file *fp, bool new_stp,
+ struct nfs4_ol_stateid *stp, u32 access, bool share_access)
+{
+ struct nfs4_ol_stateid *st;
+ struct nfs4_client *clp;
+ bool conflict = false;
+
+ lockdep_assert_held(&fp->fi_lock);
+ list_for_each_entry(st, &fp->fi_stateids, st_perfile) {
+ if (st->st_openstp || (st == stp && new_stp) ||
+ (!nfs4_check_access_deny_bmap(st,
+ access, share_access)))
+ continue;
+ clp = st->st_stid.sc_client;
+ if (nfsd4_expire_courtesy_clnt(clp))
+ continue;
+ conflict = true;
+ break;
+ }
+ return conflict;
+}
+
static __be32 nfs4_get_vfs_file(struct svc_rqst *rqstp, struct nfs4_file *fp,
struct svc_fh *cur_fh, struct nfs4_ol_stateid *stp,
- struct nfsd4_open *open)
+ struct nfsd4_open *open, bool new_stp)
{
struct nfsd_file *nf = NULL;
__be32 status;
@@ -4968,15 +5034,29 @@ static __be32 nfs4_get_vfs_file(struct svc_rqst *rqstp, struct nfs4_file *fp,
*/
status = nfs4_file_check_deny(fp, open->op_share_deny);
if (status != nfs_ok) {
- spin_unlock(&fp->fi_lock);
- goto out;
+ if (status != nfserr_share_denied) {
+ spin_unlock(&fp->fi_lock);
+ goto out;
+ }
+ if (nfs4_resolve_deny_conflicts_locked(fp, new_stp,
+ stp, open->op_share_deny, false)) {
+ spin_unlock(&fp->fi_lock);
+ goto out;
+ }
}
/* set access to the file */
status = nfs4_file_get_access(fp, open->op_share_access);
if (status != nfs_ok) {
- spin_unlock(&fp->fi_lock);
- goto out;
+ if (status != nfserr_share_denied) {
+ spin_unlock(&fp->fi_lock);
+ goto out;
+ }
+ if (nfs4_resolve_deny_conflicts_locked(fp, new_stp,
+ stp, open->op_share_access, true)) {
+ spin_unlock(&fp->fi_lock);
+ goto out;
+ }
}
/* Set access bits in stateid */
@@ -5027,7 +5107,7 @@ nfs4_upgrade_open(struct svc_rqst *rqstp, struct nfs4_file *fp, struct svc_fh *c
unsigned char old_deny_bmap = stp->st_deny_bmap;
if (!test_access(open->op_share_access, stp))
- return nfs4_get_vfs_file(rqstp, fp, cur_fh, stp, open);
+ return nfs4_get_vfs_file(rqstp, fp, cur_fh, stp, open, false);
/* test and set deny mode */
spin_lock(&fp->fi_lock);
@@ -5036,7 +5116,10 @@ nfs4_upgrade_open(struct svc_rqst *rqstp, struct nfs4_file *fp, struct svc_fh *c
set_deny(open->op_share_deny, stp);
fp->fi_share_deny |=
(open->op_share_deny & NFS4_SHARE_DENY_BOTH);
- }
+ } else if (status == nfserr_share_denied &&
+ !nfs4_resolve_deny_conflicts_locked(fp, false, stp,
+ open->op_share_deny, false))
+ status = nfs_ok;
spin_unlock(&fp->fi_lock);
if (status != nfs_ok)
@@ -5376,7 +5459,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
goto out;
}
} else {
- status = nfs4_get_vfs_file(rqstp, fp, current_fh, stp, open);
+ status = nfs4_get_vfs_file(rqstp, fp, current_fh, stp, open, true);
if (status) {
stp->st_stid.sc_type = NFS4_CLOSED_STID;
release_open_stateid(stp);
--
2.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH RFC v17 05/11] NFSD: Update nfs4_get_vfs_file() to handle courtesy client
2022-03-17 7:43 ` [PATCH RFC v17 05/11] NFSD: Update nfs4_get_vfs_file() to handle courtesy client Dai Ngo
@ 2022-03-21 21:44 ` J. Bruce Fields
2022-03-23 0:42 ` dai.ngo
0 siblings, 1 reply; 21+ messages in thread
From: J. Bruce Fields @ 2022-03-21 21:44 UTC (permalink / raw)
To: Dai Ngo; +Cc: chuck.lever, jlayton, viro, linux-nfs, linux-fsdevel
On Thu, Mar 17, 2022 at 12:43:42AM -0700, Dai Ngo wrote:
> Update nfs4_get_vfs_file and nfs4_upgrade_open to handle share
> reservation conflict with courtesy client.
>
> Update nfs4_get_vfs_file and nfs4_upgrade_open to handle share
> reservation conflict with courtesy client.
>
> When we have deny/access conflict we walk the fi_stateids of the
> file in question, looking for open stateid and check the deny/access
> of that stateid against the one from the open request. If there is
> a conflict then we check if the client that owns that stateid is
> a courtesy client. If it is then we set the client state to
> CLIENT_EXPIRED and allow the open request to continue. We have
> to scan all the stateid's of the file since the conflict can be
> caused by multiple open stateid's.
>
> Client with CLIENT_EXPIRED is expired by the laundromat.
>
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
> fs/nfsd/nfs4state.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 91 insertions(+), 8 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index f20c75890594..c6b5e05c9c34 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4950,9 +4950,75 @@ nfsd4_truncate(struct svc_rqst *rqstp, struct svc_fh *fh,
> return nfsd_setattr(rqstp, fh, &iattr, 0, (time64_t)0);
> }
>
> +static bool
> +nfs4_check_access_deny_bmap(struct nfs4_ol_stateid *stp, u32 access,
> + bool share_access)
> +{
> + if (share_access) {
> + if (!stp->st_deny_bmap)
> + return false;
> +
> + if ((stp->st_deny_bmap & (1 << NFS4_SHARE_DENY_BOTH)) ||
> + (access & NFS4_SHARE_ACCESS_READ &&
> + stp->st_deny_bmap & (1 << NFS4_SHARE_DENY_READ)) ||
> + (access & NFS4_SHARE_ACCESS_WRITE &&
> + stp->st_deny_bmap & (1 << NFS4_SHARE_DENY_WRITE))) {
> + return true;
> + }
> + return false;
Looks like you could do this with just:
return share_access & bmap_to_share_mode(stp->st_deny_bmap);
> + }
> + if ((access & NFS4_SHARE_DENY_BOTH) ||
> + (access & NFS4_SHARE_DENY_READ &&
> + stp->st_access_bmap & (1 << NFS4_SHARE_ACCESS_READ)) ||
> + (access & NFS4_SHARE_DENY_WRITE &&
> + stp->st_access_bmap & (1 << NFS4_SHARE_ACCESS_WRITE))) {
> + return true;
> + }
Likewise.
Also, I think it'd be simpler to check for both access and deny
conflicts here, instead of just one or the other.
> + return false;
> +}
> +
> +/*
> + * Check whether courtesy clients have conflicting access
> + *
> + * access: is op_share_access if share_access is true.
> + * Check if access mode, op_share_access, would conflict with
> + * the current deny mode of the file 'fp'.
> + * access: is op_share_deny if share_access is false.
> + * Check if the deny mode, op_share_deny, would conflict with
> + * current access of the file 'fp'.
> + * stp: skip checking this entry.
> + * new_stp: normal open, not open upgrade.
> + *
> + * Function returns:
> + * true - access/deny mode conflict with normal client.
> + * false - no conflict or conflict with courtesy client(s) is resolved.
> + */
> +static bool
> +nfs4_resolve_deny_conflicts_locked(struct nfs4_file *fp, bool new_stp,
> + struct nfs4_ol_stateid *stp, u32 access, bool share_access)
> +{
> + struct nfs4_ol_stateid *st;
> + struct nfs4_client *clp;
> + bool conflict = false;
> +
> + lockdep_assert_held(&fp->fi_lock);
> + list_for_each_entry(st, &fp->fi_stateids, st_perfile) {
> + if (st->st_openstp || (st == stp && new_stp) ||
I'd split this into separate if statements and add a comment at least
for the st_openstp case, which isn't too obvious:
if (st->st_openstp) /* ignore lock stateids */
continue;
> + (!nfs4_check_access_deny_bmap(st,
> + access, share_access)))
> + continue;
> + clp = st->st_stid.sc_client;
> + if (nfsd4_expire_courtesy_clnt(clp))
> + continue;
> + conflict = true;
> + break;
> + }
> + return conflict;
> +}
> +
> static __be32 nfs4_get_vfs_file(struct svc_rqst *rqstp, struct nfs4_file *fp,
> struct svc_fh *cur_fh, struct nfs4_ol_stateid *stp,
> - struct nfsd4_open *open)
> + struct nfsd4_open *open, bool new_stp)
> {
> struct nfsd_file *nf = NULL;
> __be32 status;
> @@ -4968,15 +5034,29 @@ static __be32 nfs4_get_vfs_file(struct svc_rqst *rqstp, struct nfs4_file *fp,
> */
> status = nfs4_file_check_deny(fp, open->op_share_deny);
> if (status != nfs_ok) {
> - spin_unlock(&fp->fi_lock);
> - goto out;
> + if (status != nfserr_share_denied) {
> + spin_unlock(&fp->fi_lock);
> + goto out;
> + }
> + if (nfs4_resolve_deny_conflicts_locked(fp, new_stp,
> + stp, open->op_share_deny, false)) {
> + spin_unlock(&fp->fi_lock);
> + goto out;
> + }
> }
>
> /* set access to the file */
> status = nfs4_file_get_access(fp, open->op_share_access);
> if (status != nfs_ok) {
> - spin_unlock(&fp->fi_lock);
> - goto out;
> + if (status != nfserr_share_denied) {
> + spin_unlock(&fp->fi_lock);
> + goto out;
> + }
> + if (nfs4_resolve_deny_conflicts_locked(fp, new_stp,
> + stp, open->op_share_access, true)) {
> + spin_unlock(&fp->fi_lock);
> + goto out;
> + }
> }
>
> /* Set access bits in stateid */
> @@ -5027,7 +5107,7 @@ nfs4_upgrade_open(struct svc_rqst *rqstp, struct nfs4_file *fp, struct svc_fh *c
> unsigned char old_deny_bmap = stp->st_deny_bmap;
>
> if (!test_access(open->op_share_access, stp))
> - return nfs4_get_vfs_file(rqstp, fp, cur_fh, stp, open);
> + return nfs4_get_vfs_file(rqstp, fp, cur_fh, stp, open, false);
>
> /* test and set deny mode */
> spin_lock(&fp->fi_lock);
> @@ -5036,7 +5116,10 @@ nfs4_upgrade_open(struct svc_rqst *rqstp, struct nfs4_file *fp, struct svc_fh *c
> set_deny(open->op_share_deny, stp);
> fp->fi_share_deny |=
> (open->op_share_deny & NFS4_SHARE_DENY_BOTH);
> - }
> + } else if (status == nfserr_share_denied &&
> + !nfs4_resolve_deny_conflicts_locked(fp, false, stp,
> + open->op_share_deny, false))
Looks to me like these nfs4_resolve_deny_conflicts_locked() calls could
go into nfs4_file_check_deny and nfs4_file_get_access instead.
--b.
> + status = nfs_ok;
> spin_unlock(&fp->fi_lock);
>
> if (status != nfs_ok)
> @@ -5376,7 +5459,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
> goto out;
> }
> } else {
> - status = nfs4_get_vfs_file(rqstp, fp, current_fh, stp, open);
> + status = nfs4_get_vfs_file(rqstp, fp, current_fh, stp, open, true);
> if (status) {
> stp->st_stid.sc_type = NFS4_CLOSED_STID;
> release_open_stateid(stp);
> --
> 2.9.5
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH RFC v17 05/11] NFSD: Update nfs4_get_vfs_file() to handle courtesy client
2022-03-21 21:44 ` J. Bruce Fields
@ 2022-03-23 0:42 ` dai.ngo
0 siblings, 0 replies; 21+ messages in thread
From: dai.ngo @ 2022-03-23 0:42 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: chuck.lever, jlayton, viro, linux-nfs, linux-fsdevel
On 3/21/22 2:44 PM, J. Bruce Fields wrote:
> On Thu, Mar 17, 2022 at 12:43:42AM -0700, Dai Ngo wrote:
>> Update nfs4_get_vfs_file and nfs4_upgrade_open to handle share
>> reservation conflict with courtesy client.
>>
>> Update nfs4_get_vfs_file and nfs4_upgrade_open to handle share
>> reservation conflict with courtesy client.
>>
>> When we have deny/access conflict we walk the fi_stateids of the
>> file in question, looking for open stateid and check the deny/access
>> of that stateid against the one from the open request. If there is
>> a conflict then we check if the client that owns that stateid is
>> a courtesy client. If it is then we set the client state to
>> CLIENT_EXPIRED and allow the open request to continue. We have
>> to scan all the stateid's of the file since the conflict can be
>> caused by multiple open stateid's.
>>
>> Client with CLIENT_EXPIRED is expired by the laundromat.
>>
>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
>> ---
>> fs/nfsd/nfs4state.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 91 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index f20c75890594..c6b5e05c9c34 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -4950,9 +4950,75 @@ nfsd4_truncate(struct svc_rqst *rqstp, struct svc_fh *fh,
>> return nfsd_setattr(rqstp, fh, &iattr, 0, (time64_t)0);
>> }
>>
>> +static bool
>> +nfs4_check_access_deny_bmap(struct nfs4_ol_stateid *stp, u32 access,
>> + bool share_access)
>> +{
>> + if (share_access) {
>> + if (!stp->st_deny_bmap)
>> + return false;
>> +
>> + if ((stp->st_deny_bmap & (1 << NFS4_SHARE_DENY_BOTH)) ||
>> + (access & NFS4_SHARE_ACCESS_READ &&
>> + stp->st_deny_bmap & (1 << NFS4_SHARE_DENY_READ)) ||
>> + (access & NFS4_SHARE_ACCESS_WRITE &&
>> + stp->st_deny_bmap & (1 << NFS4_SHARE_DENY_WRITE))) {
>> + return true;
>> + }
>> + return false;
> Looks like you could do this with just:
>
> return share_access & bmap_to_share_mode(stp->st_deny_bmap);
Thanks, this makes it much simpler. Fix in v18.
>
>
>> + }
>> + if ((access & NFS4_SHARE_DENY_BOTH) ||
>> + (access & NFS4_SHARE_DENY_READ &&
>> + stp->st_access_bmap & (1 << NFS4_SHARE_ACCESS_READ)) ||
>> + (access & NFS4_SHARE_DENY_WRITE &&
>> + stp->st_access_bmap & (1 << NFS4_SHARE_ACCESS_WRITE))) {
>> + return true;
>> + }
> Likewise.
Fix in v18.
>
> Also, I think it'd be simpler to check for both access and deny
> conflicts here, instead of just one or the other.
I'm not clear here.
I remove nfs4_check_access_deny_bmap, fold this functionality
into nfs4_resolve_deny_conflicts_locked by making use of
bmap_to_share_mode.
>
>> + return false;
>> +}
>> +
>> +/*
>> + * Check whether courtesy clients have conflicting access
>> + *
>> + * access: is op_share_access if share_access is true.
>> + * Check if access mode, op_share_access, would conflict with
>> + * the current deny mode of the file 'fp'.
>> + * access: is op_share_deny if share_access is false.
>> + * Check if the deny mode, op_share_deny, would conflict with
>> + * current access of the file 'fp'.
>> + * stp: skip checking this entry.
>> + * new_stp: normal open, not open upgrade.
>> + *
>> + * Function returns:
>> + * true - access/deny mode conflict with normal client.
>> + * false - no conflict or conflict with courtesy client(s) is resolved.
>> + */
>> +static bool
>> +nfs4_resolve_deny_conflicts_locked(struct nfs4_file *fp, bool new_stp,
>> + struct nfs4_ol_stateid *stp, u32 access, bool share_access)
>> +{
>> + struct nfs4_ol_stateid *st;
>> + struct nfs4_client *clp;
>> + bool conflict = false;
>> +
>> + lockdep_assert_held(&fp->fi_lock);
>> + list_for_each_entry(st, &fp->fi_stateids, st_perfile) {
>> + if (st->st_openstp || (st == stp && new_stp) ||
> I'd split this into separate if statements and add a comment at least
> for the st_openstp case, which isn't too obvious:
>
> if (st->st_openstp) /* ignore lock stateids */
> continue;
Fix in v18.
>
>> + (!nfs4_check_access_deny_bmap(st,
>> + access, share_access)))
>> + continue;
>> + clp = st->st_stid.sc_client;
>> + if (nfsd4_expire_courtesy_clnt(clp))
>> + continue;
>> + conflict = true;
>> + break;
>> + }
>> + return conflict;
>> +}
>> +
>> static __be32 nfs4_get_vfs_file(struct svc_rqst *rqstp, struct nfs4_file *fp,
>> struct svc_fh *cur_fh, struct nfs4_ol_stateid *stp,
>> - struct nfsd4_open *open)
>> + struct nfsd4_open *open, bool new_stp)
>> {
>> struct nfsd_file *nf = NULL;
>> __be32 status;
>> @@ -4968,15 +5034,29 @@ static __be32 nfs4_get_vfs_file(struct svc_rqst *rqstp, struct nfs4_file *fp,
>> */
>> status = nfs4_file_check_deny(fp, open->op_share_deny);
>> if (status != nfs_ok) {
>> - spin_unlock(&fp->fi_lock);
>> - goto out;
>> + if (status != nfserr_share_denied) {
>> + spin_unlock(&fp->fi_lock);
>> + goto out;
>> + }
>> + if (nfs4_resolve_deny_conflicts_locked(fp, new_stp,
>> + stp, open->op_share_deny, false)) {
>> + spin_unlock(&fp->fi_lock);
>> + goto out;
>> + }
>> }
>>
>> /* set access to the file */
>> status = nfs4_file_get_access(fp, open->op_share_access);
>> if (status != nfs_ok) {
>> - spin_unlock(&fp->fi_lock);
>> - goto out;
>> + if (status != nfserr_share_denied) {
>> + spin_unlock(&fp->fi_lock);
>> + goto out;
>> + }
>> + if (nfs4_resolve_deny_conflicts_locked(fp, new_stp,
>> + stp, open->op_share_access, true)) {
>> + spin_unlock(&fp->fi_lock);
>> + goto out;
>> + }
>> }
>>
>> /* Set access bits in stateid */
>> @@ -5027,7 +5107,7 @@ nfs4_upgrade_open(struct svc_rqst *rqstp, struct nfs4_file *fp, struct svc_fh *c
>> unsigned char old_deny_bmap = stp->st_deny_bmap;
>>
>> if (!test_access(open->op_share_access, stp))
>> - return nfs4_get_vfs_file(rqstp, fp, cur_fh, stp, open);
>> + return nfs4_get_vfs_file(rqstp, fp, cur_fh, stp, open, false);
>>
>> /* test and set deny mode */
>> spin_lock(&fp->fi_lock);
>> @@ -5036,7 +5116,10 @@ nfs4_upgrade_open(struct svc_rqst *rqstp, struct nfs4_file *fp, struct svc_fh *c
>> set_deny(open->op_share_deny, stp);
>> fp->fi_share_deny |=
>> (open->op_share_deny & NFS4_SHARE_DENY_BOTH);
>> - }
>> + } else if (status == nfserr_share_denied &&
>> + !nfs4_resolve_deny_conflicts_locked(fp, false, stp,
>> + open->op_share_deny, false))
> Looks to me like these nfs4_resolve_deny_conflicts_locked() calls could
> go into nfs4_file_check_deny and nfs4_file_get_access instead.
Fix in v18.
-Dai
>
> --b.
>
>> + status = nfs_ok;
>> spin_unlock(&fp->fi_lock);
>>
>> if (status != nfs_ok)
>> @@ -5376,7 +5459,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
>> goto out;
>> }
>> } else {
>> - status = nfs4_get_vfs_file(rqstp, fp, current_fh, stp, open);
>> + status = nfs4_get_vfs_file(rqstp, fp, current_fh, stp, open, true);
>> if (status) {
>> stp->st_stid.sc_type = NFS4_CLOSED_STID;
>> release_open_stateid(stp);
>> --
>> 2.9.5
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH RFC v17 06/11] NFSD: Update find_clp_in_name_tree() to handle courtesy client
2022-03-17 7:43 [PATCH RFC v17 0/11] NFSD: Initial implementation of NFSv4 Courteous Server Dai Ngo
` (4 preceding siblings ...)
2022-03-17 7:43 ` [PATCH RFC v17 05/11] NFSD: Update nfs4_get_vfs_file() to handle courtesy client Dai Ngo
@ 2022-03-17 7:43 ` Dai Ngo
2022-03-22 21:50 ` J. Bruce Fields
2022-03-17 7:43 ` [PATCH RFC v17 07/11] NFSD: Update find_in_sessionid_hashtbl() " Dai Ngo
` (4 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Dai Ngo @ 2022-03-17 7:43 UTC (permalink / raw)
To: chuck.lever, bfields; +Cc: jlayton, viro, linux-nfs, linux-fsdevel
Update find_clp_in_name_tree to check and expire courtesy client.
Update find_confirmed_client_by_name to discard the courtesy
client by setting CLIENT_EXPIRED.
Update nfsd4_setclientid to expire client with CLIENT_EXPIRED
state to prevent multiple confirmed clients with the same name
on the conf_id_hashtbl list.
Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
fs/nfsd/nfs4state.c | 27 ++++++++++++++++++++++++---
fs/nfsd/state.h | 22 ++++++++++++++++++++++
2 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c6b5e05c9c34..dc0e60bf694c 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2834,8 +2834,11 @@ find_clp_in_name_tree(struct xdr_netobj *name, struct rb_root *root)
node = node->rb_left;
else if (cmp < 0)
node = node->rb_right;
- else
+ else {
+ if (nfsd4_courtesy_clnt_expired(clp))
+ return NULL;
return clp;
+ }
}
return NULL;
}
@@ -2914,8 +2917,15 @@ static bool clp_used_exchangeid(struct nfs4_client *clp)
static struct nfs4_client *
find_confirmed_client_by_name(struct xdr_netobj *name, struct nfsd_net *nn)
{
+ struct nfs4_client *clp;
+
lockdep_assert_held(&nn->client_lock);
- return find_clp_in_name_tree(name, &nn->conf_name_tree);
+ clp = find_clp_in_name_tree(name, &nn->conf_name_tree);
+ if (clp && clp->cl_cs_client_state == NFSD4_CLIENT_RECONNECTED) {
+ nfsd4_discard_courtesy_clnt(clp);
+ clp = NULL;
+ }
+ return clp;
}
static struct nfs4_client *
@@ -4032,12 +4042,19 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct nfs4_client *unconf = NULL;
__be32 status;
struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
+ struct nfs4_client *cclient = NULL;
new = create_client(clname, rqstp, &clverifier);
if (new == NULL)
return nfserr_jukebox;
spin_lock(&nn->client_lock);
- conf = find_confirmed_client_by_name(&clname, nn);
+ /* find confirmed client by name */
+ conf = find_clp_in_name_tree(&clname, &nn->conf_name_tree);
+ if (conf && conf->cl_cs_client_state == NFSD4_CLIENT_RECONNECTED) {
+ cclient = conf;
+ conf = NULL;
+ }
+
if (conf && client_has_state(conf)) {
status = nfserr_clid_inuse;
if (clp_used_exchangeid(conf))
@@ -4068,7 +4085,11 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
new = NULL;
status = nfs_ok;
out:
+ if (cclient)
+ unhash_client_locked(cclient);
spin_unlock(&nn->client_lock);
+ if (cclient)
+ expire_client(cclient);
if (new)
free_client(new);
if (unconf) {
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index d156ae3ab46c..14b2c158ccca 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -735,6 +735,7 @@ extern void nfsd4_client_record_remove(struct nfs4_client *clp);
extern int nfsd4_client_record_check(struct nfs4_client *clp);
extern void nfsd4_record_grace_done(struct nfsd_net *nn);
+/* courteous server */
static inline bool
nfsd4_expire_courtesy_clnt(struct nfs4_client *clp)
{
@@ -749,4 +750,25 @@ nfsd4_expire_courtesy_clnt(struct nfs4_client *clp)
return rc;
}
+static inline void
+nfsd4_discard_courtesy_clnt(struct nfs4_client *clp)
+{
+ spin_lock(&clp->cl_cs_lock);
+ clp->cl_cs_client_state = NFSD4_CLIENT_EXPIRED;
+ spin_unlock(&clp->cl_cs_lock);
+}
+
+static inline bool
+nfsd4_courtesy_clnt_expired(struct nfs4_client *clp)
+{
+ bool rc = false;
+
+ spin_lock(&clp->cl_cs_lock);
+ if (clp->cl_cs_client_state == NFSD4_CLIENT_EXPIRED)
+ rc = true;
+ if (clp->cl_cs_client_state == NFSD4_CLIENT_COURTESY)
+ clp->cl_cs_client_state = NFSD4_CLIENT_RECONNECTED;
+ spin_unlock(&clp->cl_cs_lock);
+ return rc;
+}
#endif /* NFSD4_STATE_H */
--
2.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH RFC v17 06/11] NFSD: Update find_clp_in_name_tree() to handle courtesy client
2022-03-17 7:43 ` [PATCH RFC v17 06/11] NFSD: Update find_clp_in_name_tree() " Dai Ngo
@ 2022-03-22 21:50 ` J. Bruce Fields
2022-03-23 1:14 ` dai.ngo
0 siblings, 1 reply; 21+ messages in thread
From: J. Bruce Fields @ 2022-03-22 21:50 UTC (permalink / raw)
To: Dai Ngo; +Cc: chuck.lever, jlayton, viro, linux-nfs, linux-fsdevel
On Thu, Mar 17, 2022 at 12:43:43AM -0700, Dai Ngo wrote:
> Update find_clp_in_name_tree to check and expire courtesy client.
>
> Update find_confirmed_client_by_name to discard the courtesy
> client by setting CLIENT_EXPIRED.
>
> Update nfsd4_setclientid to expire client with CLIENT_EXPIRED
> state to prevent multiple confirmed clients with the same name
> on the conf_id_hashtbl list.
I could use a little more "why" here.
I'll give it another read, but right now I'm just not understanding how
this is meant to work.
--b.
>
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
> fs/nfsd/nfs4state.c | 27 ++++++++++++++++++++++++---
> fs/nfsd/state.h | 22 ++++++++++++++++++++++
> 2 files changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index c6b5e05c9c34..dc0e60bf694c 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2834,8 +2834,11 @@ find_clp_in_name_tree(struct xdr_netobj *name, struct rb_root *root)
> node = node->rb_left;
> else if (cmp < 0)
> node = node->rb_right;
> - else
> + else {
> + if (nfsd4_courtesy_clnt_expired(clp))
> + return NULL;
> return clp;
> + }
> }
> return NULL;
> }
> @@ -2914,8 +2917,15 @@ static bool clp_used_exchangeid(struct nfs4_client *clp)
> static struct nfs4_client *
> find_confirmed_client_by_name(struct xdr_netobj *name, struct nfsd_net *nn)
> {
> + struct nfs4_client *clp;
> +
> lockdep_assert_held(&nn->client_lock);
> - return find_clp_in_name_tree(name, &nn->conf_name_tree);
> + clp = find_clp_in_name_tree(name, &nn->conf_name_tree);
> + if (clp && clp->cl_cs_client_state == NFSD4_CLIENT_RECONNECTED) {
> + nfsd4_discard_courtesy_clnt(clp);
> + clp = NULL;
> + }
> + return clp;
> }
>
> static struct nfs4_client *
> @@ -4032,12 +4042,19 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> struct nfs4_client *unconf = NULL;
> __be32 status;
> struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> + struct nfs4_client *cclient = NULL;
>
> new = create_client(clname, rqstp, &clverifier);
> if (new == NULL)
> return nfserr_jukebox;
> spin_lock(&nn->client_lock);
> - conf = find_confirmed_client_by_name(&clname, nn);
> + /* find confirmed client by name */
> + conf = find_clp_in_name_tree(&clname, &nn->conf_name_tree);
> + if (conf && conf->cl_cs_client_state == NFSD4_CLIENT_RECONNECTED) {
> + cclient = conf;
> + conf = NULL;
> + }
> +
> if (conf && client_has_state(conf)) {
> status = nfserr_clid_inuse;
> if (clp_used_exchangeid(conf))
> @@ -4068,7 +4085,11 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> new = NULL;
> status = nfs_ok;
> out:
> + if (cclient)
> + unhash_client_locked(cclient);
> spin_unlock(&nn->client_lock);
> + if (cclient)
> + expire_client(cclient);
> if (new)
> free_client(new);
> if (unconf) {
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index d156ae3ab46c..14b2c158ccca 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -735,6 +735,7 @@ extern void nfsd4_client_record_remove(struct nfs4_client *clp);
> extern int nfsd4_client_record_check(struct nfs4_client *clp);
> extern void nfsd4_record_grace_done(struct nfsd_net *nn);
>
> +/* courteous server */
> static inline bool
> nfsd4_expire_courtesy_clnt(struct nfs4_client *clp)
> {
> @@ -749,4 +750,25 @@ nfsd4_expire_courtesy_clnt(struct nfs4_client *clp)
> return rc;
> }
>
> +static inline void
> +nfsd4_discard_courtesy_clnt(struct nfs4_client *clp)
> +{
> + spin_lock(&clp->cl_cs_lock);
> + clp->cl_cs_client_state = NFSD4_CLIENT_EXPIRED;
> + spin_unlock(&clp->cl_cs_lock);
> +}
> +
> +static inline bool
> +nfsd4_courtesy_clnt_expired(struct nfs4_client *clp)
> +{
> + bool rc = false;
> +
> + spin_lock(&clp->cl_cs_lock);
> + if (clp->cl_cs_client_state == NFSD4_CLIENT_EXPIRED)
> + rc = true;
> + if (clp->cl_cs_client_state == NFSD4_CLIENT_COURTESY)
> + clp->cl_cs_client_state = NFSD4_CLIENT_RECONNECTED;
> + spin_unlock(&clp->cl_cs_lock);
> + return rc;
> +}
> #endif /* NFSD4_STATE_H */
> --
> 2.9.5
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH RFC v17 06/11] NFSD: Update find_clp_in_name_tree() to handle courtesy client
2022-03-22 21:50 ` J. Bruce Fields
@ 2022-03-23 1:14 ` dai.ngo
0 siblings, 0 replies; 21+ messages in thread
From: dai.ngo @ 2022-03-23 1:14 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: chuck.lever, jlayton, viro, linux-nfs, linux-fsdevel
On 3/22/22 2:50 PM, J. Bruce Fields wrote:
> On Thu, Mar 17, 2022 at 12:43:43AM -0700, Dai Ngo wrote:
>> Update find_clp_in_name_tree to check and expire courtesy client.
>>
>> Update find_confirmed_client_by_name to discard the courtesy
>> client by setting CLIENT_EXPIRED.
>>
>> Update nfsd4_setclientid to expire client with CLIENT_EXPIRED
>> state to prevent multiple confirmed clients with the same name
>> on the conf_id_hashtbl list.
This should be nn->conf_name_tree instead of conf_id_hashtbl list.
> I could use a little more "why" here.
>
> I'll give it another read, but right now I'm just not understanding how
> this is meant to work.
Here is the problem description when running pynfs/nfs4.0 CID1 (need to
run CID5 first to create ton of clients) which requires the change in
nfsd4_setclientid to get the test to pass:
1st setclientid(name=clientA,verf=X)
. find confirmed courtesy client with same name clientA (clientid:1) and
mark it DESTROY and leave it on the confirmed list.
. assign new clientA with clientid:2
. put clientid:2 on unconfirmed list
. return clientid:2
1st setclientid_confirm(clientid:2)
. does not find any confirmed for clientid:2
. find unconfirmed clientid:2
. does not find any confirmed clientA (find_confirmed_client_by_name)
since courtesy clientA was marked DESTROY
. put clientid:2: to conf_name_tree list
PROBLEM: new clientA (clientid:2) and courtesy clientA (clientid:1)
are both now on the conf_name_tree list.
clientid:2 does OPEN -> stidX
2nd setclientid(name=clientA, verf=Y)
. does not find any confirmed clientA: skip courtesy clientA since it
was marked DESTROY and does not find confirmed client:2 due to the
PROBLEM noted above; duplicate entry in binary tree.
. create new clientid:3 and put it on unconfirmed list
. return clientid:3
2nd setclientid_confirm(clientid:3)
. does not find any confirmed clientid:3
. find unconfirmed clientid:3
. does not find any confirmed clientA: skip courtesy clientA since it
was marked DESTROY and does not find confirmed client:2 due to the
PROBLEM noted above; duplicate entry in binary tree.
. put clientid:3 on conf_name_tree list
clientid:3 does CLOSE(stidX) -> NFS4_OK since clientid:2 still valid <ERROR!>
-Dai
>
> --b.
>
>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
>> ---
>> fs/nfsd/nfs4state.c | 27 ++++++++++++++++++++++++---
>> fs/nfsd/state.h | 22 ++++++++++++++++++++++
>> 2 files changed, 46 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index c6b5e05c9c34..dc0e60bf694c 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -2834,8 +2834,11 @@ find_clp_in_name_tree(struct xdr_netobj *name, struct rb_root *root)
>> node = node->rb_left;
>> else if (cmp < 0)
>> node = node->rb_right;
>> - else
>> + else {
>> + if (nfsd4_courtesy_clnt_expired(clp))
>> + return NULL;
>> return clp;
>> + }
>> }
>> return NULL;
>> }
>> @@ -2914,8 +2917,15 @@ static bool clp_used_exchangeid(struct nfs4_client *clp)
>> static struct nfs4_client *
>> find_confirmed_client_by_name(struct xdr_netobj *name, struct nfsd_net *nn)
>> {
>> + struct nfs4_client *clp;
>> +
>> lockdep_assert_held(&nn->client_lock);
>> - return find_clp_in_name_tree(name, &nn->conf_name_tree);
>> + clp = find_clp_in_name_tree(name, &nn->conf_name_tree);
>> + if (clp && clp->cl_cs_client_state == NFSD4_CLIENT_RECONNECTED) {
>> + nfsd4_discard_courtesy_clnt(clp);
>> + clp = NULL;
>> + }
>> + return clp;
>> }
>>
>> static struct nfs4_client *
>> @@ -4032,12 +4042,19 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>> struct nfs4_client *unconf = NULL;
>> __be32 status;
>> struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
>> + struct nfs4_client *cclient = NULL;
>>
>> new = create_client(clname, rqstp, &clverifier);
>> if (new == NULL)
>> return nfserr_jukebox;
>> spin_lock(&nn->client_lock);
>> - conf = find_confirmed_client_by_name(&clname, nn);
>> + /* find confirmed client by name */
>> + conf = find_clp_in_name_tree(&clname, &nn->conf_name_tree);
>> + if (conf && conf->cl_cs_client_state == NFSD4_CLIENT_RECONNECTED) {
>> + cclient = conf;
>> + conf = NULL;
>> + }
>> +
>> if (conf && client_has_state(conf)) {
>> status = nfserr_clid_inuse;
>> if (clp_used_exchangeid(conf))
>> @@ -4068,7 +4085,11 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>> new = NULL;
>> status = nfs_ok;
>> out:
>> + if (cclient)
>> + unhash_client_locked(cclient);
>> spin_unlock(&nn->client_lock);
>> + if (cclient)
>> + expire_client(cclient);
>> if (new)
>> free_client(new);
>> if (unconf) {
>> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
>> index d156ae3ab46c..14b2c158ccca 100644
>> --- a/fs/nfsd/state.h
>> +++ b/fs/nfsd/state.h
>> @@ -735,6 +735,7 @@ extern void nfsd4_client_record_remove(struct nfs4_client *clp);
>> extern int nfsd4_client_record_check(struct nfs4_client *clp);
>> extern void nfsd4_record_grace_done(struct nfsd_net *nn);
>>
>> +/* courteous server */
>> static inline bool
>> nfsd4_expire_courtesy_clnt(struct nfs4_client *clp)
>> {
>> @@ -749,4 +750,25 @@ nfsd4_expire_courtesy_clnt(struct nfs4_client *clp)
>> return rc;
>> }
>>
>> +static inline void
>> +nfsd4_discard_courtesy_clnt(struct nfs4_client *clp)
>> +{
>> + spin_lock(&clp->cl_cs_lock);
>> + clp->cl_cs_client_state = NFSD4_CLIENT_EXPIRED;
>> + spin_unlock(&clp->cl_cs_lock);
>> +}
>> +
>> +static inline bool
>> +nfsd4_courtesy_clnt_expired(struct nfs4_client *clp)
>> +{
>> + bool rc = false;
>> +
>> + spin_lock(&clp->cl_cs_lock);
>> + if (clp->cl_cs_client_state == NFSD4_CLIENT_EXPIRED)
>> + rc = true;
>> + if (clp->cl_cs_client_state == NFSD4_CLIENT_COURTESY)
>> + clp->cl_cs_client_state = NFSD4_CLIENT_RECONNECTED;
>> + spin_unlock(&clp->cl_cs_lock);
>> + return rc;
>> +}
>> #endif /* NFSD4_STATE_H */
>> --
>> 2.9.5
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH RFC v17 07/11] NFSD: Update find_in_sessionid_hashtbl() to handle courtesy client
2022-03-17 7:43 [PATCH RFC v17 0/11] NFSD: Initial implementation of NFSv4 Courteous Server Dai Ngo
` (5 preceding siblings ...)
2022-03-17 7:43 ` [PATCH RFC v17 06/11] NFSD: Update find_clp_in_name_tree() " Dai Ngo
@ 2022-03-17 7:43 ` Dai Ngo
2022-03-17 7:43 ` [PATCH RFC v17 08/11] NFSD: Update find_client_in_id_table() " Dai Ngo
` (3 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Dai Ngo @ 2022-03-17 7:43 UTC (permalink / raw)
To: chuck.lever, bfields; +Cc: jlayton, viro, linux-nfs, linux-fsdevel
Update find_in_sessionid_hashtbl to:
. skip client with CLIENT_EXPIRED state; discarded courtesy client.
. if courtesy client was found then set CLIENT_RECONNECTED so caller
can take appropriate action.
Update nfsd4_sequence and nfsd4_bind_conn_to_session to create client
record for courtesy client with CLIENT_RECONNECTED state.
Update nfsd4_destroy_session to discard courtesy client with
CLIENT_RECONNECTED state.
Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
fs/nfsd/nfs4state.c | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index dc0e60bf694c..3b1e857eabee 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1935,13 +1935,22 @@ find_in_sessionid_hashtbl(struct nfs4_sessionid *sessionid, struct net *net,
{
struct nfsd4_session *session;
__be32 status = nfserr_badsession;
+ struct nfs4_client *clp;
session = __find_in_sessionid_hashtbl(sessionid, net);
if (!session)
goto out;
+ clp = session->se_client;
+ if (nfsd4_courtesy_clnt_expired(clp)) {
+ session = NULL;
+ goto out;
+ }
status = nfsd4_get_session_locked(session);
- if (status)
+ if (status) {
session = NULL;
+ if (clp->cl_cs_client_state == NFSD4_CLIENT_RECONNECTED)
+ nfsd4_discard_courtesy_clnt(clp);
+ }
out:
*ret = status;
return session;
@@ -3643,6 +3652,7 @@ __be32 nfsd4_bind_conn_to_session(struct svc_rqst *rqstp,
struct nfsd4_session *session;
struct net *net = SVC_NET(rqstp);
struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+ struct nfs4_client *clp;
if (!nfsd4_last_compound_op(rqstp))
return nfserr_not_only_op;
@@ -3675,6 +3685,13 @@ __be32 nfsd4_bind_conn_to_session(struct svc_rqst *rqstp,
nfsd4_init_conn(rqstp, conn, session);
status = nfs_ok;
out:
+ clp = session->se_client;
+ if (clp->cl_cs_client_state == NFSD4_CLIENT_RECONNECTED) {
+ if (status == nfs_ok)
+ nfsd4_client_record_create(clp);
+ else
+ nfsd4_discard_courtesy_clnt(clp);
+ }
nfsd4_put_session(session);
out_no_session:
return status;
@@ -3697,6 +3714,7 @@ nfsd4_destroy_session(struct svc_rqst *r, struct nfsd4_compound_state *cstate,
int ref_held_by_me = 0;
struct net *net = SVC_NET(r);
struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+ struct nfs4_client *clp;
status = nfserr_not_only_op;
if (nfsd4_compound_in_session(cstate, sessionid)) {
@@ -3709,6 +3727,12 @@ nfsd4_destroy_session(struct svc_rqst *r, struct nfsd4_compound_state *cstate,
ses = find_in_sessionid_hashtbl(sessionid, net, &status);
if (!ses)
goto out_client_lock;
+ clp = ses->se_client;
+ if (clp->cl_cs_client_state == NFSD4_CLIENT_RECONNECTED) {
+ status = nfserr_badsession;
+ nfsd4_discard_courtesy_clnt(clp);
+ goto out_put_session;
+ }
status = nfserr_wrong_cred;
if (!nfsd4_mach_creds_match(ses->se_client, r))
goto out_put_session;
@@ -3813,7 +3837,7 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct nfsd4_compoundres *resp = rqstp->rq_resp;
struct xdr_stream *xdr = resp->xdr;
struct nfsd4_session *session;
- struct nfs4_client *clp;
+ struct nfs4_client *clp = NULL;
struct nfsd4_slot *slot;
struct nfsd4_conn *conn;
__be32 status;
@@ -3923,6 +3947,12 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if (conn)
free_conn(conn);
spin_unlock(&nn->client_lock);
+ if (clp && clp->cl_cs_client_state == NFSD4_CLIENT_RECONNECTED) {
+ if (status == nfs_ok)
+ nfsd4_client_record_create(clp);
+ else
+ nfsd4_discard_courtesy_clnt(clp);
+ }
return status;
out_put_session:
nfsd4_put_session_locked(session);
--
2.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH RFC v17 08/11] NFSD: Update find_client_in_id_table() to handle courtesy client
2022-03-17 7:43 [PATCH RFC v17 0/11] NFSD: Initial implementation of NFSv4 Courteous Server Dai Ngo
` (6 preceding siblings ...)
2022-03-17 7:43 ` [PATCH RFC v17 07/11] NFSD: Update find_in_sessionid_hashtbl() " Dai Ngo
@ 2022-03-17 7:43 ` Dai Ngo
2022-03-17 7:43 ` [PATCH RFC v17 09/11] NFSD: Refactor nfsd4_laundromat() Dai Ngo
` (2 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Dai Ngo @ 2022-03-17 7:43 UTC (permalink / raw)
To: chuck.lever, bfields; +Cc: jlayton, viro, linux-nfs, linux-fsdevel
Update find_client_in_id_table to:
. skip client with CLIENT_EXPIRED; discarded courtesy client
. if courtesy client was found then set CLIENT_RECONNECTED
state so caller can take appropriate action.
Update find_confirmed_client to discard courtesy client.
Update lookup_clientid to call find_client_in_id_table directly.
Update set_client to create client record for courtesy client.
Update find_cpntf_state to discard courtesy client.
Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
fs/nfsd/nfs4state.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 3b1e857eabee..102b4aae286d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2893,6 +2893,8 @@ find_client_in_id_table(struct list_head *tbl, clientid_t *clid, bool sessions)
if (same_clid(&clp->cl_clientid, clid)) {
if ((bool)clp->cl_minorversion != sessions)
return NULL;
+ if (nfsd4_courtesy_clnt_expired(clp))
+ continue;
renew_client_locked(clp);
return clp;
}
@@ -2904,9 +2906,15 @@ static struct nfs4_client *
find_confirmed_client(clientid_t *clid, bool sessions, struct nfsd_net *nn)
{
struct list_head *tbl = nn->conf_id_hashtbl;
+ struct nfs4_client *clp;
lockdep_assert_held(&nn->client_lock);
- return find_client_in_id_table(tbl, clid, sessions);
+ clp = find_client_in_id_table(tbl, clid, sessions);
+ if (clp && clp->cl_cs_client_state == NFSD4_CLIENT_RECONNECTED) {
+ nfsd4_discard_courtesy_clnt(clp);
+ clp = NULL;
+ }
+ return clp;
}
static struct nfs4_client *
@@ -4822,9 +4830,10 @@ static struct nfs4_client *lookup_clientid(clientid_t *clid, bool sessions,
struct nfsd_net *nn)
{
struct nfs4_client *found;
+ struct list_head *tbl = nn->conf_id_hashtbl;
spin_lock(&nn->client_lock);
- found = find_confirmed_client(clid, sessions, nn);
+ found = find_client_in_id_table(tbl, clid, sessions);
if (found)
atomic_inc(&found->cl_rpc_users);
spin_unlock(&nn->client_lock);
@@ -4849,6 +4858,8 @@ static __be32 set_client(clientid_t *clid,
cstate->clp = lookup_clientid(clid, false, nn);
if (!cstate->clp)
return nfserr_expired;
+ if (cstate->clp->cl_cs_client_state == NFSD4_CLIENT_RECONNECTED)
+ nfsd4_client_record_create(cstate->clp);
return nfs_ok;
}
@@ -6168,6 +6179,13 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
found = lookup_clientid(&cps->cp_p_clid, true, nn);
if (!found)
goto out;
+ if (found->cl_cs_client_state == NFSD4_CLIENT_RECONNECTED) {
+ nfsd4_discard_courtesy_clnt(found);
+ if (atomic_dec_and_lock(&found->cl_rpc_users,
+ &nn->client_lock))
+ spin_unlock(&nn->client_lock);
+ goto out;
+ }
*stid = find_stateid_by_type(found, &cps->cp_p_stateid,
NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID);
--
2.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH RFC v17 09/11] NFSD: Refactor nfsd4_laundromat()
2022-03-17 7:43 [PATCH RFC v17 0/11] NFSD: Initial implementation of NFSv4 Courteous Server Dai Ngo
` (7 preceding siblings ...)
2022-03-17 7:43 ` [PATCH RFC v17 08/11] NFSD: Update find_client_in_id_table() " Dai Ngo
@ 2022-03-17 7:43 ` Dai Ngo
2022-03-17 7:43 ` [PATCH RFC v17 10/11] NFSD: Update laundromat to handle courtesy clients Dai Ngo
2022-03-17 7:43 ` [PATCH RFC v17 11/11] NFSD: Show state of courtesy clients in client info Dai Ngo
10 siblings, 0 replies; 21+ messages in thread
From: Dai Ngo @ 2022-03-17 7:43 UTC (permalink / raw)
To: chuck.lever, bfields; +Cc: jlayton, viro, linux-nfs, linux-fsdevel
Extract a bit of logic that is about to be expanded to handle
courtesy clients.
Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
fs/nfsd/nfs4state.c | 33 +++++++++++++++++++++------------
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 102b4aae286d..b4b976e00ce6 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5755,6 +5755,26 @@ static void nfsd4_ssc_expire_umount(struct nfsd_net *nn)
}
#endif
+static void
+nfs4_get_client_reaplist(struct nfsd_net *nn, struct list_head *reaplist,
+ struct laundry_time *lt)
+{
+ struct list_head *pos, *next;
+ struct nfs4_client *clp;
+
+ INIT_LIST_HEAD(reaplist);
+ spin_lock(&nn->client_lock);
+ list_for_each_safe(pos, next, &nn->client_lru) {
+ clp = list_entry(pos, struct nfs4_client, cl_lru);
+ if (!state_expired(lt, clp->cl_time))
+ break;
+ if (mark_client_expired_locked(clp))
+ continue;
+ list_add(&clp->cl_lru, reaplist);
+ }
+ spin_unlock(&nn->client_lock);
+}
+
static time64_t
nfs4_laundromat(struct nfsd_net *nn)
{
@@ -5777,7 +5797,6 @@ nfs4_laundromat(struct nfsd_net *nn)
goto out;
}
nfsd4_end_grace(nn);
- INIT_LIST_HEAD(&reaplist);
spin_lock(&nn->s2s_cp_lock);
idr_for_each_entry(&nn->s2s_cp_stateids, cps_t, i) {
@@ -5787,17 +5806,7 @@ nfs4_laundromat(struct nfsd_net *nn)
_free_cpntf_state_locked(nn, cps);
}
spin_unlock(&nn->s2s_cp_lock);
-
- spin_lock(&nn->client_lock);
- list_for_each_safe(pos, next, &nn->client_lru) {
- clp = list_entry(pos, struct nfs4_client, cl_lru);
- if (!state_expired(<, clp->cl_time))
- break;
- if (mark_client_expired_locked(clp))
- continue;
- list_add(&clp->cl_lru, &reaplist);
- }
- spin_unlock(&nn->client_lock);
+ nfs4_get_client_reaplist(nn, &reaplist, <);
list_for_each_safe(pos, next, &reaplist) {
clp = list_entry(pos, struct nfs4_client, cl_lru);
trace_nfsd_clid_purged(&clp->cl_clientid);
--
2.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH RFC v17 10/11] NFSD: Update laundromat to handle courtesy clients
2022-03-17 7:43 [PATCH RFC v17 0/11] NFSD: Initial implementation of NFSv4 Courteous Server Dai Ngo
` (8 preceding siblings ...)
2022-03-17 7:43 ` [PATCH RFC v17 09/11] NFSD: Refactor nfsd4_laundromat() Dai Ngo
@ 2022-03-17 7:43 ` Dai Ngo
2022-03-17 15:03 ` Chuck Lever III
2022-03-17 7:43 ` [PATCH RFC v17 11/11] NFSD: Show state of courtesy clients in client info Dai Ngo
10 siblings, 1 reply; 21+ messages in thread
From: Dai Ngo @ 2022-03-17 7:43 UTC (permalink / raw)
To: chuck.lever, bfields; +Cc: jlayton, viro, linux-nfs, linux-fsdevel
Add nfs4_anylock_blocker and nfs4_lockowner_has_blockers to check
if an expired client has any lock blockers
Update nfs4_get_client_reaplist to:
. add courtesy client in CLIENT_EXPIRED state to reaplist.
. detect if expired client still has state and no blockers then
transit it to courtesy client by setting CLIENT_COURTESY state
and removing the client record.
Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
fs/nfsd/nfs4state.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 91 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b4b976e00ce6..d5758c7101dc 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5755,24 +5755,106 @@ static void nfsd4_ssc_expire_umount(struct nfsd_net *nn)
}
#endif
+/* Check if any lock belonging to this lockowner has any blockers */
+static bool
+nfs4_lockowner_has_blockers(struct nfs4_lockowner *lo)
+{
+ struct file_lock_context *ctx;
+ struct nfs4_ol_stateid *stp;
+ struct nfs4_file *nf;
+
+ list_for_each_entry(stp, &lo->lo_owner.so_stateids, st_perstateowner) {
+ nf = stp->st_stid.sc_file;
+ ctx = nf->fi_inode->i_flctx;
+ if (!ctx)
+ continue;
+ if (locks_owner_has_blockers(ctx, lo))
+ return true;
+ }
+ return false;
+}
+
+static bool
+nfs4_anylock_blockers(struct nfs4_client *clp)
+{
+ int i;
+ struct nfs4_stateowner *so;
+ struct nfs4_lockowner *lo;
+
+ spin_lock(&clp->cl_lock);
+ for (i = 0; i < OWNER_HASH_SIZE; i++) {
+ list_for_each_entry(so, &clp->cl_ownerstr_hashtbl[i],
+ so_strhash) {
+ if (so->so_is_open_owner)
+ continue;
+ lo = lockowner(so);
+ if (nfs4_lockowner_has_blockers(lo)) {
+ spin_unlock(&clp->cl_lock);
+ return true;
+ }
+ }
+ }
+ spin_unlock(&clp->cl_lock);
+ return false;
+}
+
static void
nfs4_get_client_reaplist(struct nfsd_net *nn, struct list_head *reaplist,
struct laundry_time *lt)
{
struct list_head *pos, *next;
struct nfs4_client *clp;
+ bool cour;
+ struct list_head cslist;
INIT_LIST_HEAD(reaplist);
+ INIT_LIST_HEAD(&cslist);
spin_lock(&nn->client_lock);
list_for_each_safe(pos, next, &nn->client_lru) {
clp = list_entry(pos, struct nfs4_client, cl_lru);
if (!state_expired(lt, clp->cl_time))
break;
- if (mark_client_expired_locked(clp))
+
+ if (!client_has_state(clp))
+ goto exp_client;
+
+ if (clp->cl_cs_client_state == NFSD4_CLIENT_EXPIRED)
+ goto exp_client;
+ cour = (clp->cl_cs_client_state == NFSD4_CLIENT_COURTESY);
+ if (cour && ktime_get_boottime_seconds() >=
+ (clp->cl_time + NFSD_COURTESY_CLIENT_TIMEOUT))
+ goto exp_client;
+ if (nfs4_anylock_blockers(clp)) {
+exp_client:
+ if (mark_client_expired_locked(clp))
+ continue;
+ list_add(&clp->cl_lru, reaplist);
continue;
- list_add(&clp->cl_lru, reaplist);
+ }
+ if (!cour) {
+ spin_lock(&clp->cl_cs_lock);
+ clp->cl_cs_client_state = NFSD4_CLIENT_COURTESY;
+ spin_unlock(&clp->cl_cs_lock);
+ list_add(&clp->cl_cs_list, &cslist);
+ }
}
spin_unlock(&nn->client_lock);
+
+ while (!list_empty(&cslist)) {
+ clp = list_first_entry(&cslist, struct nfs4_client, cl_cs_list);
+ list_del_init(&clp->cl_cs_list);
+ spin_lock(&clp->cl_cs_lock);
+ /*
+ * Client might have re-connected. Make sure it's
+ * still in courtesy state before removing its record.
+ */
+ if (clp->cl_cs_client_state != NFSD4_CLIENT_COURTESY) {
+ spin_unlock(&clp->cl_cs_lock);
+ continue;
+ }
+ spin_unlock(&clp->cl_cs_lock);
+ nfsd4_client_record_remove(clp);
+ }
}
static time64_t
@@ -5818,6 +5900,13 @@ nfs4_laundromat(struct nfsd_net *nn)
dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
if (!state_expired(<, dp->dl_time))
break;
+ spin_lock(&clp->cl_cs_lock);
+ if (clp->cl_cs_client_state == NFSD4_CLIENT_COURTESY) {
+ clp->cl_cs_client_state = NFSD4_CLIENT_EXPIRED;
+ spin_unlock(&clp->cl_cs_lock);
+ continue;
+ }
+ spin_unlock(&clp->cl_cs_lock);
WARN_ON(!unhash_delegation_locked(dp));
list_add(&dp->dl_recall_lru, &reaplist);
}
--
2.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH RFC v17 10/11] NFSD: Update laundromat to handle courtesy clients
2022-03-17 7:43 ` [PATCH RFC v17 10/11] NFSD: Update laundromat to handle courtesy clients Dai Ngo
@ 2022-03-17 15:03 ` Chuck Lever III
2022-03-17 17:51 ` dai.ngo
0 siblings, 1 reply; 21+ messages in thread
From: Chuck Lever III @ 2022-03-17 15:03 UTC (permalink / raw)
To: Dai Ngo
Cc: Bruce Fields, Jeff Layton, Al Viro, Linux NFS Mailing List,
linux-fsdevel
> On Mar 17, 2022, at 3:43 AM, Dai Ngo <dai.ngo@oracle.com> wrote:
>
> Add nfs4_anylock_blocker and nfs4_lockowner_has_blockers to check
> if an expired client has any lock blockers
>
> Update nfs4_get_client_reaplist to:
> . add courtesy client in CLIENT_EXPIRED state to reaplist.
> . detect if expired client still has state and no blockers then
> transit it to courtesy client by setting CLIENT_COURTESY state
> and removing the client record.
>
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
> fs/nfsd/nfs4state.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 91 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index b4b976e00ce6..d5758c7101dc 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5755,24 +5755,106 @@ static void nfsd4_ssc_expire_umount(struct nfsd_net *nn)
> }
> #endif
>
> +/* Check if any lock belonging to this lockowner has any blockers */
> +static bool
> +nfs4_lockowner_has_blockers(struct nfs4_lockowner *lo)
> +{
> + struct file_lock_context *ctx;
> + struct nfs4_ol_stateid *stp;
> + struct nfs4_file *nf;
> +
> + list_for_each_entry(stp, &lo->lo_owner.so_stateids, st_perstateowner) {
> + nf = stp->st_stid.sc_file;
> + ctx = nf->fi_inode->i_flctx;
> + if (!ctx)
> + continue;
> + if (locks_owner_has_blockers(ctx, lo))
> + return true;
> + }
> + return false;
> +}
> +
> +static bool
> +nfs4_anylock_blockers(struct nfs4_client *clp)
> +{
> + int i;
> + struct nfs4_stateowner *so;
> + struct nfs4_lockowner *lo;
> +
> + spin_lock(&clp->cl_lock);
> + for (i = 0; i < OWNER_HASH_SIZE; i++) {
> + list_for_each_entry(so, &clp->cl_ownerstr_hashtbl[i],
> + so_strhash) {
> + if (so->so_is_open_owner)
> + continue;
> + lo = lockowner(so);
> + if (nfs4_lockowner_has_blockers(lo)) {
> + spin_unlock(&clp->cl_lock);
> + return true;
> + }
> + }
> + }
> + spin_unlock(&clp->cl_lock);
> + return false;
> +}
> +
> static void
> nfs4_get_client_reaplist(struct nfsd_net *nn, struct list_head *reaplist,
> struct laundry_time *lt)
> {
> struct list_head *pos, *next;
> struct nfs4_client *clp;
> + bool cour;
> + struct list_head cslist;
>
> INIT_LIST_HEAD(reaplist);
> + INIT_LIST_HEAD(&cslist);
> spin_lock(&nn->client_lock);
> list_for_each_safe(pos, next, &nn->client_lru) {
> clp = list_entry(pos, struct nfs4_client, cl_lru);
> if (!state_expired(lt, clp->cl_time))
> break;
> - if (mark_client_expired_locked(clp))
> +
> + if (!client_has_state(clp))
> + goto exp_client;
> +
> + if (clp->cl_cs_client_state == NFSD4_CLIENT_EXPIRED)
> + goto exp_client;
> + cour = (clp->cl_cs_client_state == NFSD4_CLIENT_COURTESY);
I've forgotten: why don't you need to hold clp->cl_cs_lock while
checking cs_client_state here?
> + if (cour && ktime_get_boottime_seconds() >=
> + (clp->cl_time + NFSD_COURTESY_CLIENT_TIMEOUT))
> + goto exp_client;
> + if (nfs4_anylock_blockers(clp)) {
> +exp_client:
> + if (mark_client_expired_locked(clp))
> + continue;
> + list_add(&clp->cl_lru, reaplist);
> continue;
> - list_add(&clp->cl_lru, reaplist);
> + }
> + if (!cour) {
> + spin_lock(&clp->cl_cs_lock);
> + clp->cl_cs_client_state = NFSD4_CLIENT_COURTESY;
> + spin_unlock(&clp->cl_cs_lock);
> + list_add(&clp->cl_cs_list, &cslist);
> + }
> }
> spin_unlock(&nn->client_lock);
> +
> + while (!list_empty(&cslist)) {
> + clp = list_first_entry(&cslist, struct nfs4_client, cl_cs_list);
> + list_del_init(&clp->cl_cs_list);
> + spin_lock(&clp->cl_cs_lock);
> + /*
> + * Client might have re-connected. Make sure it's
> + * still in courtesy state before removing its record.
> + */
> + if (clp->cl_cs_client_state != NFSD4_CLIENT_COURTESY) {
> + spin_unlock(&clp->cl_cs_lock);
> + continue;
> + }
> + spin_unlock(&clp->cl_cs_lock);
> + nfsd4_client_record_remove(clp);
> + }
> }
>
> static time64_t
> @@ -5818,6 +5900,13 @@ nfs4_laundromat(struct nfsd_net *nn)
> dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
> if (!state_expired(<, dp->dl_time))
> break;
> + spin_lock(&clp->cl_cs_lock);
> + if (clp->cl_cs_client_state == NFSD4_CLIENT_COURTESY) {
> + clp->cl_cs_client_state = NFSD4_CLIENT_EXPIRED;
> + spin_unlock(&clp->cl_cs_lock);
> + continue;
> + }
> + spin_unlock(&clp->cl_cs_lock);
> WARN_ON(!unhash_delegation_locked(dp));
> list_add(&dp->dl_recall_lru, &reaplist);
> }
> --
> 2.9.5
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH RFC v17 10/11] NFSD: Update laundromat to handle courtesy clients
2022-03-17 15:03 ` Chuck Lever III
@ 2022-03-17 17:51 ` dai.ngo
0 siblings, 0 replies; 21+ messages in thread
From: dai.ngo @ 2022-03-17 17:51 UTC (permalink / raw)
To: Chuck Lever III
Cc: Bruce Fields, Jeff Layton, Al Viro, Linux NFS Mailing List,
linux-fsdevel
On 3/17/22 8:03 AM, Chuck Lever III wrote:
>
>> On Mar 17, 2022, at 3:43 AM, Dai Ngo <dai.ngo@oracle.com> wrote:
>>
>> Add nfs4_anylock_blocker and nfs4_lockowner_has_blockers to check
>> if an expired client has any lock blockers
>>
>> Update nfs4_get_client_reaplist to:
>> . add courtesy client in CLIENT_EXPIRED state to reaplist.
>> . detect if expired client still has state and no blockers then
>> transit it to courtesy client by setting CLIENT_COURTESY state
>> and removing the client record.
>>
>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
>> ---
>> fs/nfsd/nfs4state.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 91 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index b4b976e00ce6..d5758c7101dc 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -5755,24 +5755,106 @@ static void nfsd4_ssc_expire_umount(struct nfsd_net *nn)
>> }
>> #endif
>>
>> +/* Check if any lock belonging to this lockowner has any blockers */
>> +static bool
>> +nfs4_lockowner_has_blockers(struct nfs4_lockowner *lo)
>> +{
>> + struct file_lock_context *ctx;
>> + struct nfs4_ol_stateid *stp;
>> + struct nfs4_file *nf;
>> +
>> + list_for_each_entry(stp, &lo->lo_owner.so_stateids, st_perstateowner) {
>> + nf = stp->st_stid.sc_file;
>> + ctx = nf->fi_inode->i_flctx;
>> + if (!ctx)
>> + continue;
>> + if (locks_owner_has_blockers(ctx, lo))
>> + return true;
>> + }
>> + return false;
>> +}
>> +
>> +static bool
>> +nfs4_anylock_blockers(struct nfs4_client *clp)
>> +{
>> + int i;
>> + struct nfs4_stateowner *so;
>> + struct nfs4_lockowner *lo;
>> +
>> + spin_lock(&clp->cl_lock);
>> + for (i = 0; i < OWNER_HASH_SIZE; i++) {
>> + list_for_each_entry(so, &clp->cl_ownerstr_hashtbl[i],
>> + so_strhash) {
>> + if (so->so_is_open_owner)
>> + continue;
>> + lo = lockowner(so);
>> + if (nfs4_lockowner_has_blockers(lo)) {
>> + spin_unlock(&clp->cl_lock);
>> + return true;
>> + }
>> + }
>> + }
>> + spin_unlock(&clp->cl_lock);
>> + return false;
>> +}
>> +
>> static void
>> nfs4_get_client_reaplist(struct nfsd_net *nn, struct list_head *reaplist,
>> struct laundry_time *lt)
>> {
>> struct list_head *pos, *next;
>> struct nfs4_client *clp;
>> + bool cour;
>> + struct list_head cslist;
>>
>> INIT_LIST_HEAD(reaplist);
>> + INIT_LIST_HEAD(&cslist);
>> spin_lock(&nn->client_lock);
>> list_for_each_safe(pos, next, &nn->client_lru) {
>> clp = list_entry(pos, struct nfs4_client, cl_lru);
>> if (!state_expired(lt, clp->cl_time))
>> break;
>> - if (mark_client_expired_locked(clp))
>> +
>> + if (!client_has_state(clp))
>> + goto exp_client;
>> +
>> + if (clp->cl_cs_client_state == NFSD4_CLIENT_EXPIRED)
>> + goto exp_client;
>> + cour = (clp->cl_cs_client_state == NFSD4_CLIENT_COURTESY);
> I've forgotten: why don't you need to hold clp->cl_cs_lock while
> checking cs_client_state here?
The CLIENT_EXPIRED can be set when either the client is reconnecting
or when a thread tries to resolve the conflict with the courtesy
client. The reconnecting client and the laundromat are synced by
nn->client_lock. For thread that tries to resolve conflict and set
CLIENT_EXPIRED, if the laundromat misses detecting CLIENT_EXPIRED
then it will get it the next time it runs.
The CLIENT_COURTESY state is only set by the laundromat so there is
no need for any lock when checking it here.
-Dai
>
>
>> + if (cour && ktime_get_boottime_seconds() >=
>> + (clp->cl_time + NFSD_COURTESY_CLIENT_TIMEOUT))
>> + goto exp_client;
>> + if (nfs4_anylock_blockers(clp)) {
>> +exp_client:
>> + if (mark_client_expired_locked(clp))
>> + continue;
>> + list_add(&clp->cl_lru, reaplist);
>> continue;
>> - list_add(&clp->cl_lru, reaplist);
>> + }
>> + if (!cour) {
>> + spin_lock(&clp->cl_cs_lock);
>> + clp->cl_cs_client_state = NFSD4_CLIENT_COURTESY;
>> + spin_unlock(&clp->cl_cs_lock);
>> + list_add(&clp->cl_cs_list, &cslist);
>> + }
>> }
>> spin_unlock(&nn->client_lock);
>> +
>> + while (!list_empty(&cslist)) {
>> + clp = list_first_entry(&cslist, struct nfs4_client, cl_cs_list);
>> + list_del_init(&clp->cl_cs_list);
>> + spin_lock(&clp->cl_cs_lock);
>> + /*
>> + * Client might have re-connected. Make sure it's
>> + * still in courtesy state before removing its record.
>> + */
>> + if (clp->cl_cs_client_state != NFSD4_CLIENT_COURTESY) {
>> + spin_unlock(&clp->cl_cs_lock);
>> + continue;
>> + }
>> + spin_unlock(&clp->cl_cs_lock);
>> + nfsd4_client_record_remove(clp);
>> + }
>> }
>>
>> static time64_t
>> @@ -5818,6 +5900,13 @@ nfs4_laundromat(struct nfsd_net *nn)
>> dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
>> if (!state_expired(<, dp->dl_time))
>> break;
>> + spin_lock(&clp->cl_cs_lock);
>> + if (clp->cl_cs_client_state == NFSD4_CLIENT_COURTESY) {
>> + clp->cl_cs_client_state = NFSD4_CLIENT_EXPIRED;
>> + spin_unlock(&clp->cl_cs_lock);
>> + continue;
>> + }
>> + spin_unlock(&clp->cl_cs_lock);
>> WARN_ON(!unhash_delegation_locked(dp));
>> list_add(&dp->dl_recall_lru, &reaplist);
>> }
>> --
>> 2.9.5
>>
> --
> Chuck Lever
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH RFC v17 11/11] NFSD: Show state of courtesy clients in client info
2022-03-17 7:43 [PATCH RFC v17 0/11] NFSD: Initial implementation of NFSv4 Courteous Server Dai Ngo
` (9 preceding siblings ...)
2022-03-17 7:43 ` [PATCH RFC v17 10/11] NFSD: Update laundromat to handle courtesy clients Dai Ngo
@ 2022-03-17 7:43 ` Dai Ngo
10 siblings, 0 replies; 21+ messages in thread
From: Dai Ngo @ 2022-03-17 7:43 UTC (permalink / raw)
To: chuck.lever, bfields; +Cc: jlayton, viro, linux-nfs, linux-fsdevel
Update client_info_show to show state of courtesy client
and time since last renew.
Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
fs/nfsd/nfs4state.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d5758c7101dc..21dd8265bef8 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2411,7 +2411,8 @@ static int client_info_show(struct seq_file *m, void *v)
{
struct inode *inode = m->private;
struct nfs4_client *clp;
- u64 clid;
+ u64 clid, hrs;
+ u32 mins, secs;
clp = get_nfsdfs_clp(inode);
if (!clp)
@@ -2419,10 +2420,17 @@ static int client_info_show(struct seq_file *m, void *v)
memcpy(&clid, &clp->cl_clientid, sizeof(clid));
seq_printf(m, "clientid: 0x%llx\n", clid);
seq_printf(m, "address: \"%pISpc\"\n", (struct sockaddr *)&clp->cl_addr);
- if (test_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags))
+
+ if (clp->cl_cs_client_state == NFSD4_CLIENT_COURTESY)
+ seq_puts(m, "status: courtesy\n");
+ else if (clp->cl_cs_client_state == NFSD4_CLIENT_CONFIRMED)
seq_puts(m, "status: confirmed\n");
else
seq_puts(m, "status: unconfirmed\n");
+ hrs = div_u64_rem(ktime_get_boottime_seconds() - clp->cl_time,
+ 3600, &secs);
+ mins = div_u64_rem((u64)secs, 60, &secs);
+ seq_printf(m, "time since last renew: %llu:%02u:%02u\n", hrs, mins, secs);
seq_printf(m, "name: ");
seq_quote_mem(m, clp->cl_name.data, clp->cl_name.len);
seq_printf(m, "\nminor version: %d\n", clp->cl_minorversion);
--
2.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread