[PATCH] knfsd: Correctly handle error condition from lockd_up
NeilBrown [Mon, 2 Oct 2006 09:17:53 +0000 (02:17 -0700)]
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>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

fs/lockd/clntlock.c
fs/lockd/svc.c
fs/nfsd/nfssvc.c

index 6abb465..87e1d03 100644 (file)
@@ -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. */
index 448768b..3cc369e 100644 (file)
@@ -254,15 +254,11 @@ lockd_up(int proto) /* Maybe add a 'family' option when IPv6 is supported ?? */
 
        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 'family' option when IPv6 is supported ?? */
         * 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 'family' option when IPv6 is supported ?? */
 destroy_and_out:
        svc_destroy(serv);
 out:
+       if (!error)
+               nlmsvc_users++;
        mutex_unlock(&nlmsvc_mutex);
        return error;
 }
index f1314c6..cdec399 100644 (file)
@@ -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