From: J. Bruce Fields <bfields@fieldses.org>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] Re: [PATCH 3/3] NLM enable per-ip base grace period
Date: Tue, 29 Jan 2008 19:23:13 -0500 [thread overview]
Message-ID: <20080130002313.GS28032@fieldses.org> (raw)
In-Reply-To: <4799714B.9070900@redhat.com>
Thanks for the patches; a few more comments below:
On Fri, Jan 25, 2008 at 12:19:07AM -0500, Wendy Cheng wrote:
> Hooks are added into the existing lockd global grace period checking to
> enable per-ip base grace period.
>
> -- Wendy
>
>
> Hooks are added into the existing lockd global grace period checking
> to enable per-ip base grace period.
>
> Signed-off-by: S. Wendy Cheng <wcheng@redhat.com>
> Signed-off-by: Lon Hohberger <lhh@redhat.com>
>
> fs/lockd/svc.c | 3 ++-
> fs/lockd/svc4proc.c | 13 ++++++-------
> fs/lockd/svcproc.c | 12 ++++++------
> include/linux/lockd/lockd.h | 20 ++++++++++++++++++++
> 4 files changed, 34 insertions(+), 14 deletions(-)
>
> --- linux-3/fs/lockd/svc4proc.c 2008-01-22 12:01:33.000000000 -0500
> +++ linux-4/fs/lockd/svc4proc.c 2008-01-24 17:44:12.000000000 -0500
> @@ -18,7 +18,6 @@
> #include <linux/lockd/share.h>
> #include <linux/lockd/sm_inter.h>
>
> -
> #define NLMDBG_FACILITY NLMDBG_CLIENT
>
> /*
> @@ -89,7 +88,7 @@ nlm4svc_proc_test(struct svc_rqst *rqstp
> resp->cookie = argp->cookie;
>
> /* Don't accept test requests during grace period */
> - if (nlmsvc_grace_period) {
> + if (nlmsvc_check_grace_period(rqstp, argp)) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -121,7 +120,7 @@ nlm4svc_proc_lock(struct svc_rqst *rqstp
> resp->cookie = argp->cookie;
>
> /* Don't accept new lock requests during grace period */
> - if (nlmsvc_grace_period && !argp->reclaim) {
> + if (nlmsvc_check_grace_period(rqstp, argp) && !argp->reclaim) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -166,7 +165,7 @@ nlm4svc_proc_cancel(struct svc_rqst *rqs
> resp->cookie = argp->cookie;
>
> /* Don't accept requests during grace period */
> - if (nlmsvc_grace_period) {
> + if (nlmsvc_check_grace_period(rqstp, argp)) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -199,7 +198,7 @@ nlm4svc_proc_unlock(struct svc_rqst *rqs
> resp->cookie = argp->cookie;
>
> /* Don't accept new lock requests during grace period */
> - if (nlmsvc_grace_period) {
> + if (nlmsvc_check_grace_period(rqstp, argp)) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -336,7 +335,7 @@ nlm4svc_proc_share(struct svc_rqst *rqst
> resp->cookie = argp->cookie;
>
> /* Don't accept new lock requests during grace period */
> - if (nlmsvc_grace_period && !argp->reclaim) {
> + if (nlmsvc_check_grace_period(rqstp, argp) && !argp->reclaim) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -369,7 +368,7 @@ nlm4svc_proc_unshare(struct svc_rqst *rq
> resp->cookie = argp->cookie;
>
> /* Don't accept requests during grace period */
> - if (nlmsvc_grace_period) {
> + if (nlmsvc_check_grace_period(rqstp, argp)) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> --- linux-3/fs/lockd/svcproc.c 2008-01-22 12:01:33.000000000 -0500
> +++ linux-4/fs/lockd/svcproc.c 2008-01-24 17:45:04.000000000 -0500
> @@ -118,7 +118,7 @@ nlmsvc_proc_test(struct svc_rqst *rqstp,
> resp->cookie = argp->cookie;
>
> /* Don't accept test requests during grace period */
> - if (nlmsvc_grace_period) {
> + if (nlmsvc_check_grace_period(rqstp, argp)) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -151,7 +151,7 @@ nlmsvc_proc_lock(struct svc_rqst *rqstp,
> resp->cookie = argp->cookie;
>
> /* Don't accept new lock requests during grace period */
> - if (nlmsvc_grace_period && !argp->reclaim) {
> + if (nlmsvc_check_grace_period(rqstp, argp) && !argp->reclaim) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -196,7 +196,7 @@ nlmsvc_proc_cancel(struct svc_rqst *rqst
> resp->cookie = argp->cookie;
>
> /* Don't accept requests during grace period */
> - if (nlmsvc_grace_period) {
> + if (nlmsvc_check_grace_period(rqstp, argp)) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -229,7 +229,7 @@ nlmsvc_proc_unlock(struct svc_rqst *rqst
> resp->cookie = argp->cookie;
>
> /* Don't accept new lock requests during grace period */
> - if (nlmsvc_grace_period) {
> + if (nlmsvc_check_grace_period(rqstp, argp)) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -368,7 +368,7 @@ nlmsvc_proc_share(struct svc_rqst *rqstp
> resp->cookie = argp->cookie;
>
> /* Don't accept new lock requests during grace period */
> - if (nlmsvc_grace_period && !argp->reclaim) {
> + if (nlmsvc_check_grace_period(rqstp, argp) && !argp->reclaim) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -401,7 +401,7 @@ nlmsvc_proc_unshare(struct svc_rqst *rqs
> resp->cookie = argp->cookie;
>
> /* Don't accept requests during grace period */
> - if (nlmsvc_grace_period) {
> + if (nlmsvc_check_grace_period(rqstp, argp)) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> --- linux-3/fs/lockd/svc.c 2008-01-24 17:30:55.000000000 -0500
> +++ linux-4/fs/lockd/svc.c 2008-01-24 17:41:40.000000000 -0500
> @@ -97,7 +97,7 @@ unsigned long get_nfs_grace_period(void)
> }
> EXPORT_SYMBOL(get_nfs_grace_period);
>
> -static unsigned long set_grace_period(void)
> +unsigned long set_grace_period(void)
If this is needed, then it belongs with the previous patch, doesn't it?
> {
> nlmsvc_grace_period = 1;
> return get_nfs_grace_period() + jiffies;
> @@ -208,6 +208,7 @@ lockd(struct svc_rqst *rqstp)
> nlm_shutdown_hosts();
> nlmsvc_pid = 0;
> nlmsvc_serv = NULL;
> + nlmsvc_failover_reset();
Ditto. But I thought the previous patch already added an
nlmsvc_failover_reset() call just a few lines below.
> } else
> printk(KERN_DEBUG
> "lockd: new process, skipping host shutdown\n");
> --- linux-3/include/linux/lockd/lockd.h 2008-01-24 17:09:26.000000000 -0500
> +++ linux-4/include/linux/lockd/lockd.h 2008-01-24 17:41:40.000000000 -0500
> @@ -222,6 +222,7 @@ int nlmsvc_failover_path(struc
> int nlmsvc_failover_ip(__be32 server_addr);
> int nlmsvc_failover_setgrace(void *server_ip, int ip_size);
> void nlmsvc_failover_reset(void);
> +int nlmsvc_failover_check(struct svc_rqst *rqstp);
>
> #define NLM_FO_MAX_GP_CNT 1024
>
> @@ -236,6 +237,25 @@ struct nlm_failover_struct {
> #define g_ip g_key.ipv6
> };
>
> +extern struct list_head nlm_failover_list;
> +
> +/*Check for grace period: return TRUE or FALSE */
> +static inline int
> +nlmsvc_check_grace_period(struct svc_rqst *rqstp, struct nlm_args *argp)
> +{
> + /* check for system wide grace period */
> + if (nlmsvc_grace_period)
> + return 1;
> +
> + /* check for per exported fsid grace period */
The "per exported fsid" comment seems to be out of date.
> + if (unlikely(!list_empty(&nlm_failover_list)))
> + return(nlmsvc_failover_check(rqstp));
Drop the parentheses after "return".
Is the "unlikely" a good idea? I don't know what the rule of thumb is
for those.
> +
> + return 0;
> +}
> +
> +/* end of cluster failover addition */
> +
> static __inline__ struct inode *
> nlmsvc_file_inode(struct nlm_file *file)
> {
WARNING: multiple messages have this Message-ID (diff)
From: "J. Bruce Fields" <bfields@fieldses.org>
To: Wendy Cheng <wcheng@redhat.com>
Cc: NFS list <linux-nfs@vger.kernel.org>, cluster-devel@redhat.com
Subject: Re: [PATCH 3/3] NLM enable per-ip base grace period
Date: Tue, 29 Jan 2008 19:23:13 -0500 [thread overview]
Message-ID: <20080130002313.GS28032@fieldses.org> (raw)
In-Reply-To: <4799714B.9070900@redhat.com>
Thanks for the patches; a few more comments below:
On Fri, Jan 25, 2008 at 12:19:07AM -0500, Wendy Cheng wrote:
> Hooks are added into the existing lockd global grace period checking to
> enable per-ip base grace period.
>
> -- Wendy
>
>
> Hooks are added into the existing lockd global grace period checking
> to enable per-ip base grace period.
>
> Signed-off-by: S. Wendy Cheng <wcheng@redhat.com>
> Signed-off-by: Lon Hohberger <lhh@redhat.com>
>
> fs/lockd/svc.c | 3 ++-
> fs/lockd/svc4proc.c | 13 ++++++-------
> fs/lockd/svcproc.c | 12 ++++++------
> include/linux/lockd/lockd.h | 20 ++++++++++++++++++++
> 4 files changed, 34 insertions(+), 14 deletions(-)
>
> --- linux-3/fs/lockd/svc4proc.c 2008-01-22 12:01:33.000000000 -0500
> +++ linux-4/fs/lockd/svc4proc.c 2008-01-24 17:44:12.000000000 -0500
> @@ -18,7 +18,6 @@
> #include <linux/lockd/share.h>
> #include <linux/lockd/sm_inter.h>
>
> -
> #define NLMDBG_FACILITY NLMDBG_CLIENT
>
> /*
> @@ -89,7 +88,7 @@ nlm4svc_proc_test(struct svc_rqst *rqstp
> resp->cookie = argp->cookie;
>
> /* Don't accept test requests during grace period */
> - if (nlmsvc_grace_period) {
> + if (nlmsvc_check_grace_period(rqstp, argp)) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -121,7 +120,7 @@ nlm4svc_proc_lock(struct svc_rqst *rqstp
> resp->cookie = argp->cookie;
>
> /* Don't accept new lock requests during grace period */
> - if (nlmsvc_grace_period && !argp->reclaim) {
> + if (nlmsvc_check_grace_period(rqstp, argp) && !argp->reclaim) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -166,7 +165,7 @@ nlm4svc_proc_cancel(struct svc_rqst *rqs
> resp->cookie = argp->cookie;
>
> /* Don't accept requests during grace period */
> - if (nlmsvc_grace_period) {
> + if (nlmsvc_check_grace_period(rqstp, argp)) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -199,7 +198,7 @@ nlm4svc_proc_unlock(struct svc_rqst *rqs
> resp->cookie = argp->cookie;
>
> /* Don't accept new lock requests during grace period */
> - if (nlmsvc_grace_period) {
> + if (nlmsvc_check_grace_period(rqstp, argp)) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -336,7 +335,7 @@ nlm4svc_proc_share(struct svc_rqst *rqst
> resp->cookie = argp->cookie;
>
> /* Don't accept new lock requests during grace period */
> - if (nlmsvc_grace_period && !argp->reclaim) {
> + if (nlmsvc_check_grace_period(rqstp, argp) && !argp->reclaim) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -369,7 +368,7 @@ nlm4svc_proc_unshare(struct svc_rqst *rq
> resp->cookie = argp->cookie;
>
> /* Don't accept requests during grace period */
> - if (nlmsvc_grace_period) {
> + if (nlmsvc_check_grace_period(rqstp, argp)) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> --- linux-3/fs/lockd/svcproc.c 2008-01-22 12:01:33.000000000 -0500
> +++ linux-4/fs/lockd/svcproc.c 2008-01-24 17:45:04.000000000 -0500
> @@ -118,7 +118,7 @@ nlmsvc_proc_test(struct svc_rqst *rqstp,
> resp->cookie = argp->cookie;
>
> /* Don't accept test requests during grace period */
> - if (nlmsvc_grace_period) {
> + if (nlmsvc_check_grace_period(rqstp, argp)) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -151,7 +151,7 @@ nlmsvc_proc_lock(struct svc_rqst *rqstp,
> resp->cookie = argp->cookie;
>
> /* Don't accept new lock requests during grace period */
> - if (nlmsvc_grace_period && !argp->reclaim) {
> + if (nlmsvc_check_grace_period(rqstp, argp) && !argp->reclaim) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -196,7 +196,7 @@ nlmsvc_proc_cancel(struct svc_rqst *rqst
> resp->cookie = argp->cookie;
>
> /* Don't accept requests during grace period */
> - if (nlmsvc_grace_period) {
> + if (nlmsvc_check_grace_period(rqstp, argp)) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -229,7 +229,7 @@ nlmsvc_proc_unlock(struct svc_rqst *rqst
> resp->cookie = argp->cookie;
>
> /* Don't accept new lock requests during grace period */
> - if (nlmsvc_grace_period) {
> + if (nlmsvc_check_grace_period(rqstp, argp)) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -368,7 +368,7 @@ nlmsvc_proc_share(struct svc_rqst *rqstp
> resp->cookie = argp->cookie;
>
> /* Don't accept new lock requests during grace period */
> - if (nlmsvc_grace_period && !argp->reclaim) {
> + if (nlmsvc_check_grace_period(rqstp, argp) && !argp->reclaim) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> @@ -401,7 +401,7 @@ nlmsvc_proc_unshare(struct svc_rqst *rqs
> resp->cookie = argp->cookie;
>
> /* Don't accept requests during grace period */
> - if (nlmsvc_grace_period) {
> + if (nlmsvc_check_grace_period(rqstp, argp)) {
> resp->status = nlm_lck_denied_grace_period;
> return rpc_success;
> }
> --- linux-3/fs/lockd/svc.c 2008-01-24 17:30:55.000000000 -0500
> +++ linux-4/fs/lockd/svc.c 2008-01-24 17:41:40.000000000 -0500
> @@ -97,7 +97,7 @@ unsigned long get_nfs_grace_period(void)
> }
> EXPORT_SYMBOL(get_nfs_grace_period);
>
> -static unsigned long set_grace_period(void)
> +unsigned long set_grace_period(void)
If this is needed, then it belongs with the previous patch, doesn't it?
> {
> nlmsvc_grace_period = 1;
> return get_nfs_grace_period() + jiffies;
> @@ -208,6 +208,7 @@ lockd(struct svc_rqst *rqstp)
> nlm_shutdown_hosts();
> nlmsvc_pid = 0;
> nlmsvc_serv = NULL;
> + nlmsvc_failover_reset();
Ditto. But I thought the previous patch already added an
nlmsvc_failover_reset() call just a few lines below.
> } else
> printk(KERN_DEBUG
> "lockd: new process, skipping host shutdown\n");
> --- linux-3/include/linux/lockd/lockd.h 2008-01-24 17:09:26.000000000 -0500
> +++ linux-4/include/linux/lockd/lockd.h 2008-01-24 17:41:40.000000000 -0500
> @@ -222,6 +222,7 @@ int nlmsvc_failover_path(struc
> int nlmsvc_failover_ip(__be32 server_addr);
> int nlmsvc_failover_setgrace(void *server_ip, int ip_size);
> void nlmsvc_failover_reset(void);
> +int nlmsvc_failover_check(struct svc_rqst *rqstp);
>
> #define NLM_FO_MAX_GP_CNT 1024
>
> @@ -236,6 +237,25 @@ struct nlm_failover_struct {
> #define g_ip g_key.ipv6
> };
>
> +extern struct list_head nlm_failover_list;
> +
> +/*Check for grace period: return TRUE or FALSE */
> +static inline int
> +nlmsvc_check_grace_period(struct svc_rqst *rqstp, struct nlm_args *argp)
> +{
> + /* check for system wide grace period */
> + if (nlmsvc_grace_period)
> + return 1;
> +
> + /* check for per exported fsid grace period */
The "per exported fsid" comment seems to be out of date.
> + if (unlikely(!list_empty(&nlm_failover_list)))
> + return(nlmsvc_failover_check(rqstp));
Drop the parentheses after "return".
Is the "unlikely" a good idea? I don't know what the rule of thumb is
for those.
> +
> + return 0;
> +}
> +
> +/* end of cluster failover addition */
> +
> static __inline__ struct inode *
> nlmsvc_file_inode(struct nlm_file *file)
> {
next prev parent reply other threads:[~2008-01-30 0:23 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-01-25 5:19 [Cluster-devel] [PATCH 3/3] NLM enable per-ip base grace period Wendy Cheng
2008-01-25 5:19 ` Wendy Cheng
2008-01-30 0:23 ` J. Bruce Fields [this message]
2008-01-30 0:23 ` J. Bruce Fields
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080130002313.GS28032@fieldses.org \
--to=bfields@fieldses.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.