[PATCH] Fix spin_unlock order in utrace_stop

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli [EMAIL PROTECTED]

utrace_stop() seems to get the spin_unlock sequence inverted in one of the
unlikely branches. Fix it.

Signed-off-by: Ananth N Mavinakayanahalli [EMAIL PROTECTED]
---
 kernel/utrace.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: utrace-2sep/kernel/utrace.c
===
--- utrace-2sep.orig/kernel/utrace.c
+++ utrace-2sep/kernel/utrace.c
@@ -482,8 +482,8 @@ static bool utrace_stop(struct task_stru
spin_lock_irq(task-sighand-siglock);
 
if (unlikely(sigismember(task-pending.signal, SIGKILL))) {
-   spin_unlock(utrace-lock);
spin_unlock_irq(task-sighand-siglock);
+   spin_unlock(utrace-lock);
return true;
}
 

utrace_set_events in quiesce handler?

[David Smith]

In systemtap, we've changed to stopping a thread before setting up the
events we're interested in (besides quiesce/death).  So, basically, it
looks like this (without much error handling):

// initial attach logic
// ... find an interesting thread ...
ops.report_quiesce = quiesce_handler;
ops.report_syscall_entry = syscall_entry_handler;

engine = utrace_attach_task(tsk, UTRACE_ATTACH_CREATE, ops, data);
rc = utrace_set_events(tsk, engine,
(UTRACE_EVENT(DEATH) | UTRACE_STOP | UTRACE_EVENT(QUIESCE)));

// ... do other stuff ...


// quiesce handler
u32
quiesce_handler(enum utrace_resume_action action,
struct utrace_attached_engine *engine,
struct task_struct *tsk,
unsigned long event)
{
int rc;

// Turn off quiesce handling and turn on syscall handling
rc = utrace_set_events(tsk, engine,
UTRACE_EVENT(DEATH) | UTRACE_EVENT(SYSCALL_ENTRY));
if (rc == -EINPROGRESS) {
rc = utrace_barrier(tsk, engine);
if (rc != 0)
printk(KERN_ERR
utrace_barrier returned error %d on pid %d,
rc, (int)tsk-pid);
rc = utrace_set_events(tsk, engine,
UTRACE_EVENT(DEATH) | UTRACE_EVENT(SYSCALL_ENTRY));
}
if (rc != 0)
printk(KERN_ERR
utrace_set_events returned error %d on pid %d,
rc, (int)tsk-pid);

// ... do other stuff ...
return UTRACE_RESUME;
}

The utrace_barrier() call always returns 0, but the utrace_set_events()
calls always return -EINPRPOGRESS.  I've put the -EINPROGRESS logic in a
loop, but even after 10 iterations utrace_set_events() never succeeds.

This is on 2.6.27-0.287.rc4.git7.fc10.x86_64.

So, am I doing this incorrectly?  Or is there a bug here in utrace
(where it doesn't expect to see a utrace_set_events() from within a
handler on the same thread)?  If I'm doing this incorrectly, I'd like
help in figuring out what I should be doing.

(In the original utrace there was UTRACE_ACTION_NEWSTATE which allowed
you to change the flags from the handler, but I haven't seen anything
similar in the current utrace.)

Thanks for the help.

-- 
David Smith
[EMAIL PROTECTED]
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)

Re: [PATCH] Fix spin_unlock order in utrace_stop

[Roland McGrath]

Thanks!  I don't think that ever hurts anything, but it's definitely right
to fix it.  I've put the change in and updated the patches.


Thanks,
Roland