On Wed, May 27, 2015 at 10:41:37PM -0700, Nicholas A. Bellinger wrote: > On Wed, 2015-05-27 at 13:36 -0700, Paul E. McKenney wrote: > > On Tue, May 26, 2015 at 10:13:02PM -0700, Nicholas A. Bellinger wrote: > > > On Tue, 2015-05-26 at 14:44 +0200, Bart Van Assche wrote: > > > > On 05/26/15 08:57, Nicholas A. Bellinger wrote: > > > > > - Add various rcu_dereference and lockless_dereference RCU notation > > > > > > > > Hello Nic, > > > > > > > > Feedback from an RCU expert (which I'm not) would be appreciated here. > > > > But my understanding is that lockless_dereference(p) should be used for > > > > a pointer p that has *not* been annotated as an RCU pointer. I think in > > > > the for-next branch of the target repository that this macro is used to > > > > access RCU-annotated pointers. Is that why sparse complains about how > > > > lockless_dereference() is used in the target tree ? > > > > > > > > > > Was curious about this myself.. Thanks for raising the question! > > > > > > The intention of lockless_dereference() in both this and preceding > > > series is for __rcu protected pointers that are accessed outside of > > > rcu_read_lock() protection, and who's lifetime is controlled by a: > > > > > > - struct kref > > > - struct percpu_ref > > > - struct config_group symlink > > > - RCU updater path with some manner of mutex or spinlock held > > > > > > This is supposed to be following Paul's comment in rcupdate.h: > > > > > > * Similar to rcu_dereference(), but for situations where the pointed-to > > > * object's lifetime is managed by something other than RCU. That > > > * "something other" might be reference counting or simple immortality. > > > > > > Paul, would you be to kind to clarify the intention for us..? > > > > The lockless_dereference() primitive is to be used for pointers that > > are -not- marked with __rcu. In fact, the sparse tool should yell > > at you if you use lockless_dereference() on an __rcu-marked pointer. > > Yep, definitely wrong usage of lockless_dereference on my part. > > Thanks for the clarification. > > > You could use smp_store_release() to update the pointer when inserting > > new data. If you are using one of the lists, then the _rcu variant of the > > list-insert macro should be used (list_add_rcu()), because that is needed > > to make sure that the reader sees a properly initialized new element. > > > > If you have a pointer that is sometimes protected by RCU and other times > > protected by something else, you still use one of the rcu_dereference() > > macros to access it. For example, if a given RCU-protected pointer is > > protected either by RCU or by some lock, you might write common code > > that is called from either context as follows: > > > > p = rcu_dereference_check(pointer, lockdep_is_held(&some_lock)); > > > > Does that help, or am I missing your point? > > > > This makes more sense now. > > Ok, so for an updater path where a __rcu protected pointer is being > dereferenced with a lock held synchronizing modification of an > hlist_head or hlist_node, the rcu_dereference_check() usage is clear to > me.. > > What I'm still unclear about is other three cases above, where a __rcu > protected pointer is dereferenced outside of the updater path, but it's > release is protected by some external means; kref, percpu_ref, or a > configfs parent/child config_group reference. > > For example, say a __rcu protected pointer is dereferenced under > rcu_read_lock(). The data structure itself contains a percpu_ref that > is incremented under rcu_read_lock(), and also contains a rcu_head. > rcu_read_unlock() happens immediately after the percpu_ref has been > incremented. > > If this structure is then attempted to be released from an updater path, > it first blocks on a completion waiting for the percpu_ref count to > return to zero, before invoking the final kfree_rcu(). > > So the question I'm getting at is, what's the correct notation for > dereferencing a __rcu pointer outside of rcu_read_lock(), who's data > structure is protected by some manner of reference counting obtained > under rcu_read_lock(), that prevents kfree_rcu() from happening until > the reference count is dropped..? If feasible, use rcu_dereference_check() with an expression checking for the reference being held. If that does not work for whatever reason, use rcu_dereference_raw() with a comment indicating that you are relying on a kref, percpu_ref, or whatever. Thanx, Paul -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
