Signal-related changes for 3.19
Subsystems (3)
cover Signal-related changes for 3.19
Hello!
This series updates RCU handling in signals:
1. Prevents a possible very long RCU read-side critical section
in kill_pid_info().
2. Explain why the "obviously buggy" freeing of sighand by
__cleanup_sighand() without an RCU grace period really isn't buggy, courtesy of Oleg Nesterov.
Thanx, Paul
------------------------------------------------------------------------
b/kernel/fork.c | 5 ++++- b/kernel/signal.c | 42 +++++++++++++++++++++++++----------------- 2 files changed, 29 insertions(+), 18 deletions(-)
PATCH 1/2 signal: Exit RCU read-side critical section on each pass through loop
From: "Paul E. McKenney" <[email protected]>
The kill_pid_info() can potentially loop indefinitely if tasks are created and deleted sufficiently quickly, and if this happens, this function will remain in a single RCU read-side critical section indefinitely. This commit therefore exits the RCU read-side critical section on each pass through the loop. Because a race must happen to retry the loop, this should have no performance impact in the common case.
--- kernel/signal.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-)
kernel/signal.c +14 -16
@@ -1331,23 +1331,21 @@ int kill_pid_info(int sig, struct siginfo *info, struct pid *pid)int error = -ESRCH ;struct task_struct *p; -rcu_read_lock (); -retry: - p =pid_task (pid,PIDTYPE_PID ); -if (p) { - error =group_send_sig_info (sig, info, p); -if (unlikely (error == -ESRCH )) -/* -* The task was unhashed in between, try again. -* If it is dead, pid_task() will return NULL, -* if we race with de_thread() it will find the -* new leader. -*/ - goto retry; - } -rcu_read_unlock (); +for (;;) { +rcu_read_lock (); + p =pid_task (pid,PIDTYPE_PID ); +if (p) + error =group_send_sig_info (sig, info, p); +rcu_read_unlock (); +if (likely (!p || error != -ESRCH )) +return error; -return error; +/* +* The task was unhashed in between, try again. If it +* is dead, pid_task() will return NULL, if we race with +* de_thread() it will find the new leader. +*/ + } }int kill_proc_info (int sig,struct siginfo *info,pid_t pid)
Signature · 1 line
1.8.1.5
PATCH 2/2 signal: Document the RCU protection of ->sighand
From: Oleg Nesterov <[email protected]>
__cleanup_sighand() frees sighand without RCU grace period. This is correct but this looks "obviously buggy" and constantly confuses the readers, add the comments to explain how this works.
--- kernel/fork.c | 5 ++++- kernel/signal.c | 12 +++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-)
kernel/fork.c +4 -1
@@ -1022,11 +1022,14 @@ void __cleanup_sighand(struct sighand_struct *sighand) {if (atomic_dec_and_test (&sighand->count)) {signalfd_cleanup (sighand); +/* +* sighand_cachep is SLAB_DESTROY_BY_RCU so we can free it +* without an RCU grace period, see __lock_task_sighand(). +*/ kmem_cache_free (sighand_cachep, sighand); } } -/* * Initialize POSIX timer handling for a thread group. */
kernel/signal.c +11 -1
@@ -1275,7 +1275,17 @@ struct sighand_struct *__lock_task_sighand(struct task_struct *tsk,local_irq_restore (*flags);break ; } - +/* +* This sighand can be already freed and even reused, but +* we rely on SLAB_DESTROY_BY_RCU and sighand_ctor() which +* initializes ->siglock: this slab can't go away, it has +* the same object type, ->siglock can't be reinitialized. +* +* We need to ensure that tsk->sighand is still the same +* after we take the lock, we can race with de_thread() or +* __exit_signal(). In the latter case the next iteration +* must see ->sighand == NULL. +*/ spin_lock (&sighand->siglock);if (likely (sighand == tsk->sighand)) {rcu_read_unlock ();
Signature · 1 line
1.8.1.5
Quoted 10 lines · Paul E. McKenney
Hello!
This series updates RCU handling in signals:
1. Prevents a possible very long RCU read-side critical section
in kill_pid_info().2. Explain why the "obviously buggy" freeing of sighand by
__cleanup_sighand()without an RCU grace period really isn't buggy, courtesy of Oleg Nesterov.
Quoted 9 lines
Thanx, Paul
------------------------------------------------------------------------
b/kernel/fork.c | 5 ++++- b/kernel/signal.c | 42 +++++++++++++++++++++++++----------------- 2 files changed, 29 insertions(+), 18 deletions(-)
Signature · 1 line
Pranith