"Paul E. McKenney" <paul...@kernel.org> writes: > So this looks good in and of itself, but I still do not see what prevents > the unfortunate sequence of events called out in my previous email. > On the other hand, if ->rcu and ->rcu_users were not allocated on top > of each other by a union, I would be happy to provide a Reviewed-by. > > And I am fundamentally distrusting of a refcount_dec_and_test() that > is immediately followed by code that clobbers the now-zero value. > Yes, this does have valid use cases, but it has a lot more invalid > use cases. The valid use cases have excluded all increments somehow > else, so that the refcount_dec_and_test() call's only job is to > synchronize between concurrent calls to put_task_struct_rcu_user(). > But I am not seeing the "excluded all increments somehow". > > So, what am I missing here?
Probably only that the users of the task_struct in this sense are now quite mature. The two data structures that allow rcu access to the task_struct are the pid hash and the runqueue. The practical problem is that they have two very different lifetimes. So we need some kind of logic that let's us know when they are both done. A recount does that job very well. Placing the recount on the same storage as the unused (at that point) rcu_head removes the need to be clever in other ways to avoid bloating the task_struct. If you really want a reference to the task_struct from rcu context you can just use get_task_struct. Because until the grace period completes it is guaranteed that the task_struct has a positive count. Right now I can't imagine a use case for wanting to increase rcu_users anywhere or to decrease rcu_users except where we do. If there is such a case most likely it will increase the reference count at initialization time. If anyone validly wants to increment rcu_users from an rcu critical section we can move it out of the union at that time. Eric