diff options
author | James Bottomley <James.Bottomley@steeleye.com> | 2005-08-22 11:06:19 -0400 |
---|---|---|
committer | James Bottomley <jejb@mulgrave.(none)> | 2005-08-30 23:44:20 -0400 |
commit | 53c165e0a6c8a4ff7df316557528fa7a52d20711 (patch) | |
tree | 354c599a07c45c71da2b848a90bbe4a98c42d333 /drivers | |
parent | 51490c89f95b8581782e9baa855da166441852be (diff) |
[SCSI] correct attribute_container list usage
One of the changes in the attribute_container code in the scsi-misc tree
was to add a lock to protect the list of devices per container. This,
unfortunately, leads to potential scheduling while atomic problems if
there's a sleep in the function called by a trigger.
The correct solution is to use the kernel klist infrastructure instead
which allows lockless traversal of a list.
Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>
Diffstat (limited to 'drivers')
-rw-r--r-- | drivers/base/attribute_container.c | 51 |
1 files changed, 29 insertions, 22 deletions
diff --git a/drivers/base/attribute_container.c b/drivers/base/attribute_container.c index ebcae5c34133..6c0f49340eb2 100644 --- a/drivers/base/attribute_container.c +++ b/drivers/base/attribute_container.c | |||
@@ -22,7 +22,7 @@ | |||
22 | /* This is a private structure used to tie the classdev and the | 22 | /* This is a private structure used to tie the classdev and the |
23 | * container .. it should never be visible outside this file */ | 23 | * container .. it should never be visible outside this file */ |
24 | struct internal_container { | 24 | struct internal_container { |
25 | struct list_head node; | 25 | struct klist_node node; |
26 | struct attribute_container *cont; | 26 | struct attribute_container *cont; |
27 | struct class_device classdev; | 27 | struct class_device classdev; |
28 | }; | 28 | }; |
@@ -57,8 +57,7 @@ int | |||
57 | attribute_container_register(struct attribute_container *cont) | 57 | attribute_container_register(struct attribute_container *cont) |
58 | { | 58 | { |
59 | INIT_LIST_HEAD(&cont->node); | 59 | INIT_LIST_HEAD(&cont->node); |
60 | INIT_LIST_HEAD(&cont->containers); | 60 | klist_init(&cont->containers); |
61 | spin_lock_init(&cont->containers_lock); | ||
62 | 61 | ||
63 | down(&attribute_container_mutex); | 62 | down(&attribute_container_mutex); |
64 | list_add_tail(&cont->node, &attribute_container_list); | 63 | list_add_tail(&cont->node, &attribute_container_list); |
@@ -78,13 +77,13 @@ attribute_container_unregister(struct attribute_container *cont) | |||
78 | { | 77 | { |
79 | int retval = -EBUSY; | 78 | int retval = -EBUSY; |
80 | down(&attribute_container_mutex); | 79 | down(&attribute_container_mutex); |
81 | spin_lock(&cont->containers_lock); | 80 | spin_lock(&cont->containers.k_lock); |
82 | if (!list_empty(&cont->containers)) | 81 | if (!list_empty(&cont->containers.k_list)) |
83 | goto out; | 82 | goto out; |
84 | retval = 0; | 83 | retval = 0; |
85 | list_del(&cont->node); | 84 | list_del(&cont->node); |
86 | out: | 85 | out: |
87 | spin_unlock(&cont->containers_lock); | 86 | spin_unlock(&cont->containers.k_lock); |
88 | up(&attribute_container_mutex); | 87 | up(&attribute_container_mutex); |
89 | return retval; | 88 | return retval; |
90 | 89 | ||
@@ -143,7 +142,6 @@ attribute_container_add_device(struct device *dev, | |||
143 | continue; | 142 | continue; |
144 | } | 143 | } |
145 | memset(ic, 0, sizeof(struct internal_container)); | 144 | memset(ic, 0, sizeof(struct internal_container)); |
146 | INIT_LIST_HEAD(&ic->node); | ||
147 | ic->cont = cont; | 145 | ic->cont = cont; |
148 | class_device_initialize(&ic->classdev); | 146 | class_device_initialize(&ic->classdev); |
149 | ic->classdev.dev = get_device(dev); | 147 | ic->classdev.dev = get_device(dev); |
@@ -154,13 +152,22 @@ attribute_container_add_device(struct device *dev, | |||
154 | fn(cont, dev, &ic->classdev); | 152 | fn(cont, dev, &ic->classdev); |
155 | else | 153 | else |
156 | attribute_container_add_class_device(&ic->classdev); | 154 | attribute_container_add_class_device(&ic->classdev); |
157 | spin_lock(&cont->containers_lock); | 155 | klist_add_tail(&ic->node, &cont->containers); |
158 | list_add_tail(&ic->node, &cont->containers); | ||
159 | spin_unlock(&cont->containers_lock); | ||
160 | } | 156 | } |
161 | up(&attribute_container_mutex); | 157 | up(&attribute_container_mutex); |
162 | } | 158 | } |
163 | 159 | ||
160 | /* FIXME: can't break out of this unless klist_iter_exit is also | ||
161 | * called before doing the break | ||
162 | */ | ||
163 | #define klist_for_each_entry(pos, head, member, iter) \ | ||
164 | for (klist_iter_init(head, iter); (pos = ({ \ | ||
165 | struct klist_node *n = klist_next(iter); \ | ||
166 | n ? ({ klist_iter_exit(iter) ; NULL; }) : \ | ||
167 | container_of(n, typeof(*pos), member);\ | ||
168 | }) ) != NULL; ) | ||
169 | |||
170 | |||
164 | /** | 171 | /** |
165 | * attribute_container_remove_device - make device eligible for removal. | 172 | * attribute_container_remove_device - make device eligible for removal. |
166 | * | 173 | * |
@@ -187,18 +194,19 @@ attribute_container_remove_device(struct device *dev, | |||
187 | 194 | ||
188 | down(&attribute_container_mutex); | 195 | down(&attribute_container_mutex); |
189 | list_for_each_entry(cont, &attribute_container_list, node) { | 196 | list_for_each_entry(cont, &attribute_container_list, node) { |
190 | struct internal_container *ic, *tmp; | 197 | struct internal_container *ic; |
198 | struct klist_iter iter; | ||
191 | 199 | ||
192 | if (attribute_container_no_classdevs(cont)) | 200 | if (attribute_container_no_classdevs(cont)) |
193 | continue; | 201 | continue; |
194 | 202 | ||
195 | if (!cont->match(cont, dev)) | 203 | if (!cont->match(cont, dev)) |
196 | continue; | 204 | continue; |
197 | spin_lock(&cont->containers_lock); | 205 | |
198 | list_for_each_entry_safe(ic, tmp, &cont->containers, node) { | 206 | klist_for_each_entry(ic, &cont->containers, node, &iter) { |
199 | if (dev != ic->classdev.dev) | 207 | if (dev != ic->classdev.dev) |
200 | continue; | 208 | continue; |
201 | list_del(&ic->node); | 209 | klist_remove(&ic->node); |
202 | if (fn) | 210 | if (fn) |
203 | fn(cont, dev, &ic->classdev); | 211 | fn(cont, dev, &ic->classdev); |
204 | else { | 212 | else { |
@@ -206,7 +214,6 @@ attribute_container_remove_device(struct device *dev, | |||
206 | class_device_unregister(&ic->classdev); | 214 | class_device_unregister(&ic->classdev); |
207 | } | 215 | } |
208 | } | 216 | } |
209 | spin_unlock(&cont->containers_lock); | ||
210 | } | 217 | } |
211 | up(&attribute_container_mutex); | 218 | up(&attribute_container_mutex); |
212 | } | 219 | } |
@@ -232,7 +239,8 @@ attribute_container_device_trigger(struct device *dev, | |||
232 | 239 | ||
233 | down(&attribute_container_mutex); | 240 | down(&attribute_container_mutex); |
234 | list_for_each_entry(cont, &attribute_container_list, node) { | 241 | list_for_each_entry(cont, &attribute_container_list, node) { |
235 | struct internal_container *ic, *tmp; | 242 | struct internal_container *ic; |
243 | struct klist_iter iter; | ||
236 | 244 | ||
237 | if (!cont->match(cont, dev)) | 245 | if (!cont->match(cont, dev)) |
238 | continue; | 246 | continue; |
@@ -242,12 +250,10 @@ attribute_container_device_trigger(struct device *dev, | |||
242 | continue; | 250 | continue; |
243 | } | 251 | } |
244 | 252 | ||
245 | spin_lock(&cont->containers_lock); | 253 | klist_for_each_entry(ic, &cont->containers, node, &iter) { |
246 | list_for_each_entry_safe(ic, tmp, &cont->containers, node) { | ||
247 | if (dev == ic->classdev.dev) | 254 | if (dev == ic->classdev.dev) |
248 | fn(cont, dev, &ic->classdev); | 255 | fn(cont, dev, &ic->classdev); |
249 | } | 256 | } |
250 | spin_unlock(&cont->containers_lock); | ||
251 | } | 257 | } |
252 | up(&attribute_container_mutex); | 258 | up(&attribute_container_mutex); |
253 | } | 259 | } |
@@ -397,15 +403,16 @@ attribute_container_find_class_device(struct attribute_container *cont, | |||
397 | { | 403 | { |
398 | struct class_device *cdev = NULL; | 404 | struct class_device *cdev = NULL; |
399 | struct internal_container *ic; | 405 | struct internal_container *ic; |
406 | struct klist_iter iter; | ||
400 | 407 | ||
401 | spin_lock(&cont->containers_lock); | 408 | klist_for_each_entry(ic, &cont->containers, node, &iter) { |
402 | list_for_each_entry(ic, &cont->containers, node) { | ||
403 | if (ic->classdev.dev == dev) { | 409 | if (ic->classdev.dev == dev) { |
404 | cdev = &ic->classdev; | 410 | cdev = &ic->classdev; |
411 | /* FIXME: must exit iterator then break */ | ||
412 | klist_iter_exit(&iter); | ||
405 | break; | 413 | break; |
406 | } | 414 | } |
407 | } | 415 | } |
408 | spin_unlock(&cont->containers_lock); | ||
409 | 416 | ||
410 | return cdev; | 417 | return cdev; |
411 | } | 418 | } |