From 9cb8092cf84d18e7435f03eef3963e01ea4e993f Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Wed, 9 Jun 2010 11:50:48 +0200 Subject: [PATCH] Loop on utrace_barrier if utrace_control UTRACE_DETACH returns -EINPROGRESS. When utrace_control(tsk, eng, UTRACE_DETACH) returns -EINPROGRESS that means there are still handlers running. So loop on utrace_barrier(tsk, eng) in that case, till it no longer returns -ERESTARTSYS. That makes sure that no engine handler will be called afterwards, so we can safely unload the stap module. Not doing this might have caused PR11672 (utrace_report_syscall_exit crash), although we don't yet have a simple reproducer for that issue. * runtime/itrace.c (remove_usr_itrace_info): Loop on utrace_barrier if utrace_control returned -EINPROGRESS. * runtime/task_finder.c (stap_utrace_detach): Likewise. (stap_utrace_detach_ops): Likewise. And warn if stap_utrace_detach didn't return successfully. (__stp_utrace_attach): Loop on -ERESTARTSYS after utrace_barrier. (__stp_utrace_task_finder_target_quiesce): Likewise. --- runtime/itrace.c | 5 +++++ runtime/task_finder.c | 29 +++++++++++++++++++---------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/runtime/itrace.c b/runtime/itrace.c index 399bfde02..5f701b994 100644 --- a/runtime/itrace.c +++ b/runtime/itrace.c @@ -356,6 +356,11 @@ void static remove_usr_itrace_info(struct itrace_info *ui) if (ui->tsk && ui->engine) { status = utrace_control(ui->tsk, ui->engine, UTRACE_DETACH); + if (status == -EINPROGRESS) { + do { + status = utrace_barrier(ui->tsk, ui->engine); + } while (status == -ERESTARTSYS); + } if (status < 0 && status != -ESRCH && status != -EALREADY) printk(KERN_ERR "utrace_control(UTRACE_DETACH) returns %d\n", diff --git a/runtime/task_finder.c b/runtime/task_finder.c index 538e07b69..e4390d23a 100644 --- a/runtime/task_finder.c +++ b/runtime/task_finder.c @@ -307,8 +307,10 @@ stap_utrace_detach(struct task_struct *tsk, rc = 0; /* ignore these errors */ break; case -EINPROGRESS: + do { + rc = utrace_barrier(tsk, engine); + } while (rc == -ERESTARTSYS); debug_task_finder_detach(); - rc = 0; break; default: rc = -rc; @@ -354,8 +356,9 @@ stap_utrace_detach_ops(struct utrace_engine_ops *ops) /* Notice we're purposefully ignoring errors from * stap_utrace_detach(). Even if we got an error on * this task, we need to keep detaching from other - * tasks. */ - (void) stap_utrace_detach(tsk, ops); + * tasks. But warn, we might be unloading and dangling + * engines are bad news. */ + WARN_ON (stap_utrace_detach(tsk, ops) != 0); } while_each_thread(grp, tsk); rcu_read_unlock(); debug_task_finder_report(); @@ -504,7 +507,9 @@ __stp_utrace_attach(struct task_struct *tsk, * stale task pointer, if we have an engine * ref. */ - rc = utrace_barrier(tsk, engine); + do { + rc = utrace_barrier(tsk, engine); + } while (rc == -ERESTARTSYS); if (rc != 0 && rc != -ESRCH && rc != -EALREADY) _stp_error("utrace_barrier returned error %d on pid %d", rc, (int)tsk->pid); @@ -514,13 +519,15 @@ __stp_utrace_attach(struct task_struct *tsk, if (action != UTRACE_RESUME) { rc = utrace_control(tsk, engine, UTRACE_STOP); - /* EINPROGRESS means we must wait for - * a callback, which is what we want. */ - if (rc != 0 && rc != -EINPROGRESS) + if (rc == -EINPROGRESS) + /* EINPROGRESS means we must wait for + * a callback, which is what we want. */ + do { + rc = utrace_barrier(tsk, engine); + } while (rc == -ERESTARTSYS); + if (rc != 0) _stp_error("utrace_control returned error %d on pid %d", rc, (int)tsk->pid); - else - rc = 0; } } @@ -1245,7 +1252,9 @@ __stp_utrace_task_finder_target_quiesce(enum utrace_resume_action action, * safe to call utrace_barrier() even with * a stale task pointer, if we have an engine ref. */ - rc = utrace_barrier(tsk, engine); + do { + rc = utrace_barrier(tsk, engine); + } while (rc == -ERESTARTSYS); if (rc == 0) rc = utrace_set_events(tsk, engine, __STP_ATTACHED_TASK_BASE_EVENTS(tgt)); -- 2.43.5