Messages in this thread | | | Date | Tue, 9 Nov 2010 12:29:00 -0800 | From | "Paul E. McKenney" <> | Subject | Re: INFO: suspicious rcu_dereference_check() usage - kernel/pid.c:419 invoked rcu_dereference_check() without protection! |
| |
On Mon, Nov 08, 2010 at 04:15:09PM +0100, Oleg Nesterov wrote: > On 11/07, Paul E. McKenney wrote: > > > > On Tue, Oct 12, 2010 at 12:08:46AM -0700, Greg Thelen wrote: > > > > > > ioprio_set() contains a comment warning against of usage of > > > rcu_read_lock() to avoid this warning: > > > /* > > > * We want IOPRIO_WHO_PGRP/IOPRIO_WHO_USER to be "atomic", > > > * so we can't use rcu_read_lock(). See re-copy of ->ioprio > > > * in copy_process(). > > > */ > > > > > > So I'm not sure what the best fix is. > > (please note that "we can't use rcu_read_lock()" actually meant > rcu_read_lock() is not _enough_) > > > I must defer to Oleg, who wrote the comment. But please see below. > > I added this comment to explain some oddities in copy_process(). > Nobody confirmed my understanding was correct ;) > > In any case, this comment doesn't look right today. This code was > changed by fd0928df98b9578be8a786ac0cb78a47a5e17a20 > "ioprio: move io priority from task_struct to io_context" after that, > tasklist can't help to make sys_ioprio_set(IOPRIO_WHO_PGRP) atomic. > > I think tasklist_lock can be removed now. > > And, as Paul pointed out, we need rcu_read_lock() anyway, it was > already added by Sergey.
Thank you, Oleg! Greg, would you be willing to update your patch to remove the comment? (Perhaps tasklist_lock as well...)
Thanx, Paul
| |