Messages in this thread |  | | | Date | Wed, 18 May 2011 14:06:49 +0900 | | From | KOSAKI Motohiro <> | | Subject | Re: [PATCH 1/4] comm: Introduce comm_lock spinlock to protect task->comm access |
| |
>> 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.
spinlock -> irq_disable is wrong order.
local_irq_save() task_lock() spin_lock(task->comm)
is better. I think.
I mean if the task get interrupt at following point,
task_lock(tsk); // HERE spin_lock_irqsave(&tsk->comm_lock, flags); the task hold task-lock long time rather than expected.
>> 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?
I'm prefer code comment. but another way is also good.
Thanks.
|  |