lkml.org 
[lkml]   [2006]   [Jan]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [rfc][patch] Avoid taking global tasklist_lock for single threadedprocess at getrusage()
    On Sun, Jan 08, 2006 at 02:49:31PM +0300, Oleg Nesterov wrote:
    > Sorry for delay,
    >
    > Ravikiran G Thirumalai wrote:
    > >
    > > static void k_getrusage(struct task_struct *p, int who, struct rusage *r)
    > > @@ -1681,14 +1697,22 @@ static void k_getrusage(struct task_stru
    > > struct task_struct *t;
    > > unsigned long flags;
    > > cputime_t utime, stime;
    > > + int need_lock = 0;
    >
    > Unneeded initialization

    akpm changed the condition statement below with an if test. So it is needed now.

    >
    > > memset((char *) r, 0, sizeof *r);
    > > -
    > > - if (unlikely(!p->signal))
    > > - return;
    > > -
    > > utime = stime = cputime_zero;
    > >
    > > + need_lock = !(p == current && thread_group_empty(p));
    > > + if (need_lock) {
    > > + read_lock(&tasklist_lock);
    > > + if (unlikely(!p->signal)) {
    > > + read_unlock(&tasklist_lock);
    > > + return;
    > > + }
    > > + } else
    > > + /* See locking comments above */
    > > + smp_rmb();
    >
    > This patch doesn't try to optimize ->sighand.siglock locking,
    > and I think this is right. But this also means we don't need
    > rmb() here. It was needed to protect against "another thread
    > just exited, cpu can read ->c* values before thread_group_empty()
    > without taking siglock" case, now it is not possible.

    Don't we still need rmb for the RUSAGE_SELF case? we do not take the
    siglock for rusage self and the non c* signal fields are written to
    at __exit_signal...

    What is wrong with optimizing by not taking the siglock in RUSAGE_BOTH
    and RUSAGE_CHILDREN? I would like to add that in too unless I am
    missing something and the optimization is incorrect.

    Thanks,
    Kiran
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-01-08 21:01    [W:0.022 / U:0.840 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site