Re: [PATCH 2/2] mm/linux-next: Fix rcu locking in vm_is_stack

From: Siddhesh Poyarekar
Date: Thu Mar 08 2012 - 13:35:58 EST


On Thu, Mar 8, 2012 at 11:10 PM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> Not in this case. see __unhash_process(p)->list_del_rcu(p->thread_group).
>
> You missed the fact that ->thread_group differs from the "usual" rcu
> protected list. The _head_ of the list can be list_del_rcu'd. Not the
> first/last/any entry, even the head.
>
> Or IOW, we do not really have the head. Every task is the list entry,
> but it also can be be used as a head by while_each_thread().

Ahh ok, I did not notice this. That's an interesting quirk. The more I
think I understand rcu the more I realize there are gaps in my
understanding.

>> In that case too, before this
>> happens, the proc entry is removed
>
> I guess you meant proc_flush_task()... Not sure I really understand,
> it can't "remove" the opened entry. This is just optimization which
> tries to shrink the cache.

Yes, and I was obviously wrong, now that I read the whole thing again,
including the unmounting of the namespace. I misread the unmounting of
proc as being an unmount of the thread/thread group namespace (the nr
== 1 check).

> Yes, yes, yes, but this "next element" can exit too before you take
> rcu_read_lock, and in this case the deleted entry won't be updated.
> That is the problem.

I will post the entire, consolidated patch once again next week with
changes for this as well as some other things (*not* marking the
process stack with the TID to maintain backward compatibility and some
code cleanup). Thanks for not giving up trying to explain the same
thing over and over again ;)


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