lkml.org 
[lkml]   [2011]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 1/4] comm: Introduce comm_lock spinlock to protect task->comm access
    From
    Date
    On Wed, 2011-05-18 at 11:01 +0900, KOSAKI Motohiro wrote:
    > > diff --git a/fs/exec.c b/fs/exec.c
    > > index 5e62d26..34fa611 100644
    > > --- a/fs/exec.c
    > > +++ b/fs/exec.c
    > > @@ -998,17 +998,28 @@ static void flush_old_files(struct files_struct * files)
    > >
    > > char *get_task_comm(char *buf, struct task_struct *tsk)
    > > {
    > > - /* buf must be at least sizeof(tsk->comm) in size */
    > > - task_lock(tsk);
    > > + unsigned long flags;
    > > +
    > > + spin_lock_irqsave(&tsk->comm_lock, flags);
    > > strncpy(buf, tsk->comm, sizeof(tsk->comm));
    > > - task_unlock(tsk);
    > > + spin_unlock_irqrestore(&tsk->comm_lock, flags);
    > > return buf;
    > > }
    > >
    > > void set_task_comm(struct task_struct *tsk, char *buf)
    > > {
    > > + unsigned long flags;
    > > +
    > > + /*
    > > + * XXX - Even though comm is protected by comm_lock,
    > > + * we take the task_lock here to serialize against
    > > + * current users that directly access comm.
    > > + * Once those users are removed, we can drop the
    > > + * task locking& memsetting.
    > > + */
    >
    > If we provide __get_task_comm(), we can't remove memset() forever.

    True enough. I'll fix that comment up then.

    >
    > > task_lock(tsk);
    > > + spin_lock_irqsave(&tsk->comm_lock, flags);
    >
    > This is strange order. task_lock() doesn't disable interrupt.

    Strange order? Can you explain why you think that is? Having comm_lock
    as an inner-most lock seems quite reasonable, given the limited nature
    of what it protects.

    > And, can you please document why we need interrupt disabling?

    Since we might access current->comm from irq context. Where would you
    like this documented? Just there in the code?

    thanks
    -john




    \
     
     \ /
      Last update: 2011-05-18 06:13    [W:0.022 / U:0.040 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site