Re: [PATCH 1/1] release_task: kill unnecessary rcu_read_lock() around dec_rlimit_ucounts()

[David Howells <dhowells@redhat.com>]

Oleg Nesterov <oleg@redhat.com> wrote:

> rcu_read_lock() was added to shut RCU-lockdep up when this code used
> __task_cred()->rcu_dereference(), but after the commit 21d1c5e386bc
> ("Reimplement RLIMIT_NPROC on top of ucounts") it is no longer needed:
> task_ucounts()->task_cred_xxx() takes rcu_read_lock() itself.
> 
> NOTE: task_ucounts() returns the pointer to another rcu-protected data,
> struct ucounts. So it should either be used when task->real_cred and thus
> task->real_cred->ucounts is stable (release_task, copy_process, copy_creds),
> or it should be called under rcu_read_lock(). In both cases it is pointless
> to take rcu_read_lock() to read the cred->ucounts pointer.

Yeah, accessing the pointer that task_ucounts() gives you isn't RCU safe
unless you're holding the rcu_read_lock() or are in a context where RCU safety
is irrelevant.  The task doing the dismantling in release_task() would seem to
qualify for that.

David