Skip to contentSkip to conversation

rhashtable annotation fixes

PATCH In review Thomas Graf 2 participants 6 messages Aug 13, 2014 – Aug 14, 2014

Subsystems (4)
Networking Network drivers Library code Rhashtable
lore
Share & tools
lore

Thomas Graf

author maintainer
#0 lore raw

cover rhashtable annotation fixes

Thomas Graf (4):
rhashtable: RCU annotations for next pointers
rhashtable: unexport and make rht_obj() static
rhashtable: fix annotations for rht_for_each_entry_rcu()
netlink: Annotate RCU locking for seq_file walker

 include/linux/rhashtable.h | 17 ++++++-----------
 lib/rhashtable.c           | 10 ++--------
 net/netlink/af_netlink.c   |  2 ++
 3 files changed, 10 insertions(+), 19 deletions(-)
Signature · 1 line
1.9.3
author maintainer
#1 lore raw

PATCH 1/4 rhashtable: RCU annotations for next pointers

Properly annotate next pointers as access is RCU protected in the lookup path.

Signed-off-by Thomas Graf <[email protected]>
---
 include/linux/rhashtable.h | 4 ++--
 lib/rhashtable.c           | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
include/linux/rhashtable.h +2 -2
index 9cda293..8c6048e 100644
@@ -21,7 +21,7 @@
 #include <linux/rculist.h>
 
 struct rhash_head {
-	struct rhash_head		*next;
+	struct rhash_head __rcu		*next;
 };
 
 #define INIT_HASH_HEAD(ptr) ((ptr)->next = NULL)

@@ -97,7 +97,7 @@ u32 rhashtable_obj_hashfn(const struct rhashtable *ht, void *ptr);
 void rhashtable_insert(struct rhashtable *ht, struct rhash_head *node, gfp_t);
 bool rhashtable_remove(struct rhashtable *ht, struct rhash_head *node, gfp_t);
 void rhashtable_remove_pprev(struct rhashtable *ht, struct rhash_head *obj,
-			     struct rhash_head **pprev, gfp_t flags);
+			     struct rhash_head __rcu **pprev, gfp_t flags);
 
 bool rht_grow_above_75(const struct rhashtable *ht, size_t new_size);
 bool rht_shrink_below_30(const struct rhashtable *ht, size_t new_size);
lib/rhashtable.c +1 -1
index e6940cf..338dd7a 100644
@@ -386,7 +386,7 @@ EXPORT_SYMBOL_GPL(rhashtable_insert);
  * deletion when combined with walking or lookup.
  */
 void rhashtable_remove_pprev(struct rhashtable *ht, struct rhash_head *obj,
-			     struct rhash_head **pprev, gfp_t flags)
+			     struct rhash_head __rcu **pprev, gfp_t flags)
 {
 	struct bucket_table *tbl = rht_dereference(ht->tbl, ht);
Signature · 1 line
1.9.3
author maintainer
#2 lore raw

PATCH 2/4 rhashtable: unexport and make rht_obj() static

In reply to Thomas Graf

No need to export rht_obj(), all inner to outer object translations occur internally. It was intended to be used with rht_for_each() which now primarily serves as the iterator for rhashtable_remove_pprev() to effectively flush and free the full table.

Signed-off-by Thomas Graf <[email protected]>
---
 include/linux/rhashtable.h | 1 -
 lib/rhashtable.c           | 8 +-------
 2 files changed, 1 insertion(+), 8 deletions(-)
include/linux/rhashtable.h +0 -1
index 8c6048e..af967c4 100644
@@ -117,7 +117,6 @@ void rhashtable_destroy(const struct rhashtable *ht);
 #define rht_dereference_rcu(p, ht) \
 	rcu_dereference_check(p, lockdep_rht_mutex_is_held(ht))
 
-/* Internal, use rht_obj() instead */
 #define rht_entry(ptr, type, member) container_of(ptr, type, member)
 #define rht_entry_safe(ptr, type, member) \
 ({ \
lib/rhashtable.c +1 -7
index 338dd7a..a2c7881 100644
@@ -38,16 +38,10 @@ int lockdep_rht_mutex_is_held(const struct rhashtable *ht)
 EXPORT_SYMBOL_GPL(lockdep_rht_mutex_is_held);
 #endif
 
-/**
- * rht_obj - cast hash head to outer object
- * @ht:		hash table
- * @he:		hashed node
- */
-void *rht_obj(const struct rhashtable *ht, const struct rhash_head *he)
+static void *rht_obj(const struct rhashtable *ht, const struct rhash_head *he)
 {
 	return (void *) he - ht->p.head_offset;
 }
-EXPORT_SYMBOL_GPL(rht_obj);
 
 static u32 __hashfn(const struct rhashtable *ht, const void *key,
 		      u32 len, u32 hsize)
Signature · 1 line
1.9.3
author maintainer
#3 lore raw

PATCH 3/4 rhashtable: fix annotations for rht_for_each_entry_rcu()

In reply to Thomas Graf

Call rcu_deference_raw() directly from within rht_for_each_entry_rcu() as list_for_each_entry_rcu() does.

Fixes the following sparse warnings:
net/netlink/af_netlink.c:2906:25: expected struct rhash_head const *__mptr
net/netlink/af_netlink.c:2906:25: got struct rhash_head [noderef] <asn:4>*<noident>

Fixes e341694e3eb57fc ("netlink: Convert netlink_lookup() to use RCU protected hash table")
Signed-off-by Thomas Graf <[email protected]>
---
 include/linux/rhashtable.h | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)
include/linux/rhashtable.h +4 -8
index af967c4..36826c0 100644
@@ -123,11 +123,6 @@ void rhashtable_destroy(const struct rhashtable *ht);
 	typeof(ptr) __ptr = (ptr); \
 	   __ptr ? rht_entry(__ptr, type, member) : NULL; \
 })
-#define rht_entry_safe_rcu(ptr, type, member) \
-({ \
-	typeof(*ptr) __rcu *__ptr = (typeof(*ptr) __rcu __force *)ptr; \
-	__ptr ? container_of((typeof(ptr))rcu_dereference_raw(__ptr), type, member) : NULL; \
-})
 
 #define rht_next_entry_safe(pos, ht, member) \
 ({ \

@@ -204,9 +199,10 @@ void rhashtable_destroy(const struct rhashtable *ht);
  * traversal is guarded by rcu_read_lock().
  */
 #define rht_for_each_entry_rcu(pos, head, member) \
-	for (pos = rht_entry_safe_rcu(head, typeof(*(pos)), member); \
+	for (pos = rht_entry_safe(rcu_dereference_raw(head), \
+				  typeof(*(pos)), member); \
 	     pos; \
-	     pos = rht_entry_safe_rcu((pos)->member.next, \
-				      typeof(*(pos)), member))
+	     pos = rht_entry_safe(rcu_dereference_raw((pos)->member.next), \
+				  typeof(*(pos)), member))
 
 #endif /* _LINUX_RHASHTABLE_H */
Signature · 1 line
1.9.3
author maintainer
#4 lore raw

PATCH 4/4 netlink: Annotate RCU locking for seq_file walker

In reply to Thomas Graf

Silences the following sparse warnings:
net/netlink/af_netlink.c:2926:21: warning: context imbalance in 'netlink_seq_start' - wrong count at exit net/netlink/af_netlink.c:2972:13: warning: context imbalance in 'netlink_seq_stop' - unexpected unlock

Signed-off-by Thomas Graf <[email protected]>
---
 net/netlink/af_netlink.c | 2 ++
 1 file changed, 2 insertions(+)
net/netlink/af_netlink.c +2 -0
index 2e152e5..c416725 100644
@@ -2921,6 +2921,7 @@ static struct sock *netlink_seq_socket_idx(struct seq_file *seq, loff_t pos)
 }
 
 static void *netlink_seq_start(struct seq_file *seq, loff_t *pos)
+	__acquires(RCU)
 {
 	rcu_read_lock();
 	return *pos ? netlink_seq_socket_idx(seq, *pos - 1) : SEQ_START_TOKEN;

@@ -2970,6 +2971,7 @@ static void *netlink_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 }
 
 static void netlink_seq_stop(struct seq_file *seq, void *v)
+	__releases(RCU)
 {
 	rcu_read_unlock();
 }
Signature · 1 line
1.9.3
#5 lore raw
In reply to Thomas Graf

From: Thomas Graf <[email protected]>
Date: Wed, 13 Aug 2014 16:38:28 +0200

Quoted 5 lines

Thomas Graf (4):
rhashtable: RCU annotations for next pointers
rhashtable: unexport and make rht_obj() static
rhashtable: fix annotations for rht_for_each_entry_rcu()
netlink: Annotate RCU locking for seq_file walker

Series applied, thanks Thomas.