[RFC][PATCH] Kernel thread signal handling.

[RFC][PATCH] Kernel thread signal handling.

Post by David Wood » Tue, 14 Oct 2003 19:40:10


. We need disallow_signal() to complement allow_signal().

2. We need a function which does dequeue_signal() _with_ locking.

3. We might need a better name for #2. Change if you care.

4. We need allow_signal() to actually allow signals other than
SIGKILL. Currently they get either converted to SIGKILL or
silently dropped, according to whether your kernel thread
happens to have sa_handler set for the signal in question.

It would be nicer to fix this in the signal delivery code
itself if (!tsk->mm) rather than by faking a handler in
allow_signal(). I'm not touching the signal delivery code
with a bargepole though. Hopefully the proposed change to
allow_signal() will provoke someone else into doing so.

===== include/linux/sched.h 1.171 vs edited =====
--- 1.171/include/linux/sched.h Thu Oct 9 23:13:53 2003
+++ edited/include/linux/sched.h Mon Oct 13 10:11:07 2003
@@ -576,6 +576,19 @@
extern void flush_signals(struct task_struct *);
extern void flush_signal_handlers(struct task_struct *, int force_default);
extern int dequeue_signal(struct task_struct *tsk, sigset_t *mask, siginfo_t *info);
+
+static inline int dequeue_signal_lock(struct task_struct *tsk, sigset_t *mask, siginfo_t *info)
+{
+ unsigned long flags;
+ int ret;
+
+ spin_lock_irqsave(&tsk->sighand->siglock, flags);
+ ret = dequeue_signal(tsk, mask, info);
+ spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
+
+ return ret;
+}
+
extern void block_all_signals(int (*notifier)(void *priv), void *priv,
sigset_t *mask);
extern void unblock_all_signals(void);
@@ -673,6 +686,7 @@
extern void reparent_to_init(void);
extern void daemonize(const char *, ...);
extern int allow_signal(int);
+extern int disallow_signal(int);
extern task_t *child_reaper;

extern int do_execve(char *, char __user * __user *, char __user * __user *, struct pt_regs *);
===== kernel/exit.c 1.116 vs edited =====
--- 1.116/kernel/exit.c Thu Oct 9 23:13:54 2003
+++ edited/kernel/exit.c Mon Oct 13 11:19:04 2003
@@ -273,12 +273,33 @@

spin_lock_irq(¤t->sighand->siglock);
sigdelset(¤t->blocked, sig);
+ if (!current->mm) {
+ /* Kernel threads handle their own signals.
+ Let the signal code know it'll be handled, so
+ that they don't get converted to SIGKILL or
+ just silently dropped */
+ current->sighand->action[(sig)-1].sa.sa_handler = (void *)2;
+ }
recalc_sigpending();
spin_unlock_irq(¤t->sighand->siglock);
return 0;
}

EXPORT_SYMBOL(allow_signal);
+
+int disallow_signal(int sig)
+{
+ if (sig < 1 || sig > _NSIG)
+ return -EINVAL;
+
+ spin_lock_irq(¤t->sighand->siglock);
+ sigaddset(¤t->blocked, sig);
+ recalc_sigpending();
+ spin_unlock_irq(¤t->sighand->siglock);
+ return 0;
+}
+
+EXPORT_SYMBOL(disallow_signal);

