On Wed, Sep 04, 2019 at 05:32:46PM +0200, Oleg Nesterov wrote:
> On 09/04, Frederic Weisbecker wrote:
> >
> > So what happens if, say:
> >
> >
> > CPU 1 CPU 2
> > --------------------------------------------------------------
> > rcu_read_lock()
> > p = rcu_dereference(rq->task)
> > if (refcount_inc_not_zero(p->rcu_users)) {
> > .....
> > release_task() {
> > put_task_struct_rcu_user() {
> > call_rcu() {
> > queue rcu_head
>
> in this particular case call_rcu() won't be called, so
Yeah I confused myself in finding the scenario but like you say below,
release_task()
can simply happen between the p = rcu_dereference() and the refcount_inc to
break things.
I thought the point of these rcu_users was to be able to do:
rcu_read_lock()
p = rcu_dereference(task)
if (!refcount_inc_not_zero(p->rcu_users)) {
rcu_read_unlock();
return -1;
}
rcu_read_unlock();
//do stuff with task
put_task_struct_rcu_user(p);
Thanks.
>
> > }
> > }
> > }
> > put_task_struct_rcu_user(); //here rcu_users has been overwritten
>
> rcu_users won't be overwritten.
>
> But nobody should try to increment ->rcu_users,
>
> rcu_read_lock();
> p = rcu_dereference(rq->task);
> refcount_inc_not_zero(p->rcu_users);
>
> is already wrong because both release_task/last-schedule can happen in
> between, before refcount_inc_not_zero().
>
> Oleg.
>
