Re: [PATCH] lockd: convert reclaimer thread to kthread interface

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Nov 3, 2008, at 19:19, Jeff Layton <jlayton@xxxxxxxxxx> wrote:

On Mon, 3 Nov 2008 13:12:15 -0800
Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:

On Wed, 29 Oct 2008 07:15:45 -0400
Jeff Layton <jlayton@xxxxxxxxxx> wrote:

My understanding is that there is a push to turn the kernel_thread
interface into a non-exported symbol and move all kernel threads to use the kthread API. This patch changes lockd to use kthread_run to spawn
the reclaimer thread.

I've made the assumption here that the extra module references taken
when we spawn this thread are unnecessary and removed them. I've also added a KERN_ERR printk that pops if the thread can't be spawned to warn
the admin that the locks won't be reclaimed.

I consider this patch 2.6.29 material.

Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
---
fs/lockd/clntlock.c |   14 +++++++++-----
1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/fs/lockd/clntlock.c b/fs/lockd/clntlock.c
index 8307dd6..fcc2378 100644
--- a/fs/lockd/clntlock.c
+++ b/fs/lockd/clntlock.c
@@ -14,6 +14,7 @@
#include <linux/sunrpc/svc.h>
#include <linux/lockd/lockd.h>
#include <linux/smp_lock.h>
+#include <linux/kthread.h>

#define NLMDBG_FACILITY        NLMDBG_CLIENT

@@ -191,11 +192,15 @@ __be32 nlmclnt_grant(const struct sockaddr *addr, const struct nlm_lock *lock)
void
nlmclnt_recovery(struct nlm_host *host)
{
+    struct task_struct *task;
+
   if (!host->h_reclaiming++) {
       nlm_get_host(host);
-        __module_get(THIS_MODULE);
- if (kernel_thread(reclaimer, host, CLONE_FS | CLONE_FILES) < 0)
-            module_put(THIS_MODULE);
+ task = kthread_run(reclaimer, host, "%s-reclaim", host- >h_name);
+        if (IS_ERR(task))
+            printk(KERN_ERR "lockd: unable to spawn reclaimer "
+                "thread. Locks for %s won't be reclaimed! "
+                "(%ld)\n", host->h_name, PTR_ERR(task));
   }
}

@@ -207,7 +212,6 @@ reclaimer(void *ptr)
   struct file_lock *fl, *next;
   u32 nsmstate;

-    daemonize("%s-reclaim", host->h_name);
   allow_signal(SIGKILL);

   down_write(&host->h_rwsem);
@@ -261,5 +265,5 @@ restart:
   nlm_release_host(host);
   lockd_down();
   unlock_kernel();
-    module_put_and_exit(0);
+    return 0;
}

Looks OK to me. I assume the SIGKILL handling has been carefully tested?


Is it correct to emit a warning and keep going if the thread didn't
start? Or would it be safer&saner to fail the whole mount (or whatever
syscall we're doing here..)


Forgot to answer this part...

This thread gets kicked off when the server has rebooted and we need to
reclaim our locks. There isn't a syscall on which we can return an
error to the user.

Aside from just warning the admin, I'm not sure what we can do here. We
might be able to start making all syscalls on the mount fail somehow,
but I don't think we have infrastructure for that and that may be
overkill anyway. I suppose we could also go to sleep and try to spawn the
thread again, but there's no guarantee of success there.







At some point RSN we should implement SIGLOST. That would be the closest thing we have to a *NIX standard for reporting the loss of filesystem state to the application.

Trond
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux