Commit fb4214db50b00558cc6e274c88b3f7325068e942
Committed by
Al Viro
1 parent
4f5e65a1cc
Exists in
smarc-imx_3.14.28_1.0.0_ga
and in
1 other branch
llist: fix/simplify llist_add() and llist_add_batch()
1. This is mostly theoretical, but llist_add*() need ACCESS_ONCE(). Otherwise it is not guaranteed that the first cmpxchg() uses the same value for old_entry and new_last->next. 2. These helpers cache the result of cmpxchg() and read the initial value of head->first before the main loop. I do not think this makes sense. In the likely case cmpxchg() succeeds, otherwise it doesn't hurt to reload head->first. I think it would be better to simplify the code and simply read ->first before cmpxchg(). Signed-off-by: Oleg Nesterov <oleg@redhat.com> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Andrey Vagin <avagin@openvz.org> Cc: "Eric W. Biederman" <ebiederm@xmission.com> Cc: David Howells <dhowells@redhat.com> Cc: Huang Ying <ying.huang@intel.com> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Showing 2 changed files with 12 additions and 22 deletions Side-by-side Diff
include/linux/llist.h
... | ... | @@ -151,18 +151,13 @@ |
151 | 151 | */ |
152 | 152 | static inline bool llist_add(struct llist_node *new, struct llist_head *head) |
153 | 153 | { |
154 | - struct llist_node *entry, *old_entry; | |
154 | + struct llist_node *first; | |
155 | 155 | |
156 | - entry = head->first; | |
157 | - for (;;) { | |
158 | - old_entry = entry; | |
159 | - new->next = entry; | |
160 | - entry = cmpxchg(&head->first, old_entry, new); | |
161 | - if (entry == old_entry) | |
162 | - break; | |
163 | - } | |
156 | + do { | |
157 | + new->next = first = ACCESS_ONCE(head->first); | |
158 | + } while (cmpxchg(&head->first, first, new) != first); | |
164 | 159 | |
165 | - return old_entry == NULL; | |
160 | + return !first; | |
166 | 161 | } |
167 | 162 | |
168 | 163 | /** |
lib/llist.c
... | ... | @@ -39,18 +39,13 @@ |
39 | 39 | bool llist_add_batch(struct llist_node *new_first, struct llist_node *new_last, |
40 | 40 | struct llist_head *head) |
41 | 41 | { |
42 | - struct llist_node *entry, *old_entry; | |
42 | + struct llist_node *first; | |
43 | 43 | |
44 | - entry = head->first; | |
45 | - for (;;) { | |
46 | - old_entry = entry; | |
47 | - new_last->next = entry; | |
48 | - entry = cmpxchg(&head->first, old_entry, new_first); | |
49 | - if (entry == old_entry) | |
50 | - break; | |
51 | - } | |
44 | + do { | |
45 | + new_last->next = first = ACCESS_ONCE(head->first); | |
46 | + } while (cmpxchg(&head->first, first, new_first) != first); | |
52 | 47 | |
53 | - return old_entry == NULL; | |
48 | + return !first; | |
54 | 49 | } |
55 | 50 | EXPORT_SYMBOL_GPL(llist_add_batch); |
56 | 51 |