[PATCH 1/4] utrace: kill mb() in tracehook_report_death()
As Peter pointed out, this barrier is not needed. utrace_set_events()
and tracehook_report_death() can rely on tasklist_lock.
utrace_set_events() checks -exit_state == 0 and adds DEATH_EVENTS
under tasklist_lock. After exit_notify() sets -exit_state under
write_lock(tasklist) we must see the change in flags.
utrace_set_events() does not set _UTRACE_DEATH_EVENTS if -exit_state
was already set by exit_notify(), and after we set -exit_state under
write_lock(tasklist) we must see the change in -utrace_flags.
Signed-off-by: Oleg Nesterov o...@redhat.com
---
include/linux/tracehook.h |9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
--- UTRACE-PTRACE/include/linux/tracehook.h~1_REPORT_DEATH_KILL_MB
2009-11-18 21:16:23.0 +0100
+++ UTRACE-PTRACE/include/linux/tracehook.h 2009-12-12 16:32:39.0
+0100
@@ -626,17 +626,12 @@ static inline void tracehook_report_deat
int group_dead)
{
/*
-* This barrier ensures that our caller's setting of
-* @task-exit_state precedes checking @task-utrace_flags here.
* If utrace_set_events() was just called to enable
* UTRACE_EVENT(DEATH), then we are obliged to call
* utrace_report_death() and not miss it. utrace_set_events()
-* uses tasklist_lock to synchronize enabling the bit with the
-* actual change to @task-exit_state, but we need this barrier
-* to be sure we see a flags change made just before our caller
-* took the tasklist_lock.
+* checks @task-exit_state under tasklist_lock to synchronize
+* with exit_notify(), the caller.
*/
- smp_mb();
if (task_utrace_flags(task) _UTRACE_DEATH_EVENTS)
utrace_report_death(task, death_cookie, group_dead, signal);
}
[PATCH 2/4] utrace: fix the -cloning check in utrace_attach_delay()
Due to typo, utrace_attach_delay() always succeeds if the caller
is not traced. The creator must have the valid -utrace != NULL
and utrace-cloning should be target.
Signed-off-by: Oleg Nesterov o...@redhat.com
---
kernel/utrace.c | 20
1 file changed, 12 insertions(+), 8 deletions(-)
--- UTRACE-PTRACE/kernel/utrace.c~2_ATTACH_DELAY_TYPO 2009-12-05
16:02:50.0 +0100
+++ UTRACE-PTRACE/kernel/utrace.c 2009-12-12 16:49:26.0 +0100
@@ -184,14 +184,18 @@ static struct utrace_engine *matching_en
*/
static inline int utrace_attach_delay(struct task_struct *target)
{
- if ((target-flags PF_STARTING)
- task_utrace_struct(current)
- task_utrace_struct(current)-cloning != target)
- do {
- schedule_timeout_interruptible(1);
- if (signal_pending(current))
- return -ERESTARTNOINTR;
- } while (target-flags PF_STARTING);
+ if (!unlikely(target-flags PF_STARTING))
+ return 0;
+
+ if (task_utrace_struct(current)
+ task_utrace_struct(current)-cloning == target)
+ return 0;
+
+ do {
+ schedule_timeout_interruptible(1);
+ if (signal_pending(current))
+ return -ERESTARTNOINTR;
+ } while (target-flags PF_STARTING);
return 0;
}
[PATCH 3/4] utrace: improve the comment in tracehook_notify_resume()
My attempt to make the comment more explicit.
Signed-off-by: Oleg Nesterov o...@redhat.com
---
include/linux/tracehook.h |7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
--- UTRACE-PTRACE/include/linux/tracehook.h~3_NOTIFY_RESUME_MB_COMMENT
2009-12-12 16:32:39.0 +0100
+++ UTRACE-PTRACE/include/linux/tracehook.h 2009-12-12 17:28:03.0
+0100
@@ -670,9 +670,10 @@ static inline void tracehook_notify_resu
{
struct task_struct *task = current;
/*
-* This pairs with the barrier implicit in set_notify_resume().
-* It ensures that we read the nonzero utrace_flags set before
-* set_notify_resume() was called by utrace setup.
+* If we race with attach which sets nonzero -utrace_flags,
+* make sure we do not read -utrace_flags before the caller
+* clears TIF_NOTIFY_RESUME. This pairs with the implicit mb()
+* before setting TIF_NOTIFY_RESUME in set_notify_resume().
*/
smp_rmb();
if (task_utrace_flags(task))
[PATCH 4/4] utrace: fix the comments about rmb() in task_utrace_struct()
Move the comment from utrace_task_alloc() to utrace_add_engine()
and try to make it a bit more clear.
Signed-off-by: Oleg Nesterov o...@redhat.com
---
include/linux/utrace.h |4 ++--
kernel/utrace.c| 11 +--
2 files changed, 7 insertions(+), 8 deletions(-)
--- UTRACE-PTRACE/include/linux/utrace.h~4_ADD_ENGINE_DOCUMENT_IMPLICIT_MB
2009-11-21 15:12:27.0 +0100
+++ UTRACE-PTRACE/include/linux/utrace.h2009-12-12 17:42:16.0
+0100
@@ -164,8 +164,8 @@ static inline struct utrace *task_utrace
* is ordered before this load of task-utrace. We use those
* utrace_flags checks in the hot path to decide to call into
* the utrace code. The first attach installs task-utrace before
-* setting task-utrace_flags nonzero, with a barrier between.
-* See utrace_task_alloc().
+* setting task-utrace_flags nonzero with implicit barrier in
+* between, see utrace_add_engine().
*/
smp_rmb();
utrace = task-utrace;
--- UTRACE-PTRACE/kernel/utrace.c~4_ADD_ENGINE_DOCUMENT_IMPLICIT_MB
2009-12-12 16:49:26.0 +0100
+++ UTRACE-PTRACE/kernel/utrace.c 2009-12-12 17:46:55.0 +0100
@@ -107,11 +107,7 @@ static bool utrace_task_alloc(struct tas
task-utrace = utrace;
}
task_unlock(task);
- /*
-* That unlock after storing task-utrace acts as a memory barrier
-* ordering any subsequent task-utrace_flags store afterwards.
-* This pairs with smp_rmb() in task_utrace_struct().
-*/
+
if (unlikely(task-utrace != utrace))
kmem_cache_free(utrace_cachep, utrace);
return true;
@@ -221,7 +217,10 @@ static int utrace_add_engine(struct task
/*
* In case we had no engines before, make sure that
-* utrace_flags is not zero.
+* utrace_flags is not zero. Since we did unlock+lock
+* at least once after utrace_task_alloc() installed
+* -utrace, we have the necessary barrier which pairs
+* with rmb() in task_utrace_struct().
*/
ret = -ESRCH;
if (!target-utrace_flags) {
(no subject)
inline: polygamize.jpg
