| Message ID | 152506269061.7246.13075216914692813995.stgit@noble (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 30, 2018 at 02:31:30PM +1000, NeilBrown wrote: > list_for_each_entry_from_rcu() is an RCU version of > list_for_each_entry_from(). It walks a linked list under rcu > protection, from a given start point. > > It is similar to list_for_each_entry_continue_rcu() but starts *at* > the given position rather than *after* it. > > Naturally, the start point must be known to be in the list. I'd suggest giving an explicit advisory comment to clarify and suggest correct usage: "This would typically require either that you obtained the node from a previous walk of the list in the same RCU read-side critical section, or that you held some sort of non-RCU reference (such as a reference count) to keep the node alive *and* in the list." (Feel free to wordsmith the exact wording, but something like that seems like it would help people understand how to use this correctly, and make it less likely that they'd use it incorrectly.) -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Apr 29, 2018 at 10:20:33PM -0700, Josh Triplett wrote: > On Mon, Apr 30, 2018 at 02:31:30PM +1000, NeilBrown wrote: > > list_for_each_entry_from_rcu() is an RCU version of > > list_for_each_entry_from(). It walks a linked list under rcu > > protection, from a given start point. > > > > It is similar to list_for_each_entry_continue_rcu() but starts *at* > > the given position rather than *after* it. > > > > Naturally, the start point must be known to be in the list. > > I'd suggest giving an explicit advisory comment to clarify and suggest > correct usage: > > "This would typically require either that you obtained the node from a > previous walk of the list in the same RCU read-side critical section, or > that you held some sort of non-RCU reference (such as a reference count) > to keep the node alive *and* in the list." > > (Feel free to wordsmith the exact wording, but something like that seems > like it would help people understand how to use this correctly, and make > it less likely that they'd use it incorrectly.) What Josh said! Could you also contrast this with the existing list_for_each_entry_continue_rcu() macro in the header comment as well as in the commit log? Thanx, Paul -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 30 Apr 2018 06:43:08 -0700 "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote: > On Sun, Apr 29, 2018 at 10:20:33PM -0700, Josh Triplett wrote: > > On Mon, Apr 30, 2018 at 02:31:30PM +1000, NeilBrown wrote: > > > list_for_each_entry_from_rcu() is an RCU version of > > > list_for_each_entry_from(). It walks a linked list under rcu > > > protection, from a given start point. > > > > > > It is similar to list_for_each_entry_continue_rcu() but starts *at* > > > the given position rather than *after* it. > > > > > > Naturally, the start point must be known to be in the list. > > > > I'd suggest giving an explicit advisory comment to clarify and suggest > > correct usage: > > > > "This would typically require either that you obtained the node from a > > previous walk of the list in the same RCU read-side critical section, or > > that you held some sort of non-RCU reference (such as a reference count) > > to keep the node alive *and* in the list." > > > > (Feel free to wordsmith the exact wording, but something like that seems > > like it would help people understand how to use this correctly, and make > > it less likely that they'd use it incorrectly.) > > What Josh said! Could you also contrast this with the existing > list_for_each_entry_continue_rcu() macro in the header comment as well > as in the commit log? > Should Documentation/RCU/whatisRCU.txt be updated? Specifically section: 7. FULL LIST OF RCU APIs -- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 30, 2018 at 11:14:54AM -0400, Steven Rostedt wrote: > On Mon, 30 Apr 2018 06:43:08 -0700 > "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote: > > > On Sun, Apr 29, 2018 at 10:20:33PM -0700, Josh Triplett wrote: > > > On Mon, Apr 30, 2018 at 02:31:30PM +1000, NeilBrown wrote: > > > > list_for_each_entry_from_rcu() is an RCU version of > > > > list_for_each_entry_from(). It walks a linked list under rcu > > > > protection, from a given start point. > > > > > > > > It is similar to list_for_each_entry_continue_rcu() but starts *at* > > > > the given position rather than *after* it. > > > > > > > > Naturally, the start point must be known to be in the list. > > > > > > I'd suggest giving an explicit advisory comment to clarify and suggest > > > correct usage: > > > > > > "This would typically require either that you obtained the node from a > > > previous walk of the list in the same RCU read-side critical section, or > > > that you held some sort of non-RCU reference (such as a reference count) > > > to keep the node alive *and* in the list." > > > > > > (Feel free to wordsmith the exact wording, but something like that seems > > > like it would help people understand how to use this correctly, and make > > > it less likely that they'd use it incorrectly.) > > > > What Josh said! Could you also contrast this with the existing > > list_for_each_entry_continue_rcu() macro in the header comment as well > > as in the commit log? > > Should Documentation/RCU/whatisRCU.txt be updated? > > Specifically section: 7. FULL LIST OF RCU APIs Very much so, thank you for catching this! Thanx, Paul -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/rculist.h b/include/linux/rculist.h index 127f534fec94..36df6ccbc874 100644 --- a/include/linux/rculist.h +++ b/include/linux/rculist.h @@ -403,6 +403,19 @@ static inline void list_splice_tail_init_rcu(struct list_head *list, &pos->member != (head); \ pos = list_entry_rcu(pos->member.next, typeof(*pos), member)) +/** + * list_for_each_entry_from_rcu - iterate over a list from current point + * @pos: the type * to use as a loop cursor. + * @head: the head for your list. + * @member: the name of the list_node within the struct. + * + * Iterate over the tail of a list starting from a given position, + * which must have been in the list when the RCU read lock was taken. + */ +#define list_for_each_entry_from_rcu(pos, head, member) \ + for (; &(pos)->member != (head); \ + pos = list_entry_rcu(pos->member.next, typeof(*(pos)), member)) + /** * hlist_del_rcu - deletes entry from hash list without re-initialization * @n: the element to delete from the hash list.
list_for_each_entry_from_rcu() is an RCU version of list_for_each_entry_from(). It walks a linked list under rcu protection, from a given start point. It is similar to list_for_each_entry_continue_rcu() but starts *at* the given position rather than *after* it. Naturally, the start point must be known to be in the list. Signed-off-by: NeilBrown <neilb@suse.com> --- include/linux/rculist.h | 13 +++++++++++++ 1 file changed, 13 insertions(+) -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html