lkml.org 
[lkml]   [2013]   [Jun]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH 2/3] 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>
---
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


\
 
 \ /
  Last update: 2013-06-15 19:41    [W:0.092 / U:0.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site