Messages in this thread | | | Date | Mon, 25 May 2009 19:20:33 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 1/1] signal: make group kill signal fatal |
| |
On 05/25, Jiri Slaby wrote: > > On 05/25/2009 02:07 AM, Oleg Nesterov wrote: > > On 05/24, Jiri Slaby wrote: > >> > >> __fatal_signal_pending() returns now true only for a non-group sent > >> sigkill, i. e. for example tgkill, send_sig... > > > > No. Please look at complete_signal(). If we queue a fatal signal, > > we always add SIGKILL to any thread. > > Ah, thanks. But it looks like it doesn't work well in some cases. > Consider this kernel code: > > ... > static int release(struct inode *ino, struct file *file) > { > unsigned int a; > printk(KERN_DEBUG "fst\n"); > for (a = 0; a < 10; a++) { > printk(KERN_DEBUG "%s: SP=%u FSP=%u pend=%.16lx > shpend=%.16lx\n", > __func__, > signal_pending(current), > fatal_signal_pending(current), > current->pending.signal.sig[0], > > current->signal->shared_pending.signal.sig[0]); > > ... > > int main(int argc, char **argv) > { > struct pollfd fds = { .events = POLLIN }; > int fd; > > fd = open("/dev/m", O_RDONLY); > if (fd < 0) > err(1, "open"); > fds.fd = fd; > if (poll(&fds, 1, -1) < 0) > err(2, "poll"); > close(fd); > return 0; > } > > ---------------------------------------------- > It outputs: > fst > release: SP=1 FSP=0 pend=0000000000000000 shpend=0000000000000100
Because (I guess) ->release() is called by do_exit()->close_files(), by this time the private SIGKILL is already dequeued.
If you kill this program, poll() never returns to the user-space.
> If the poll isn't there, it works well.
Hmm. this is strange. Do you mean that if this program does sleep(10000) (or something else) instead of poll() above, it prints pend != 0 ?
Note. There are known issues with fatal_signal_pending() in exit() path. But in any case, we should not check ->shared_pending. And even if ->signal != NULL we must not use ->siglock. Because schedule() checks fatal_signal_pending() under rq->lock, we can deadlock. Also, the fact that SIGKILL is actually pending is the current implementation detail, probably this will be changed.
__fatal_signal_pending() could check SIGNAL_GROUP_EXIT, but we can't do this now, ->signal can be NULL. Actually signal_group_exit() is more correct.
And. Why do you need fatal_signal_pending() ? It is special, should be used by things like wait_event_killable().
Oleg.
| |