/*
* Put all the gunge required to become a kernel thread without
===== fs/jffs2/background.c 1.20 vs edited =====
--- 1.20/fs/jffs2/background.c Sat Oct 11 15:47:54 2003
+++ edited/fs/jffs2/background.c Mon Oct 13 11:17:54 2003
@@ -19,6 +19,7 @@
#include <linux/completion.h>
#include <linux/sched.h>
#include <linux/unistd.h>
+#include <linux/suspend.h>
#include "nodelist.h"


@@ -75,6 +76,9 @@
struct jffs2_sb_info *c = _c;

daemonize("jffs2_gcd_mtd%d", c->mtd->index);
+ allow_signal(SIGK
 
 
 

[RFC][PATCH] Kernel thread signal handling.

Post by Andrew Mor » Tue, 14 Oct 2003 20:10:05


Sigh. Using signals to communicate with kernel threads is evil. It keeps
on breaking and each site does it differently and we've had plenty of bugs
due to this practice.

Signals are for userspace and the signal developers shouldn't have to worry
about weird in-kernel abuse and we have other simpler, more reliable
mechanisms available in-kernel and even more such ranting you get the
point.

Is there no way in which jffs2 can be weaned off this obnoxious habit?


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/

 
 
 

[RFC][PATCH] Kernel thread signal handling.

Post by Russell Ki » Tue, 14 Oct 2003 20:20:10


Even so, how does the current signal code react to the case where
a userspace process installs (eg) a SIGINT handler immediately
before entering a region which it needs to clean up. Meanwhile
a SIGINT is sent on some other CPU, discovers that there wasn't a
SIGINT handler when it looked, and queues a SIGKILL instead.
(this case was one which dwmw2 mentioned.)

I'm not certain if this can happen (the signal code is far to hairly
to work through the possibilities.) However, we used to decide if
the signal was fatal to the process far later in the signal handling
(when delivering it to the user space process in the context of that
process) rather than in some other random context which may be on a
different CPU.


jffs2 is using signal handlers as a method of communicating from user
space to kernel space. Maybe it should create some sysfs files.
However, since there aren't any existing sysfs entities for jffs2 to
attach these files to, this wouldn't seem to be reasonable.

Maybe jffs2 needs /proc/fs/jffs2/<jffs2_instance>/foo but we all know
peoples feelings on procfs.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.yqcomputer.com/
maintainer of: 2.6 PCMCIA - http://www.yqcomputer.com/
2.6 Serial core
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

[RFC][PATCH] Kernel thread signal handling.

Post by David Wood » Tue, 14 Oct 2003 20:30:15


The point in cleaning up allow_signal() et al. is that it gets simple
and it stops breaking. Not that I recall having signal problems with the
JFFS2 garbage collection thread other than this one, mind you.


We have a kernel thread which performs garbage collection on our
log-structured file system, to make space ahead of time for writes to
happen. It's purely an optimisation -- we also perform garbage
collection just-in-time in the context of a process which wants to
actually _write_, if there's no free space but some could be made.

This garbage collection involves reading, writing and erasing the flash.
It takes CPU time and power. Sometimes userspace wants it to stop
happening in the background; sometimes userspace wants it to resume
again.

So userspace sends SIGSTOP, SIGCONT and SIGKILL to the garbage
collection thread and all of them have the expected effect.

Since we handle these signals anyway, the normal wakeup of the GC thread
when the amount of free space changes is also done by SIGHUP, which
userspace can also send to trigger a single pass.

I don't any the benefit in changing this practice.

--
dwmw2

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

[RFC][PATCH] Kernel thread signal handling.

Post by David Wood » Tue, 14 Oct 2003 20:40:07


To clarify -- the JFFS2 garbage collect thread is using SIGSTOP, SIGCONT
and SIGKILL from userspace to mean exactly what SIGSTOP, SIGCONT and
SIGKILL always mean in userspace.

Since it already has signal handling code, we also use SIGHUP for
wakeup, and userspace can send that too to trigger a single GC pass.

--
dwmw2

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

[RFC][PATCH] Kernel thread signal handling.

Post by Andrew Mor » Tue, 14 Oct 2003 20:40:30


It will encourage kernel developers to use signals-to-kernel threads more,
and we don't *need* this capability.

People think "I need to send a message to a kernel thread" and then,
immediately, "ah-hah! I'll use a signal!"


Sounds like the GC should have been performed by a userspace process in the
first place?

How does userspace identify the JFFS2 process to which to send the signal?


Well I know I'm going to lose this one, but at least I get to *** about
stuff.

sysfs would be appropriate, as would a sysctl handler. An ioctl might even
work, although it gets a visit from the ioctl police and sometimes it is
hard to obtain an fd on the appropriate filesystem. If the call rate is
low, `mount -o remount,...' can be used to send a message to a filesystem.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

[RFC][PATCH] Kernel thread signal handling.

Post by David Wood » Tue, 14 Oct 2003 21:00:23


I've seen relatively little of this. Most of the problems I've been
aware of have been kernel threads _not_ handling signals (or handling
only SIGKILL) and going into endless loops of bouncing straight back out
of schedule().

That problem is almost unrelated -- it happens because driver writes
want to sleep in TASK_INTERRUPTIBLE state rather than
TASK_UNINTERRUPTIBLE. The fix for that is to have a per-task
'uninterruptible_count' along much the same lines as preempt_count,
where each function which is unable to handle an -EINTR return
increments the count before calling down to another function which may
have done that. But that's a 2.7 thing and mostly not related to this
particular bug.


Well, it would have to actually be done in kernel space but I suppose
there could be an ioctl or syscall or something which causes a call to
jffs2_garbage_collect_pass() to happen in the context of the caller, and
the variables which are used to decide when to wake up could be exposed
to userspace via sysfs, and the userspace daemon itself could register
with the JFFS2 code so that it gets woken when those variables change...
or maybe I could poll() on the sysfs file which contains them I
suppose...

Er, no :)


daemonize("jffs2_gcd_mtd%d", c->mtd->index);


Fair enough :)

*** ing accomplished; now can we fix the bug?

--
dwmw2

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

[RFC][PATCH] Kernel thread signal handling.

Post by Andrew Mor » Tue, 14 Oct 2003 21:20:10


And then what? Parse the output of ps(1)? Use pidof(8)?


Insufficient contrition detected :)

Why cannot a procfs or sysfs control be used?

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

[RFC][PATCH] Kernel thread signal handling.

Post by David Wood » Tue, 14 Oct 2003 21:40:14


Those work.


Hehe.


Mostly because I think that idea sucks, and partly because I think your
ire is misdirected -- it shouldn't be directed at kernel code which
handles signals, but rather at kernel code which sleeps in state
TASK_INTERRUPTIBLE but _doesn't_ handle signals.

On the other hand, if you ever find people actually trying to pass
_data_ to kernel threads with sys_rt_sigqueueinfo(), I'll be right
behind you with my own baseball bat waiting to bash the pieces you leave
behind :)

--
dwmw2

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

[RFC][PATCH] Kernel thread signal handling.

Post by Linus Torv » Wed, 15 Oct 2003 01:40:28


Yeah, it's not wonderfully pretty, but at least this patch makes bugs
_less_ likely, by having more of the common stuff abstracted out. And
clearly allow_signal() was broken before, since it didn't allow anything
but deadly signals, despite the fact that it was supposed to be generic.

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/