Hi Jeff,
[ Trimmed netdev from Cc: list, added Christoph. ]
On 6/26/07, Jeff Layton <[email protected]> wrote:
On Tue, 26 Jun 2007 01:11:20 +0530
"Satyam Sharma" <[email protected]> wrote:
> [...]
> Yes, why not embed a send_sig(SIGKILL) just before the wake_up_process()
> in kthread_stop() itself?
>
> Looking at some happily out-of-date comments in the kthread code, I can
> guess that at some point of time (perhaps very early drafts) Rusty actually
> *did* implement the whole kthread_stop() functionality using signals, but
> I suspect it might've been discarded and the kthread_stop_info approach
> used instead to protect from spurious signals from userspace. (?)
>
> So could we have signals in _addition_ to kthread_stop_info and change
> kthread_should_stop() to check for both:
>
> kthread_stop_info.k == current && signal_pending(current)
>
> If !kthread_should_stop() && signal_pending(current) => spurious signal,
> so just flush and discard (in the kthread).
> [...]
> Why is it wrong for kthreads to let signals through? We can ignore out
> all signals we're not interested in, and flush the spurious ones ...
> otherwise there really isn't much those kthreads can do that get blocked
> in such functions, is there?
Yes, after I wrote that I began to question that assumption too. I was
pretty much going on a statement by Christoph Hellwig on an earlier
patch that I did:
Ok, I found both the threads / patches you referred to ...
-----[snip]------
The right way to fix this is to stop sending signals at all and have
a kernel-internal way to get out of kernel_recvmsg. Uses of signals by
kernel thread generally are bugs.
-----[snip]------
Though this makes no sense to me. I don't see any reason why kthreads
can't use signals, and hacking support for breaking out of sleeping
functions seems redundant.
Right, signals _are_ the "signalling" mechanism all through kernel code
already, anything else would clearly be redundant.
But I've listened / participated in other discussions about kthreads and
signals and the general feeling is that (somebody correct me if I'm wrong)
kernel threads are a kernel _implementation detail_ after all, and good
design must ensure that userspace be unaware of even their existence.
And I agree with that, but the real ugly uses of signals by kernel threads
are those cases where we want to export a full signals-based interface to
our kthread to userspace (such cases do exist in mainline, I think).
But that's clearly not the case with the usage here.
My latest patch for cifsd has it block all signals from userspace
and uses force_sig() instead of send_sig() when trying to stop the
thread. This seems to work pretty well and still insulates the thread
from userspace signals.
Thanks, I find this solution much cleaner too, so now we avoid any
sort of spuriousness getting in from userspace (and pretty much takes
care of all the checking-if-spurious-and-flushing business I referred to
earlier).
But how about making this part of kthreads proper? Functions such as
skb_recv_datagram etc are pretty standard (and others such would also
exist or get added in due time) and it is not exactly intuitive for a developer
to add a force_sig(SIGKILL) before the kthread_stop() to ensure that the
kthread using such functions does exit properly. [ I can foresee cases in
the future when such functions are added to kthreads that did not have
them previously, and suddenly someone reports a regression that the
kthread stops exiting cleanly. ]
Satyam
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
