linux-kernel - RCU condition checks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <26884.1270658130@redhat.com>
Date:	Wed, 07 Apr 2010 17:35:30 +0100
From:	David Howells <dhowells@...hat.com>
To:	paulmck@...ux.vnet.ibm.com
Cc:	dhowells@...hat.com, Eric Dumazet <eric.dumazet@...il.com>,
	Trond.Myklebust@...app.com, linux-nfs@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: RCU condition checks

Paul E. McKenney <paulmck@...ux.vnet.ibm.com> wrote:

> > Why is there a need for 'c'?
> 
> An example use is where rcu_access_pointer() is legal because we are
> either initializing or cleaning up, so that no other CPU has access
> to the pointer.  In these cases, you might do something like:
> 
> 	q = rcu_access_pointer(p->a, p->refcnt == 0);

I think the main problem I have with this is that the fact that p->refcnt
should be 0 here is unrelated to the fact that we're wanting to look at the
value of p->a.  I'd say that this should be two separate statements, for
example:

	ASSERT(p->refcnt == 0);
	q = rcu_access_pointer(p->a);

I could see using a lockdep-managed ASSERT here would work, though.

The other problem I have with this is that I'm assuming rcu_access_pointer()
is simply for looking at the value of the pointer without dereferencing it -
in which case, is there any need for the lock-describing condition?

I agree, though, that:

	q = rcu_dereference_check(p->a,
				  rcu_read_lock_held() || (
				   lockdep_is_held(p->lock) &&
				   lockdep_is_held(q->lock)));

is a reasonable way of keeping the dereference and the lock checks together,
though that could equally well be written, say:

	LOCKDEP_ASSERT(rcu_read_lock_held() || (
		        lockdep_is_held(p->lock) &&
			lockdep_is_held(q->lock)));
	q = rcu_dereference_protected(p->a);

but combining those makes it easier to ensure people to write lock checking.

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