login
Header Space

 
 

Re: [PATCH] kill_something_info: don't take tasklist_lock for pid==-1 case

Score:
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Oleg Nesterov <oleg@...>
Cc: Atsushi Tsuji <a-tsuji@...>, <linux-kernel@...>, Roland McGrath <roland@...>
Date: Saturday, May 31, 2008 - 7:55 pm

Oleg Nesterov <oleg@tv-sign.ru> writes:


We can read old next values when walking the task list under the rcu
lock.  So I don't believe we are guaranteed to see additions that
happen while we hold the rcu lock.

If a new process spawns, passes the check for the parent having the
signal, the signal is delivered the signal, and then appends to the
task list.  We might miss it.  I'm not certain, but that feels right.


Hmm.  Does that problem affect normal signal deliver.  I seem to recall
being careful and doing something to make that case work.

Does that fix only apply when we have a specific pid, not when we have
a task and are walking the task_list.  Because kill_pid_info can retry
if we receive -ESRCH?

To fix the pid namespace case we need to start walking the list of
pids, not the task list for kill -1.


Oh right.  I had forgotten about that special case.  Grr Special cases
suck!

We have a hole with init spawning new children, during kill -1.
Ugh.  Or are those tasks indistinguishable from children spawned by
init just after the signal was sent?

A practical question.  I need to rework the signal delivery for
the case of kill -1 to be based on find_ge_pid.  So that it
works with pid namespaces.  That kills our nice in order list
property. :(  A rework does give the opportunity to investigate
these properties in detail, and see if there is a way to drop
the tasklist_lock.  Especially since we are already discussing it.

The guarantee is that the signal delivery needs to appear atomic
so that we don't miss any processes in the group of processes
the signal is sent to.  The rcu list gives us an effectively atomic
snapshot of the list.  However new processes can be added while we
walk that list.

To prevent the current visible DOS we need to only hold processes
in fork for the duration of our traversal of the set of all processes
in the system and then let them go.

The -ERESTARTNOINTR trick ensures the signal is delivered to the
parent before a fork executes.

Init only receives signals that it wants (the rest are effectively
ignored and discared) so it can be said to have forked after signal
delivery when there is a race.

We grab sighand lock on each process that we are delivering the signal
to.

This almost feels like a recipe for atomic signal delivery.  Without
an ordered list.  I just don't see it yet.  I will keep picking at it
since it sounds like we are close to the point where the tasklist is
a problem even on small systems.


Eric
--
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
Re: [PATCH] kill_something_info: don't take tasklist_lock fo..., Eric W. Biederman, (Tue May 20, 11:47 pm)
Re: [PATCH] kill_something_info: don't take tasklist_lock fo..., Eric W. Biederman, (Sat May 31, 7:55 pm)
Re: [PATCH] kill_something_info: don't take tasklist_lock fo..., Eric W. Biederman, (Wed May 28, 11:03 am)
Re: [PATCH] kill_something_info: don't take tasklist_lock fo..., Eric W. Biederman, (Tue May 20, 10:53 pm)
speck-geostationary