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 <o...@redhat.com>
---
include/linux/llist.h | 19 +++++++------------
lib/llist.c | 15 +++++----------
2 files changed, 12 insertions(+), 22 deletions(-)
diff --git a/include/linux/llist.h b/include/linux/llist.h
index a5199f6..3e2b969 100644
--- a/include/linux/llist.h
+++ b/include/linux/llist.h
@@ -151,18 +151,13 @@ static inline struct llist_node *llist_next(struct
llist_node *node)
*/
static inline bool llist_add(struct llist_node *new, struct llist_head *head)
{
- struct llist_node *entry, *old_entry;
-
- entry = head->first;
- for (;;) {
- old_entry = entry;
- new->next = entry;
- entry = cmpxchg(&head->first, old_entry, new);
- if (entry == old_entry)
- break;
- }
-
- return old_entry == NULL;
+ struct llist_node *first;
+
+ do {
+ new->next = first = ACCESS_ONCE(head->first);
+ } while (cmpxchg(&head->first, first, new) != first);
+
+ return !first;
}
/**
diff --git a/lib/llist.c b/lib/llist.c
index 4a15115..4a70d12 100644
--- a/lib/llist.c
+++ b/lib/llist.c
@@ -39,18 +39,13 @@
bool llist_add_batch(struct llist_node *new_first, struct llist_node *new_last,
struct llist_head *head)
{
- struct llist_node *entry, *old_entry;
+ struct llist_node *first;
- entry = head->first;
- for (;;) {
- old_entry = entry;
- new_last->next = entry;
- entry = cmpxchg(&head->first, old_entry, new_first);
- if (entry == old_entry)
- break;
- }
+ do {
+ new_last->next = first = ACCESS_ONCE(head->first);
+ } while (cmpxchg(&head->first, first, new_first) != first);
- return old_entry == NULL;
+ return !first;
}
EXPORT_SYMBOL_GPL(llist_add_batch);
--
1.5.5.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
