1 From: http://xenbits.xensource.com/linux-2.6.18-xen.hg?rev/ab1d4fbbe4bf
2 # HG changeset 848 patch
3 # User Keir Fraser <keir.fraser@citrix.com>
4 # Date 1238497203 -3600
5 # Node ID ab1d4fbbe4bf93f203e0c4d62c18bd248e4f1d4a
6 # Parent ad4d307bf9ced378d0b49d4559ae33ecbff3c1b7
7 Subject: netfront accel: Simplify, document, and fix a theoretical bug in use
8 of spin locks by netfront acceleration plugins
10 Patch-mainline: obsolete
12 Signed-off-by: Kieran Mansley <kmansley@solarflare.com>
13 Acked-by: jbeulich@novell.com
15 --- sle11-2009-04-09.orig/drivers/xen/netfront/accel.c 2009-04-09 14:41:04.000000000 +0200
16 +++ sle11-2009-04-09/drivers/xen/netfront/accel.c 2009-04-09 14:43:45.000000000 +0200
17 @@ -57,9 +57,6 @@ static int netfront_load_accelerator(str
19 static struct list_head accelerators_list;
21 -/* Lock to protect access to accelerators_list */
22 -static spinlock_t accelerators_lock;
24 /* Workqueue to process acceleration configuration changes */
25 struct workqueue_struct *accel_watch_workqueue;
27 @@ -69,7 +66,6 @@ DEFINE_MUTEX(accelerator_mutex);
28 void netif_init_accel(void)
30 INIT_LIST_HEAD(&accelerators_list);
31 - spin_lock_init(&accelerators_lock);
33 accel_watch_workqueue = create_workqueue("net_accel");
35 @@ -77,13 +73,11 @@ void netif_init_accel(void)
36 void netif_exit_accel(void)
38 struct netfront_accelerator *accelerator, *tmp;
39 - unsigned long flags;
41 flush_workqueue(accel_watch_workqueue);
42 destroy_workqueue(accel_watch_workqueue);
44 - spin_lock_irqsave(&accelerators_lock, flags);
46 + /* No lock required as everything else should be quiet by now */
47 list_for_each_entry_safe(accelerator, tmp, &accelerators_list, link) {
48 BUG_ON(!list_empty(&accelerator->vif_states));
50 @@ -91,8 +85,6 @@ void netif_exit_accel(void)
51 kfree(accelerator->frontend);
55 - spin_unlock_irqrestore(&accelerators_lock, flags);
59 @@ -245,16 +237,12 @@ static int match_accelerator(const char
62 * Add a frontend vif to the list of vifs that is using a netfront
63 - * accelerator plugin module.
64 + * accelerator plugin module. Must be called with the accelerator
67 static void add_accelerator_vif(struct netfront_accelerator *accelerator,
68 struct netfront_info *np)
70 - unsigned long flags;
72 - /* Need lock to write list */
73 - spin_lock_irqsave(&accelerator->vif_states_lock, flags);
75 if (np->accelerator == NULL) {
76 np->accelerator = accelerator;
78 @@ -267,13 +255,13 @@ static void add_accelerator_vif(struct n
80 BUG_ON(np->accelerator != accelerator);
83 - spin_unlock_irqrestore(&accelerator->vif_states_lock, flags);
88 * Initialise the state to track an accelerator plugin module.
90 + * Must be called with the accelerator mutex held.
92 static int init_accelerator(const char *frontend,
93 struct netfront_accelerator **result,
94 @@ -281,7 +269,6 @@ static int init_accelerator(const char *
96 struct netfront_accelerator *accelerator =
97 kmalloc(sizeof(struct netfront_accelerator), GFP_KERNEL);
98 - unsigned long flags;
102 @@ -303,9 +290,7 @@ static int init_accelerator(const char *
104 accelerator->hooks = hooks;
106 - spin_lock_irqsave(&accelerators_lock, flags);
107 list_add(&accelerator->link, &accelerators_list);
108 - spin_unlock_irqrestore(&accelerators_lock, flags);
110 *result = accelerator;
112 @@ -316,11 +301,14 @@ static int init_accelerator(const char *
114 * Modify the hooks stored in the per-vif state to match that in the
115 * netfront accelerator's state.
117 + * Takes the vif_states_lock spinlock and may sleep.
120 accelerator_set_vif_state_hooks(struct netfront_accel_vif_state *vif_state)
122 - /* This function must be called with the vif_states_lock held */
123 + struct netfront_accelerator *accelerator;
124 + unsigned long flags;
126 DPRINTK("%p\n",vif_state);
128 @@ -328,19 +316,25 @@ accelerator_set_vif_state_hooks(struct n
129 netif_poll_disable(vif_state->np->netdev);
130 netif_tx_lock_bh(vif_state->np->netdev);
132 - vif_state->hooks = vif_state->np->accelerator->hooks;
133 + accelerator = vif_state->np->accelerator;
134 + spin_lock_irqsave(&accelerator->vif_states_lock, flags);
135 + vif_state->hooks = accelerator->hooks;
136 + spin_unlock_irqrestore(&accelerator->vif_states_lock, flags);
138 netif_tx_unlock_bh(vif_state->np->netdev);
139 netif_poll_enable(vif_state->np->netdev);
144 + * Must be called with the accelerator mutex held. Takes the
145 + * vif_states_lock spinlock.
147 static void accelerator_probe_new_vif(struct netfront_info *np,
148 struct xenbus_device *dev,
149 struct netfront_accelerator *accelerator)
151 struct netfront_accel_hooks *hooks;
152 - unsigned long flags;
156 @@ -349,17 +343,8 @@ static void accelerator_probe_new_vif(st
158 hooks = accelerator->hooks;
161 - if (hooks->new_device(np->netdev, dev) == 0) {
163 - (&accelerator->vif_states_lock, flags);
165 - accelerator_set_vif_state_hooks(&np->accel_vif_state);
167 - spin_unlock_irqrestore
168 - (&accelerator->vif_states_lock, flags);
171 + if (hooks && hooks->new_device(np->netdev, dev) == 0)
172 + accelerator_set_vif_state_hooks(&np->accel_vif_state);
176 @@ -368,7 +353,10 @@ static void accelerator_probe_new_vif(st
178 * Request that a particular netfront accelerator plugin is loaded.
179 * Usually called as a result of the vif configuration specifying
180 - * which one to use. Must be called with accelerator_mutex held
181 + * which one to use.
183 + * Must be called with accelerator_mutex held. Takes the
184 + * vif_states_lock spinlock.
186 static int netfront_load_accelerator(struct netfront_info *np,
187 struct xenbus_device *dev,
188 @@ -407,13 +395,15 @@ static int netfront_load_accelerator(str
189 * this accelerator. Notify the accelerator plugin of the relevant
190 * device if so. Called when an accelerator plugin module is first
191 * loaded and connects to netfront.
193 + * Must be called with accelerator_mutex held. Takes the
194 + * vif_states_lock spinlock.
197 accelerator_probe_vifs(struct netfront_accelerator *accelerator,
198 struct netfront_accel_hooks *hooks)
200 struct netfront_accel_vif_state *vif_state, *tmp;
201 - unsigned long flags;
203 DPRINTK("%p\n", accelerator);
205 @@ -425,29 +415,22 @@ accelerator_probe_vifs(struct netfront_a
206 BUG_ON(hooks == NULL);
207 accelerator->hooks = hooks;
210 - * currently hold accelerator_mutex, so don't need
211 - * vif_states_lock to read the list
213 + /* Holds accelerator_mutex to iterate list */
214 list_for_each_entry_safe(vif_state, tmp, &accelerator->vif_states,
216 struct netfront_info *np = vif_state->np;
218 - if (hooks->new_device(np->netdev, vif_state->dev) == 0) {
220 - (&accelerator->vif_states_lock, flags);
222 + if (hooks->new_device(np->netdev, vif_state->dev) == 0)
223 accelerator_set_vif_state_hooks(vif_state);
225 - spin_unlock_irqrestore
226 - (&accelerator->vif_states_lock, flags);
233 - * Called by the netfront accelerator plugin module when it has loaded
234 + * Called by the netfront accelerator plugin module when it has
237 + * Takes the accelerator_mutex and vif_states_lock spinlock.
239 int netfront_accelerator_loaded(int version, const char *frontend,
240 struct netfront_accel_hooks *hooks)
241 @@ -503,15 +486,21 @@ EXPORT_SYMBOL_GPL(netfront_accelerator_l
244 * Remove the hooks from a single vif state.
246 + * Takes the vif_states_lock spinlock and may sleep.
249 accelerator_remove_single_hook(struct netfront_accelerator *accelerator,
250 struct netfront_accel_vif_state *vif_state)
252 + unsigned long flags;
254 /* Make sure there are no data path operations going on */
255 netif_poll_disable(vif_state->np->netdev);
256 netif_tx_lock_bh(vif_state->np->netdev);
258 + spin_lock_irqsave(&accelerator->vif_states_lock, flags);
261 * Remove the hooks, but leave the vif_state on the
262 * accelerator's list as that signifies this vif is
263 @@ -520,6 +509,8 @@ accelerator_remove_single_hook(struct ne
265 vif_state->hooks = NULL;
267 + spin_unlock_irqrestore(&accelerator->vif_states_lock, flags);
269 netif_tx_unlock_bh(vif_state->np->netdev);
270 netif_poll_enable(vif_state->np->netdev);
272 @@ -527,25 +518,25 @@ accelerator_remove_single_hook(struct ne
275 * Safely remove the accelerator function hooks from a netfront state.
277 + * Must be called with the accelerator mutex held. Takes the
278 + * vif_states_lock spinlock.
280 static void accelerator_remove_hooks(struct netfront_accelerator *accelerator)
282 - struct netfront_accel_hooks *hooks;
283 struct netfront_accel_vif_state *vif_state, *tmp;
286 - /* Mutex is held so don't need vif_states_lock to iterate list */
287 + /* Mutex is held to iterate list */
288 list_for_each_entry_safe(vif_state, tmp,
289 &accelerator->vif_states,
291 - spin_lock_irqsave(&accelerator->vif_states_lock, flags);
293 if(vif_state->hooks) {
294 - hooks = vif_state->hooks;
296 + spin_lock_irqsave(&accelerator->vif_states_lock, flags);
298 /* Last chance to get statistics from the accelerator */
299 - hooks->get_stats(vif_state->np->netdev,
300 - &vif_state->np->stats);
301 + vif_state->hooks->get_stats(vif_state->np->netdev,
302 + &vif_state->np->stats);
304 spin_unlock_irqrestore(&accelerator->vif_states_lock,
306 @@ -553,9 +544,6 @@ static void accelerator_remove_hooks(str
307 accelerator_remove_single_hook(accelerator, vif_state);
309 accelerator->hooks->remove(vif_state->dev);
311 - spin_unlock_irqrestore(&accelerator->vif_states_lock,
316 @@ -567,47 +555,47 @@ static void accelerator_remove_hooks(str
317 * Called by a netfront accelerator when it is unloaded. This safely
318 * removes the hooks into the plugin and blocks until all devices have
319 * finished using it, so on return it is safe to unload.
321 + * Takes the accelerator mutex, and vif_states_lock spinlock.
323 void netfront_accelerator_stop(const char *frontend)
325 struct netfront_accelerator *accelerator;
326 - unsigned long flags;
328 mutex_lock(&accelerator_mutex);
329 - spin_lock_irqsave(&accelerators_lock, flags);
331 list_for_each_entry(accelerator, &accelerators_list, link) {
332 if (match_accelerator(frontend, accelerator)) {
333 - spin_unlock_irqrestore(&accelerators_lock, flags);
335 accelerator_remove_hooks(accelerator);
340 - spin_unlock_irqrestore(&accelerators_lock, flags);
342 mutex_unlock(&accelerator_mutex);
344 EXPORT_SYMBOL_GPL(netfront_accelerator_stop);
347 -/* Helper for call_remove and do_suspend */
348 -static int do_remove(struct netfront_info *np, struct xenbus_device *dev,
349 - unsigned long *lock_flags)
351 + * Helper for call_remove and do_suspend
353 + * Must be called with the accelerator mutex held. Takes the
354 + * vif_states_lock spinlock.
356 +static int do_remove(struct netfront_info *np, struct xenbus_device *dev)
358 struct netfront_accelerator *accelerator = np->accelerator;
359 - struct netfront_accel_hooks *hooks;
360 + unsigned long flags;
363 if (np->accel_vif_state.hooks) {
364 - hooks = np->accel_vif_state.hooks;
365 + spin_lock_irqsave(&accelerator->vif_states_lock, flags);
367 /* Last chance to get statistics from the accelerator */
368 - hooks->get_stats(np->netdev, &np->stats);
369 + np->accel_vif_state.hooks->get_stats(np->netdev, &np->stats);
371 spin_unlock_irqrestore(&accelerator->vif_states_lock,
376 * Try and do the opposite of accelerator_probe_new_vif
377 @@ -618,20 +606,21 @@ static int do_remove(struct netfront_inf
378 &np->accel_vif_state);
380 rc = accelerator->hooks->remove(dev);
382 - spin_lock_irqsave(&accelerator->vif_states_lock, *lock_flags);
390 + * Must be called with the accelerator mutex held. Takes the
391 + * vif_states_lock spinlock
393 static int netfront_remove_accelerator(struct netfront_info *np,
394 struct xenbus_device *dev)
396 struct netfront_accelerator *accelerator;
397 struct netfront_accel_vif_state *tmp_vif_state;
398 - unsigned long flags;
401 /* Check that we've got a device that was accelerated */
402 @@ -640,8 +629,6 @@ static int netfront_remove_accelerator(s
404 accelerator = np->accelerator;
406 - spin_lock_irqsave(&accelerator->vif_states_lock, flags);
408 list_for_each_entry(tmp_vif_state, &accelerator->vif_states,
410 if (tmp_vif_state == &np->accel_vif_state) {
411 @@ -650,16 +637,18 @@ static int netfront_remove_accelerator(s
415 - rc = do_remove(np, dev, &flags);
416 + rc = do_remove(np, dev);
418 np->accelerator = NULL;
420 - spin_unlock_irqrestore(&accelerator->vif_states_lock, flags);
427 + * No lock pre-requisites. Takes the accelerator mutex and the
428 + * vif_states_lock spinlock.
430 int netfront_accelerator_call_remove(struct netfront_info *np,
431 struct xenbus_device *dev)
433 @@ -671,11 +660,14 @@ int netfront_accelerator_call_remove(str
440 + * No lock pre-requisites. Takes the accelerator mutex and the
441 + * vif_states_lock spinlock.
443 int netfront_accelerator_suspend(struct netfront_info *np,
444 struct xenbus_device *dev)
446 - unsigned long flags;
449 netfront_accelerator_remove_watch(np);
450 @@ -690,11 +682,7 @@ int netfront_accelerator_suspend(struct
451 * Call the remove accelerator hook, but leave the vif_state
452 * on the accelerator's list in case there is a suspend_cancel.
454 - spin_lock_irqsave(&np->accelerator->vif_states_lock, flags);
456 - rc = do_remove(np, dev, &flags);
458 - spin_unlock_irqrestore(&np->accelerator->vif_states_lock, flags);
459 + rc = do_remove(np, dev);
461 mutex_unlock(&accelerator_mutex);
463 @@ -713,15 +701,16 @@ int netfront_accelerator_suspend_cancel(
464 netfront_accelerator_add_watch(np);
472 + * No lock pre-requisites. Takes the accelerator mutex
474 void netfront_accelerator_resume(struct netfront_info *np,
475 struct xenbus_device *dev)
477 struct netfront_accel_vif_state *accel_vif_state = NULL;
478 - spinlock_t *vif_states_lock;
479 - unsigned long flags;
482 mutex_lock(&accelerator_mutex);
484 /* Check that we've got a device that was accelerated */
485 @@ -734,9 +723,6 @@ void netfront_accelerator_resume(struct
486 if (accel_vif_state->dev == dev) {
487 BUG_ON(accel_vif_state != &np->accel_vif_state);
489 - vif_states_lock = &np->accelerator->vif_states_lock;
490 - spin_lock_irqsave(vif_states_lock, flags);
493 * Remove it from the accelerator's list so
494 * state is consistent for probing new vifs
495 @@ -744,9 +730,7 @@ void netfront_accelerator_resume(struct
497 list_del(&accel_vif_state->link);
498 np->accelerator = NULL;
500 - spin_unlock_irqrestore(vif_states_lock, flags);
506 @@ -757,11 +741,13 @@ void netfront_accelerator_resume(struct
511 + * No lock pre-requisites. Takes the vif_states_lock spinlock
513 int netfront_check_accelerator_queue_ready(struct net_device *dev,
514 struct netfront_info *np)
516 struct netfront_accelerator *accelerator;
517 - struct netfront_accel_hooks *hooks;
521 @@ -770,8 +756,8 @@ int netfront_check_accelerator_queue_rea
522 /* Call the check_ready accelerator hook. */
523 if (np->accel_vif_state.hooks && accelerator) {
524 spin_lock_irqsave(&accelerator->vif_states_lock, flags);
525 - hooks = np->accel_vif_state.hooks;
526 - if (hooks && np->accelerator == accelerator)
527 + if (np->accel_vif_state.hooks &&
528 + np->accelerator == accelerator)
529 rc = np->accel_vif_state.hooks->check_ready(dev);
530 spin_unlock_irqrestore(&accelerator->vif_states_lock, flags);
532 @@ -780,11 +766,13 @@ int netfront_check_accelerator_queue_rea
537 + * No lock pre-requisites. Takes the vif_states_lock spinlock
539 void netfront_accelerator_call_stop_napi_irq(struct netfront_info *np,
540 struct net_device *dev)
542 struct netfront_accelerator *accelerator;
543 - struct netfront_accel_hooks *hooks;
546 accelerator = np->accelerator;
547 @@ -792,19 +780,21 @@ void netfront_accelerator_call_stop_napi
548 /* Call the stop_napi_interrupts accelerator hook. */
549 if (np->accel_vif_state.hooks && accelerator != NULL) {
550 spin_lock_irqsave(&accelerator->vif_states_lock, flags);
551 - hooks = np->accel_vif_state.hooks;
552 - if (hooks && np->accelerator == accelerator)
553 + if (np->accel_vif_state.hooks &&
554 + np->accelerator == accelerator)
555 np->accel_vif_state.hooks->stop_napi_irq(dev);
556 spin_unlock_irqrestore(&accelerator->vif_states_lock, flags);
562 + * No lock pre-requisites. Takes the vif_states_lock spinlock
564 int netfront_accelerator_call_get_stats(struct netfront_info *np,
565 struct net_device *dev)
567 struct netfront_accelerator *accelerator;
568 - struct netfront_accel_hooks *hooks;
572 @@ -813,8 +803,8 @@ int netfront_accelerator_call_get_stats(
573 /* Call the get_stats accelerator hook. */
574 if (np->accel_vif_state.hooks && accelerator != NULL) {
575 spin_lock_irqsave(&accelerator->vif_states_lock, flags);
576 - hooks = np->accel_vif_state.hooks;
577 - if (hooks && np->accelerator == accelerator)
578 + if (np->accel_vif_state.hooks &&
579 + np->accelerator == accelerator)
580 rc = np->accel_vif_state.hooks->get_stats(dev,
582 spin_unlock_irqrestore(&accelerator->vif_states_lock, flags);
583 --- sle11-2009-04-09.orig/drivers/xen/netfront/netfront.c 2009-04-09 14:41:04.000000000 +0200
584 +++ sle11-2009-04-09/drivers/xen/netfront/netfront.c 2009-04-09 14:41:33.000000000 +0200
585 @@ -2238,10 +2238,9 @@ static void __exit netif_exit(void)
587 unregister_inetaddr_notifier(¬ifier_inetdev);
589 + xenbus_unregister_driver(&netfront_driver);
593 - return xenbus_unregister_driver(&netfront_driver);
595 module_exit(netif_exit);