radix_tree_lookup_slot() comment

PATCH Peter Zijlstra 2 participants 2 messages Sep 23, 2006 – Sep 24, 2006

Subsystems (2)

Memory management Library code

lore

Share & tools

lore

Peter Zijlstra

author

2006-09-23 18:40 UTC #0 lore raw

Hi Nick,

I noticed the comment above radix_tree_lookup_slot() did not match the uses in your lockless pagecache. Would this patch be correct?

---
 lib/radix-tree.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

lib/radix-tree.c +4 -5

Index: linux-2.6-mm/lib/radix-tree.c
===================================================================
@@ -380,11 +380,10 @@ EXPORT_SYMBOL(radix_tree_insert);
  *	Returns:  the slot corresponding to the position @index in the
  *	radix tree @root. This is useful for update-if-exists operations.
  *
- *	This function cannot be called under rcu_read_lock, it must be
- *	excluded from writers, as must the returned slot for subsequent
- *	use by radix_tree_deref_slot() and radix_tree_replace slot.
- *	Caller must hold tree write locked across slot lookup and
- *	replace.
+ * 	This function can be called under rcu_read_lock iff the slot is not
+ * 	modified by radix_tree_replace_slot, otherwise it must be called
+ * 	exclusive from other writers. Any dereference of the slot must be done
+ * 	using radix_tree_deref_slot.
  */
 void **radix_tree_lookup_slot(struct radix_tree_root *root, unsigned long index)
 {

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Nick Piggin

2006-09-24 14:40 UTC #1 lore raw

Hi Peter,

Yes, that looks much better, thanks.

BTW. this latest patchset has at least one -mm mismerge (page flags bit), so I would advise against testing (though do appreciate reviews). I'll send Andrew a more thorough version against 2.6.18-mm1.

Thanks,
Nick

Quoted 21 lines · Peter Zijlstra

Hi Nick,
I noticed the comment above radix_tree_lookup_slot() did not match the uses in your lockless pagecache. Would this patch be correct?
---
lib/radix-tree.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
Index: linux-2.6-mm/lib/radix-tree.c
===================================================================
lib/radix-tree.c +0 -2
@@ -380,11 +380,10 @@ EXPORT_SYMBOL(radix_tree_insert);
 *	Returns:  the slot corresponding to the position @index in the
 *	radix tree @root. This is useful for update-if-exists operations.
 *
- *	This function cannot be called under rcu_read_lock, it must be
- *	excluded from writers, as must the returned slot for subsequent

lib/radix-tree.c EXPORT_SYMBOL(radix_tree_insert); · line 389

Quoted 10 lines

lib/radix-tree.c +4 -3

- *	use by radix_tree_deref_slot() and radix_tree_replace slot.
- *	Caller must hold tree write locked across slot lookup and
- *	replace.
+ * 	This function can be called under rcu_read_lock iff the slot is not
+ * 	modified by radix_tree_replace_slot, otherwise it must be called
+ * 	exclusive from other writers. Any dereference of the slot must be done
+ * 	using radix_tree_deref_slot.
 */
 void **radix_tree_lookup_slot(struct radix_tree_root *root, unsigned long index)
 {