kthread: update lockd to use kthread

kthread: update lockd to use kthread

[Cedric Le Goater]

Convert lockd to use kthread rather than kernel_thread, which is deprecated.

Not sure how to test it.

Signed-off-by: Cedric Le Goater <clg@fr.ibm.com>
Cc: Neil Brown <neilb@suse.de>
Cc: nfs@lists.sourceforge.net

---
 fs/lockd/clntlock.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Index: 2.6.18-rc4-mm2/fs/lockd/clntlock.c
===================================================================
--- 2.6.18-rc4-mm2.orig/fs/lockd/clntlock.c
+++ 2.6.18-rc4-mm2/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

@@ -181,9 +182,12 @@ nlmclnt_recovery(struct nlm_host *host,
 		return;
 	host->h_nsmstate = newstate;
 	if (!host->h_reclaiming++) {
+		struct task_struct* task;
+
 		nlm_get_host(host);
 		__module_get(THIS_MODULE);
-		if (kernel_thread(reclaimer, host, CLONE_KERNEL) < 0)
+		task = kthread_run(reclaimer, host, "%s-reclaim", host->h_name);
+		if (IS_ERR(task))
 			module_put(THIS_MODULE);
 	}
 }
@@ -196,7 +200,6 @@ reclaimer(void *ptr)
 	struct file_lock *fl, *next;
 	u32 nsmstate;

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

 	/* This one ensures that our parent doesn't terminate while the

Re: [NFS] kthread: update lockd to use kthread

[Christoph Hellwig]

>  	host->h_nsmstate = newstate;
>  	if (!host->h_reclaiming++) {
> +		struct task_struct* task;
> +
>  		nlm_get_host(host);
>  		__module_get(THIS_MODULE);
> -		if (kernel_thread(reclaimer, host, CLONE_KERNEL) < 0)
> +		task = kthread_run(reclaimer, host, "%s-reclaim", host->h_name);
> +		if (IS_ERR(task))
>  			module_put(THIS_MODULE);

Folks, this kind of patches is really useless.  If I wanted to just replace
kernel_thread() with kthread_run() I could do it myself in a day or two.

The whole point of the kthread API is that we now have a coherent set
of functions that deal with all aspects of kernel thread handling.  And
a conversion to that always involves rething the whole way a driver
uses kernel threads, and that's a good thing because most users were
buggy or at least rather odd.

sunrpc is not an exception to that, the thread handling is very interesting,
including things like using signals for various things possibly not waiting
for threads to exit.

If you don't feel like poking into all these nasty internal leave the
conversation to someone else, preferably a nfs developer.

Re: [NFS] kthread: update lockd to use kthread

[Cedric Le Goater]

Hello !

Christoph Hellwig wrote:
>>  	host->h_nsmstate = newstate;
>>  	if (!host->h_reclaiming++) {
>> +		struct task_struct* task;
>> +
>>  		nlm_get_host(host);
>>  		__module_get(THIS_MODULE);
>> -		if (kernel_thread(reclaimer, host, CLONE_KERNEL) < 0)
>> +		task = kthread_run(reclaimer, host, "%s-reclaim", host->h_name);
>> +		if (IS_ERR(task))
>>  			module_put(THIS_MODULE);
> 
> Folks, this kind of patches is really useless.  If I wanted to just replace
> kernel_thread() with kthread_run() I could do it myself in a day or two.

ok ok. The real work that needs to be done is in sunrpc and I'm still
*very* far from providing any patches.

This kernel thread is a sub thread of the lockd thread. It's a bit more
simple and does not require as much modification. It still depends on the
way its parent is killed and it will require some more work when the sunrpc
thread are fixed.

> The whole point of the kthread API is that we now have a coherent set
> of functions that deal with all aspects of kernel thread handling.  And
> a conversion to that always involves rething the whole way a driver
> uses kernel threads, and that's a good thing because most users were
> buggy or at least rather odd.
>
> sunrpc is not an exception to that, the thread handling is very interesting,
> including things like using signals for various things possibly not waiting
> for threads to exit.

The SIGKILL used to terminate the threads is a challenge ... it propagates
to sub threads. Some other stuff that bother me in using the kthread api in
sunrpc : nfs_callback_down() uses a wait on a completion with a timeout.
This is not possible with the kthread but might not be really useful. dunno.

We might need some enhancements to kthread to make everyone happy or some
work around in the model.

> If you don't feel like poking into all these nasty internal leave the
> conversation to someone else, preferably a nfs developer.

They would certainly do a better job than me. I'm discovering the code.

Thanks,

C.

Re: [NFS] kthread: update lockd to use kthread

[Christoph Hellwig]

On Fri, Aug 25, 2006 at 07:07:59PM +0200, Cedric Le Goater wrote:
> This kernel thread is a sub thread of the lockd thread. It's a bit more
> simple and does not require as much modification. It still depends on the
> way its parent is killed and it will require some more work when the sunrpc
> thread are fixed.

Yes, the whole sunrpc/nfs thread situation is rather difficult.

> The SIGKILL used to terminate the threads is a challenge ... it propagates
> to sub threads. Some other stuff that bother me in using the kthread api in
> sunrpc : nfs_callback_down() uses a wait on a completion with a timeout.
> This is not possible with the kthread but might not be really useful. dunno.

The usage look buggy to me because we can't guarantee thread exit.  But
I really don't even understand the requirements for those threads yet,
I wish someone could write up a coherent description.