From: NeilBrown <neilb@suse.de>
To: Andrew Morton <akpm@osdl.org>
Cc: "J. Bruce Fields" <bfields@fieldses.org>,
nfs@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: [PATCH 004 of 4] knfsd: Correctly handle error condition from lockd_up
Date: Fri, 28 Jul 2006 15:10:03 +1000 [thread overview]
Message-ID: <1060728051003.570@suse.de> (raw)
In-Reply-To: 20060728150606.29533.patches@notabene
If lockd_up fails - what should we expect? Do we have to later call lockd_down?
Well the nfs client thinks "no", the nfs server thinks "yes".
lockd thinks "yes".
The only answer that really makes sense is "no" !!
So:
Make lockd_up only increment nlmsvc_users on success.
Make nfsd handle errors from lockd_up properly.
Make sure lockd_up(0) never fails when lockd is running
so that the 'reclaimer' call to lockd_up doesn't need to
be error checked.
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Signed-off-by: Neil Brown <neilb@suse.de>
### Diffstat output
./fs/lockd/clntlock.c | 2 +-
./fs/lockd/svc.c | 12 +++++-------
./fs/nfsd/nfssvc.c | 16 ++++++++++------
3 files changed, 16 insertions(+), 14 deletions(-)
diff .prev/fs/lockd/clntlock.c ./fs/lockd/clntlock.c
--- .prev/fs/lockd/clntlock.c 2006-07-28 14:53:28.000000000 +1000
+++ ./fs/lockd/clntlock.c 2006-07-28 15:01:38.000000000 +1000
@@ -202,7 +202,7 @@ reclaimer(void *ptr)
/* This one ensures that our parent doesn't terminate while the
* reclaim is in progress */
lock_kernel();
- lockd_up(0);
+ lockd_up(0); /* note: this cannot fail as lockd is already running */
nlmclnt_prepare_reclaim(host);
/* First, reclaim all locks that have been marked. */
diff .prev/fs/lockd/svc.c ./fs/lockd/svc.c
--- .prev/fs/lockd/svc.c 2006-07-28 15:01:30.000000000 +1000
+++ ./fs/lockd/svc.c 2006-07-28 15:01:38.000000000 +1000
@@ -254,15 +254,11 @@ lockd_up(int proto) /* Maybe add a 'fami
mutex_lock(&nlmsvc_mutex);
/*
- * Unconditionally increment the user count ... this is
- * the number of clients who _want_ a lockd process.
- */
- nlmsvc_users++;
- /*
* Check whether we're already up and running.
*/
if (nlmsvc_pid) {
- error = make_socks(nlmsvc_serv, proto);
+ if (proto)
+ error = make_socks(nlmsvc_serv, proto);
goto out;
}
@@ -270,7 +266,7 @@ lockd_up(int proto) /* Maybe add a 'fami
* Sanity check: if there's no pid,
* we should be the first user ...
*/
- if (nlmsvc_users > 1)
+ if (nlmsvc_users)
printk(KERN_WARNING
"lockd_up: no pid, %d users??\n", nlmsvc_users);
@@ -302,6 +298,8 @@ lockd_up(int proto) /* Maybe add a 'fami
destroy_and_out:
svc_destroy(serv);
out:
+ if (!error)
+ nlmsvc_users++;
mutex_unlock(&nlmsvc_mutex);
return error;
}
diff .prev/fs/nfsd/nfssvc.c ./fs/nfsd/nfssvc.c
--- .prev/fs/nfsd/nfssvc.c 2006-07-28 14:53:28.000000000 +1000
+++ ./fs/nfsd/nfssvc.c 2006-07-28 15:01:38.000000000 +1000
@@ -221,18 +221,22 @@ static int nfsd_init_socks(int port)
if (!list_empty(&nfsd_serv->sv_permsocks))
return 0;
- error = svc_makesock(nfsd_serv, IPPROTO_UDP, port);
- if (error < 0)
- return error;
error = lockd_up(IPPROTO_UDP);
+ if (error >= 0) {
+ error = svc_makesock(nfsd_serv, IPPROTO_UDP, port);
+ if (error < 0)
+ lockd_down();
+ }
if (error < 0)
return error;
#ifdef CONFIG_NFSD_TCP
- error = svc_makesock(nfsd_serv, IPPROTO_TCP, port);
- if (error < 0)
- return error;
error = lockd_up(IPPROTO_TCP);
+ if (error >= 0) {
+ error = svc_makesock(nfsd_serv, IPPROTO_TCP, port);
+ if (error < 0)
+ lockd_down();
+ }
if (error < 0)
return error;
#endif
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs
WARNING: multiple messages have this Message-ID (diff)
From: NeilBrown <neilb@suse.de>
To: Andrew Morton <akpm@osdl.org>
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Cc: nfs@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: [PATCH 004 of 4] knfsd: Correctly handle error condition from lockd_up
Date: Fri, 28 Jul 2006 15:10:03 +1000 [thread overview]
Message-ID: <1060728051003.570@suse.de> (raw)
In-Reply-To: 20060728150606.29533.patches@notabene
If lockd_up fails - what should we expect? Do we have to later call lockd_down?
Well the nfs client thinks "no", the nfs server thinks "yes".
lockd thinks "yes".
The only answer that really makes sense is "no" !!
So:
Make lockd_up only increment nlmsvc_users on success.
Make nfsd handle errors from lockd_up properly.
Make sure lockd_up(0) never fails when lockd is running
so that the 'reclaimer' call to lockd_up doesn't need to
be error checked.
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Signed-off-by: Neil Brown <neilb@suse.de>
### Diffstat output
./fs/lockd/clntlock.c | 2 +-
./fs/lockd/svc.c | 12 +++++-------
./fs/nfsd/nfssvc.c | 16 ++++++++++------
3 files changed, 16 insertions(+), 14 deletions(-)
diff .prev/fs/lockd/clntlock.c ./fs/lockd/clntlock.c
--- .prev/fs/lockd/clntlock.c 2006-07-28 14:53:28.000000000 +1000
+++ ./fs/lockd/clntlock.c 2006-07-28 15:01:38.000000000 +1000
@@ -202,7 +202,7 @@ reclaimer(void *ptr)
/* This one ensures that our parent doesn't terminate while the
* reclaim is in progress */
lock_kernel();
- lockd_up(0);
+ lockd_up(0); /* note: this cannot fail as lockd is already running */
nlmclnt_prepare_reclaim(host);
/* First, reclaim all locks that have been marked. */
diff .prev/fs/lockd/svc.c ./fs/lockd/svc.c
--- .prev/fs/lockd/svc.c 2006-07-28 15:01:30.000000000 +1000
+++ ./fs/lockd/svc.c 2006-07-28 15:01:38.000000000 +1000
@@ -254,15 +254,11 @@ lockd_up(int proto) /* Maybe add a 'fami
mutex_lock(&nlmsvc_mutex);
/*
- * Unconditionally increment the user count ... this is
- * the number of clients who _want_ a lockd process.
- */
- nlmsvc_users++;
- /*
* Check whether we're already up and running.
*/
if (nlmsvc_pid) {
- error = make_socks(nlmsvc_serv, proto);
+ if (proto)
+ error = make_socks(nlmsvc_serv, proto);
goto out;
}
@@ -270,7 +266,7 @@ lockd_up(int proto) /* Maybe add a 'fami
* Sanity check: if there's no pid,
* we should be the first user ...
*/
- if (nlmsvc_users > 1)
+ if (nlmsvc_users)
printk(KERN_WARNING
"lockd_up: no pid, %d users??\n", nlmsvc_users);
@@ -302,6 +298,8 @@ lockd_up(int proto) /* Maybe add a 'fami
destroy_and_out:
svc_destroy(serv);
out:
+ if (!error)
+ nlmsvc_users++;
mutex_unlock(&nlmsvc_mutex);
return error;
}
diff .prev/fs/nfsd/nfssvc.c ./fs/nfsd/nfssvc.c
--- .prev/fs/nfsd/nfssvc.c 2006-07-28 14:53:28.000000000 +1000
+++ ./fs/nfsd/nfssvc.c 2006-07-28 15:01:38.000000000 +1000
@@ -221,18 +221,22 @@ static int nfsd_init_socks(int port)
if (!list_empty(&nfsd_serv->sv_permsocks))
return 0;
- error = svc_makesock(nfsd_serv, IPPROTO_UDP, port);
- if (error < 0)
- return error;
error = lockd_up(IPPROTO_UDP);
+ if (error >= 0) {
+ error = svc_makesock(nfsd_serv, IPPROTO_UDP, port);
+ if (error < 0)
+ lockd_down();
+ }
if (error < 0)
return error;
#ifdef CONFIG_NFSD_TCP
- error = svc_makesock(nfsd_serv, IPPROTO_TCP, port);
- if (error < 0)
- return error;
error = lockd_up(IPPROTO_TCP);
+ if (error >= 0) {
+ error = svc_makesock(nfsd_serv, IPPROTO_TCP, port);
+ if (error < 0)
+ lockd_down();
+ }
if (error < 0)
return error;
#endif
next prev parent reply other threads:[~2006-07-28 5:10 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-07-28 5:09 [PATCH 000 of 4] knfsd: Introduction NeilBrown
2006-07-28 5:09 ` NeilBrown
2006-07-28 5:09 ` [PATCH 001 of 4] knfsd: Drop 'serv' option to svc_recv and svc_process NeilBrown
2006-07-28 5:09 ` NeilBrown
2006-07-28 5:09 ` [PATCH 002 of 4] knfsd: Check return value of lockd_up in write_ports NeilBrown
2006-07-28 5:09 ` NeilBrown
2006-07-28 5:09 ` [PATCH 003 of 4] knfsd: Move makesock failed warning into make_socks NeilBrown
2006-07-28 5:09 ` NeilBrown
2006-07-28 5:10 ` NeilBrown [this message]
2006-07-28 5:10 ` [PATCH 004 of 4] knfsd: Correctly handle error condition from lockd_up NeilBrown
2006-07-28 21:10 ` [PATCH 000 of 4] knfsd: Introduction J. Bruce Fields
2006-08-03 1:22 ` Neil Brown
2006-08-03 1:22 ` [NFS] " Neil Brown
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=1060728051003.570@suse.de \
--to=neilb@suse.de \
--cc=akpm@osdl.org \
--cc=bfields@fieldses.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nfs@lists.sourceforge.net \
/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.