lkml.org 
[lkml]   [2009]   [Dec]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC,PATCH 14/14] utrace core
    On 12/08, Peter Zijlstra wrote:
    >
    > On Tue, 2009-12-08 at 16:04 +0100, Oleg Nesterov wrote:
    > >
    > > Well, this is subjective, but I don't agree that
    > >
    > > get_task_struct(task);
    > > task->utrace_flags = flags;
    > > spin_unlock(&utrace->lock);
    > > put_task_struct(task);
    > >
    > > looks better.
    >
    > No, what I mean by assymetric locking is that utrace_reset() and
    > utrace_reap() drop the utrace->lock where their caller acquired it,
    > resulting in non-obvious like:
    >
    > utrace_control()
    > {
    >
    > ...
    > spin_lock(&utrace->lock);
    >
    > ...
    >
    > if (reset)
    > utrace_reset(utrace);
    > else
    > spin_unlock(&utrace->lock);
    > }

    Agreed, the code like this never looks good.

    > If you take a task ref you can write the much saner:
    >
    > utrace_control()
    > {
    > ...
    > spin_lock(&utrace->lock);
    > ...
    > if (reset)
    > utrace_reset(utrace);
    >
    > spin_unlock(&utrace->lock);
    > }

    No, get_task_struct() in utrace_reset() can't help, we should move
    it into utrace_control() then. And in this case it becomes even more
    subtle: it is needed because ->utrace_flags may be cleared inside
    utrace_reset() and after that utrace_control()->spin_unlock() becomes
    unsafe.

    Also. utrace_reset() drops utrace->lock to call put_detached_list()
    lockless. If we want to avoid the assymetric locking, every caller
    should pass "struct list_head *detached" to utrace_reset(), drop
    utrace->lock, and call put_detached_list().

    Oleg.



    \
     
     \ /
      Last update: 2009-12-08 17:39    [W:0.025 / U:1.060 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site