lkml.org 
[lkml]   [2010]   [Nov]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: rcu_read_lock/unlock protect find_task_by_vpid call in posix_cpu_timer_create
    (add Sergey)

    On 11/25, Peter Zijlstra wrote:
    >
    > On Wed, 2010-11-24 at 20:09 -0500, Dave Jones wrote:
    > > --- a/kernel/posix-cpu-timers.c
    > > +++ b/kernel/posix-cpu-timers.c
    > > @@ -391,6 +391,7 @@ int posix_cpu_timer_create(struct k_itimer *new_timer)
    > > INIT_LIST_HEAD(&new_timer->it.cpu.entry);
    > >
    > > read_lock(&tasklist_lock);
    > > + rcu_read_lock();
    > > if (CPUCLOCK_PERTHREAD(new_timer->it_clock)) {
    > > if (pid == 0) {
    > > p = current;
    > > @@ -414,6 +415,7 @@ int posix_cpu_timer_create(struct k_itimer *new_timer)
    > > } else {
    > > ret = -EINVAL;
    > > }
    > > + rcu_read_unlock();
    > > read_unlock(&tasklist_lock);
    > >
    > > return ret;
    >
    > Do we still need the tasklist_lock in this case?

    No. posix-cpu-timer.c shouldn't use tasklist at all. But it is not
    completely trivial to remove it.

    In particular, this patch is not exactly right, we can't trust
    thread_group_leader() without tasklist.

    Sergey already sent the patch which removes tasklist from
    posix_cpu_timer_create() and posix_cpu_timer_create(), and iirc
    Thomas queued it.

    > Also, why is that think complaining, surely the tasklist_lock pins any
    > and all PID objects?

    The only problem is: if copy_process() fails, it does free_pid()
    lockless. This means, without rcu lock it is not safe to scan the
    rcu-protected lists.

    We can change copy_process() (in fact I sent the patch several
    years ago), but everybody think that find_pid/etc should always
    take rcu_read_lock() instead. I tend to agree.

    Oleg.



    \
     
     \ /
      Last update: 2010-11-25 12:09    [W:0.023 / U:0.056 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site