← Back to branch summary

~zequence/pulseaudio/ubuntu.precise-fix

« back to all changes in this revision

Viewing changes to debian/patches/0110-flist-Avoid-the-ABA-problem.patch

  • Committer: David Henningsson
  • Date: 2012-03-08 23:10:53 UTC
  • Revision ID: david.henningsson@canonical.com-20120308231053-4xgey3vkwibmykci
https://launchpad.net/bugs/924416
[ Luke Yelavich ]
[ David Henningsson ]
0110-flist-Avoid-the-ABA-problem.patch:
Fix occasional crashes in pa_memblock_free, pa_memblock_ref and drop_block
(LP: #924416)

Show diffs side-by-side

added added

removed removed

Lines of Context:
debian/patches/0110-flist-Avoid-the-ABA-problem.patch
 
1
From 641103314a423328094624db0f518630d5d90dee Mon Sep 17 00:00:00 2001
 
2
From: David Henningsson <david.henningsson@canonical.com>
 
3
Date: Sun, 4 Mar 2012 06:07:37 +0100
 
4
Subject: [PATCH 1/2] flist: Avoid the ABA problem
 
5
 
 
6
Our flist implementation suffers from the ABA problem
 
7
(see http://en.wikipedia.org/wiki/ABA_problem), causing PulseAudio
 
8
to crash very rarely, usually inside memblock operations.
 
9
By turning stored pointers into stored table indices, we have some
 
10
extra bits that we can use to store tag bits, which is a known
 
11
workaround for the ABA problem.
 
12
 
 
13
Buglink: https://bugs.launchpad.net/bugs/924416
 
14
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
 
15
---
 
16
 src/pulsecore/flist.c |   70 ++++++++++++++++++++++++++++++++++--------------
 
17
 1 files changed, 49 insertions(+), 21 deletions(-)
 
18
 
 
19
diff --git a/src/pulsecore/flist.c b/src/pulsecore/flist.c
 
20
index d279271..fa8974a 100644
 
21
--- a/src/pulsecore/flist.c
 
22
+++ b/src/pulsecore/flist.c
 
23
@@ -3,6 +3,7 @@
 
24
 
 
25
   Copyright 2006-2008 Lennart Poettering
 
26
   Copyright (C) 2009 Nokia Corporation and/or its subsidiary(-ies).
 
27
+  Copyright (C) 2012 Canonical Ltd.
 
28
 
 
29
   Contact: Jyri Sarha <Jyri.Sarha@nokia.com>
 
30
 
 
31
@@ -38,9 +39,15 @@
 
32
 
 
33
 #define FLIST_SIZE 128
 
34
 
 
35
+/* Atomic table indices contain
 
36
+   sign bit = if set, indicates empty/NULL value
 
37
+   tag bits (to avoid the ABA problem)
 
38
+   actual index bits
 
39
+*/
 
40
+
 
41
 /* Lock free single linked list element. */
 
42
 struct pa_flist_elem {
 
43
-    pa_atomic_ptr_t next;
 
44
+    pa_atomic_t next;
 
45
     pa_atomic_ptr_t ptr;
 
46
 };
 
47
 
 
48
@@ -49,34 +56,49 @@ typedef struct pa_flist_elem pa_flist_elem;
 
49
 struct pa_flist {
 
50
     const char *name;
 
51
     unsigned size;
 
52
+
 
53
+    pa_atomic_t current_tag;
 
54
+    int index_mask;
 
55
+    int tag_shift;
 
56
+    int tag_mask;
 
57
+
 
58
     /* Stack that contains pointers stored into free list */
 
59
-    pa_atomic_ptr_t stored;
 
60
+    pa_atomic_t stored;
 
61
     /* Stack that contains empty list elements */
 
62
-    pa_atomic_ptr_t empty;
 
63
+    pa_atomic_t empty;
 
64
     pa_flist_elem table[];
 
65
 };
 
66
 
 
67
 /* Lock free pop from linked list stack */
 
68
-static pa_flist_elem *stack_pop(pa_atomic_ptr_t *list) {
 
69
-    pa_flist_elem *poped;
 
70
+static pa_flist_elem *stack_pop(pa_flist *flist, pa_atomic_t *list) {
 
71
+    pa_flist_elem *popped;
 
72
+    int idx;
 
73
     pa_assert(list);
 
74
 
 
75
     do {
 
76
-        poped = (pa_flist_elem *) pa_atomic_ptr_load(list);
 
77
-    } while (poped != NULL && !pa_atomic_ptr_cmpxchg(list, poped, pa_atomic_ptr_load(&poped->next)));
 
78
+        idx = pa_atomic_load(list);
 
79
+        if (idx < 0)
 
80
+            return NULL;
 
81
+        popped = &flist->table[idx & flist->index_mask];
 
82
+    } while (!pa_atomic_cmpxchg(list, idx, pa_atomic_load(&popped->next)));
 
83
 
 
84
-    return poped;
 
85
+    return popped;
 
86
 }
 
87
 
 
88
 /* Lock free push to linked list stack */
 
89
-static void stack_push(pa_atomic_ptr_t *list, pa_flist_elem *new_elem) {
 
90
-    pa_flist_elem *next;
 
91
+static void stack_push(pa_flist *flist, pa_atomic_t *list, pa_flist_elem *new_elem) {
 
92
+    int tag, newindex, next;
 
93
     pa_assert(list);
 
94
 
 
95
+    tag = pa_atomic_inc(&flist->current_tag);
 
96
+    newindex = new_elem - flist->table;
 
97
+    pa_assert(newindex >= 0 && newindex < (int) flist->size);
 
98
+    newindex |= (tag << flist->tag_shift) & flist->tag_mask;
 
99
+
 
100
     do {
 
101
-        next = pa_atomic_ptr_load(list);
 
102
-        pa_atomic_ptr_store(&new_elem->next, next);
 
103
-    } while (!pa_atomic_ptr_cmpxchg(list, next, new_elem));
 
104
+        next = pa_atomic_load(list);
 
105
+        pa_atomic_store(&new_elem->next, next);
 
106
+    } while (!pa_atomic_cmpxchg(list, next, newindex));
 
107
 }
 
108
 
 
109
 pa_flist *pa_flist_new_with_name(unsigned size, const char *name) {
 
110
@@ -91,10 +113,16 @@ pa_flist *pa_flist_new_with_name(unsigned size, const char *name) {
 
111
 
 
112
     l->name = pa_xstrdup(name);
 
113
     l->size = size;
 
114
-    pa_atomic_ptr_store(&l->stored, NULL);
 
115
-    pa_atomic_ptr_store(&l->empty, NULL);
 
116
+
 
117
+    while (1 << l->tag_shift < (int) size)
 
118
+        l->tag_shift++;
 
119
+    l->index_mask = (1 << l->tag_shift) - 1;
 
120
+    l->tag_mask = INT_MAX - l->index_mask;
 
121
+
 
122
+    pa_atomic_store(&l->stored, -1);
 
123
+    pa_atomic_store(&l->empty, -1);
 
124
     for (i=0; i < size; i++) {
 
125
-        stack_push(&l->empty, &l->table[i]);
 
126
+        stack_push(l, &l->empty, &l->table[i]);
 
127
     }
 
128
     return l;
 
129
 }
 
130
@@ -109,7 +137,7 @@ void pa_flist_free(pa_flist *l, pa_free_cb_t free_cb) {
 
131
 
 
132
     if (free_cb) {
 
133
         pa_flist_elem *elem;
 
134
-        while((elem = stack_pop(&l->stored)))
 
135
+        while((elem = stack_pop(l, &l->stored)))
 
136
             free_cb(pa_atomic_ptr_load(&elem->ptr));
 
137
     }
 
138
 
 
139
@@ -122,14 +150,14 @@ int pa_flist_push(pa_flist *l, void *p) {
 
140
     pa_assert(l);
 
141
     pa_assert(p);
 
142
 
 
143
-    elem = stack_pop(&l->empty);
 
144
+    elem = stack_pop(l, &l->empty);
 
145
     if (elem == NULL) {
 
146
         if (pa_log_ratelimit(PA_LOG_DEBUG))
 
147
             pa_log_debug("%s flist is full (don't worry)", l->name);
 
148
         return -1;
 
149
     }
 
150
     pa_atomic_ptr_store(&elem->ptr, p);
 
151
-    stack_push(&l->stored, elem);
 
152
+    stack_push(l, &l->stored, elem);
 
153
 
 
154
     return 0;
 
155
 }
 
156
@@ -139,13 +167,13 @@ void* pa_flist_pop(pa_flist *l) {
 
157
     void *ptr;
 
158
     pa_assert(l);
 
159
 
 
160
-    elem = stack_pop(&l->stored);
 
161
+    elem = stack_pop(l, &l->stored);
 
162
     if (elem == NULL)
 
163
         return NULL;
 
164
 
 
165
     ptr = pa_atomic_ptr_load(&elem->ptr);
 
166
 
 
167
-    stack_push(&l->empty, elem);
 
168
+    stack_push(l, &l->empty, elem);
 
169
 
 
170
     return ptr;
 
171
 }
 
172
-- 
 
173
1.7.5.4
 
174