src/patches/suse-2.6.27.25/patches.xen/848-sfc-vif-states-lock.patch

   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
   9 References: bnc#489109
  10 Patch-mainline: obsolete
  11
  12 Signed-off-by: Kieran Mansley <kmansley@solarflare.com>
  13 Acked-by: jbeulich@novell.com
  14
  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
  18   */
  19  static struct list_head accelerators_list;
  20
  21 -/* Lock to protect access to accelerators_list */
  22 -static spinlock_t accelerators_lock;
  23 -
  24  /* Workqueue to process acceleration configuration changes */
  25  struct workqueue_struct *accel_watch_workqueue;
  26
  27 @@ -69,7 +66,6 @@ DEFINE_MUTEX(accelerator_mutex);
  28  void netif_init_accel(void)
  29  {
  30         INIT_LIST_HEAD(&accelerators_list);
  31 -       spin_lock_init(&accelerators_lock);
  32
  33         accel_watch_workqueue = create_workqueue("net_accel");
  34  }
  35 @@ -77,13 +73,11 @@ void netif_init_accel(void)
  36  void netif_exit_accel(void)
  37  {
  38         struct netfront_accelerator *accelerator, *tmp;
  39 -       unsigned long flags;
  40
  41         flush_workqueue(accel_watch_workqueue);
  42         destroy_workqueue(accel_watch_workqueue);
  43
  44 -       spin_lock_irqsave(&accelerators_lock, flags);
  45 -
  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));
  49
  50 @@ -91,8 +85,6 @@ void netif_exit_accel(void)
  51                 kfree(accelerator->frontend);
  52                 kfree(accelerator);
  53         }
  54 -
  55 -       spin_unlock_irqrestore(&accelerators_lock, flags);
  56  }
  57
  58
  59 @@ -245,16 +237,12 @@ static int match_accelerator(const char
  60
  61  /*
  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
  65 + * mutex held.
  66   */
  67  static void add_accelerator_vif(struct netfront_accelerator *accelerator,
  68                                 struct netfront_info *np)
  69  {
  70 -       unsigned long flags;
  71 -
  72 -       /* Need lock to write list */
  73 -       spin_lock_irqsave(&accelerator->vif_states_lock, flags);
  74 -
  75         if (np->accelerator == NULL) {
  76                 np->accelerator = accelerator;
  77
  78 @@ -267,13 +255,13 @@ static void add_accelerator_vif(struct n
  79                  */
  80                 BUG_ON(np->accelerator != accelerator);
  81         }
  82 -
  83 -       spin_unlock_irqrestore(&accelerator->vif_states_lock, flags);
  84  }
  85
  86
  87  /*
  88   * Initialise the state to track an accelerator plugin module.
  89 + *
  90 + * Must be called with the accelerator mutex held.
  91   */
  92  static int init_accelerator(const char *frontend,
  93                             struct netfront_accelerator **result,
  94 @@ -281,7 +269,6 @@ static int init_accelerator(const char *
  95  {
  96         struct netfront_accelerator *accelerator =
  97                 kmalloc(sizeof(struct netfront_accelerator), GFP_KERNEL);
  98 -       unsigned long flags;
  99         int frontend_len;
 100
 101         if (!accelerator) {
 102 @@ -303,9 +290,7 @@ static int init_accelerator(const char *
 103
 104         accelerator->hooks = hooks;
 105
 106 -       spin_lock_irqsave(&accelerators_lock, flags);
 107         list_add(&accelerator->link, &accelerators_list);
 108 -       spin_unlock_irqrestore(&accelerators_lock, flags);
 109
 110         *result = accelerator;
 111
 112 @@ -316,11 +301,14 @@ static int init_accelerator(const char *
 113  /*
 114   * Modify the hooks stored in the per-vif state to match that in the
 115   * netfront accelerator's state.
 116 + *
 117 + * Takes the vif_states_lock spinlock and may sleep.
 118   */
 119  static void
 120  accelerator_set_vif_state_hooks(struct netfront_accel_vif_state *vif_state)
 121  {
 122 -       /* This function must be called with the vif_states_lock held */
 123 +       struct netfront_accelerator *accelerator;
 124 +       unsigned long flags;
 125
 126         DPRINTK("%p\n",vif_state);
 127
 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);
 131
 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);
 137
 138         netif_tx_unlock_bh(vif_state->np->netdev);
 139         netif_poll_enable(vif_state->np->netdev);
 140  }
 141
 142
 143 +/*
 144 + * Must be called with the accelerator mutex held.  Takes the
 145 + * vif_states_lock spinlock.
 146 + */
 147  static void accelerator_probe_new_vif(struct netfront_info *np,
 148                                       struct xenbus_device *dev,
 149                                       struct netfront_accelerator *accelerator)
 150  {
 151         struct netfront_accel_hooks *hooks;
 152 -       unsigned long flags;
 153
 154         DPRINTK("\n");
 155
 156 @@ -349,17 +343,8 @@ static void accelerator_probe_new_vif(st
 157
 158         hooks = accelerator->hooks;
 159
 160 -       if (hooks) {
 161 -               if (hooks->new_device(np->netdev, dev) == 0) {
 162 -                       spin_lock_irqsave
 163 -                               (&accelerator->vif_states_lock, flags);
 164 -
 165 -                       accelerator_set_vif_state_hooks(&np->accel_vif_state);
 166 -
 167 -                       spin_unlock_irqrestore
 168 -                               (&accelerator->vif_states_lock, flags);
 169 -               }
 170 -       }
 171 +       if (hooks && hooks->new_device(np->netdev, dev) == 0)
 172 +               accelerator_set_vif_state_hooks(&np->accel_vif_state);
 173
 174         return;
 175  }
 176 @@ -368,7 +353,10 @@ static void accelerator_probe_new_vif(st
 177  /*
 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.
 182 + *
 183 + * Must be called with accelerator_mutex held.  Takes the
 184 + * vif_states_lock spinlock.
 185   */
 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.
 192 + *
 193 + * Must be called with accelerator_mutex held.  Takes the
 194 + * vif_states_lock spinlock.
 195   */
 196  static void
 197  accelerator_probe_vifs(struct netfront_accelerator *accelerator,
 198                        struct netfront_accel_hooks *hooks)
 199  {
 200         struct netfront_accel_vif_state *vif_state, *tmp;
 201 -       unsigned long flags;
 202
 203         DPRINTK("%p\n", accelerator);
 204
 205 @@ -425,29 +415,22 @@ accelerator_probe_vifs(struct netfront_a
 206         BUG_ON(hooks == NULL);
 207         accelerator->hooks = hooks;
 208
 209 -       /*
 210 -        *  currently hold accelerator_mutex, so don't need
 211 -        *  vif_states_lock to read the list
 212 -        */
 213 +       /* Holds accelerator_mutex to iterate list */
 214         list_for_each_entry_safe(vif_state, tmp, &accelerator->vif_states,
 215                                  link) {
 216                 struct netfront_info *np = vif_state->np;
 217
 218 -               if (hooks->new_device(np->netdev, vif_state->dev) == 0) {
 219 -                       spin_lock_irqsave
 220 -                               (&accelerator->vif_states_lock, flags);
 221 -
 222 +               if (hooks->new_device(np->netdev, vif_state->dev) == 0)
 223                         accelerator_set_vif_state_hooks(vif_state);
 224 -
 225 -                       spin_unlock_irqrestore
 226 -                               (&accelerator->vif_states_lock, flags);
 227 -               }
 228         }
 229  }
 230
 231
 232  /*
 233 - * Called by the netfront accelerator plugin module when it has loaded
 234 + * Called by the netfront accelerator plugin module when it has
 235 + * loaded.
 236 + *
 237 + * Takes the accelerator_mutex and vif_states_lock spinlock.
 238   */
 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
 242
 243  /*
 244   * Remove the hooks from a single vif state.
 245 + *
 246 + * Takes the vif_states_lock spinlock and may sleep.
 247   */
 248  static void
 249  accelerator_remove_single_hook(struct netfront_accelerator *accelerator,
 250                                struct netfront_accel_vif_state *vif_state)
 251  {
 252 +       unsigned long flags;
 253 +
 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);
 257
 258 +       spin_lock_irqsave(&accelerator->vif_states_lock, flags);
 259 +
 260         /*
 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
 264          */
 265         vif_state->hooks = NULL;
 266
 267 +       spin_unlock_irqrestore(&accelerator->vif_states_lock, flags);
 268 +
 269         netif_tx_unlock_bh(vif_state->np->netdev);
 270         netif_poll_enable(vif_state->np->netdev);
 271  }
 272 @@ -527,25 +518,25 @@ accelerator_remove_single_hook(struct ne
 273
 274  /*
 275   * Safely remove the accelerator function hooks from a netfront state.
 276 + *
 277 + * Must be called with the accelerator mutex held.  Takes the
 278 + * vif_states_lock spinlock.
 279   */
 280  static void accelerator_remove_hooks(struct netfront_accelerator *accelerator)
 281  {
 282 -       struct netfront_accel_hooks *hooks;
 283         struct netfront_accel_vif_state *vif_state, *tmp;
 284         unsigned long flags;
 285
 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,
 290                                  link) {
 291 -               spin_lock_irqsave(&accelerator->vif_states_lock, flags);
 292 -
 293                 if(vif_state->hooks) {
 294 -                       hooks = vif_state->hooks;
 295 -
 296 +                       spin_lock_irqsave(&accelerator->vif_states_lock, flags);
 297 +
 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);
 303
 304                         spin_unlock_irqrestore(&accelerator->vif_states_lock,
 305                                                flags);
 306 @@ -553,9 +544,6 @@ static void accelerator_remove_hooks(str
 307                         accelerator_remove_single_hook(accelerator, vif_state);
 308
 309                         accelerator->hooks->remove(vif_state->dev);
 310 -               } else {
 311 -                       spin_unlock_irqrestore(&accelerator->vif_states_lock,
 312 -                                              flags);
 313                 }
 314         }
 315
 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.
 320 + *
 321 + * Takes the accelerator mutex, and vif_states_lock spinlock.
 322   */
 323  void netfront_accelerator_stop(const char *frontend)
 324  {
 325         struct netfront_accelerator *accelerator;
 326 -       unsigned long flags;
 327
 328         mutex_lock(&accelerator_mutex);
 329 -       spin_lock_irqsave(&accelerators_lock, flags);
 330
 331         list_for_each_entry(accelerator, &accelerators_list, link) {
 332                 if (match_accelerator(frontend, accelerator)) {
 333 -                       spin_unlock_irqrestore(&accelerators_lock, flags);
 334 -
 335                         accelerator_remove_hooks(accelerator);
 336 -
 337                         goto out;
 338                 }
 339         }
 340 -       spin_unlock_irqrestore(&accelerators_lock, flags);
 341   out:
 342         mutex_unlock(&accelerator_mutex);
 343  }
 344  EXPORT_SYMBOL_GPL(netfront_accelerator_stop);
 345
 346
 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)
 350 +/*
 351 + * Helper for call_remove and do_suspend
 352 + *
 353 + * Must be called with the accelerator mutex held.  Takes the
 354 + * vif_states_lock spinlock.
 355 + */
 356 +static int do_remove(struct netfront_info *np, struct xenbus_device *dev)
 357  {
 358         struct netfront_accelerator *accelerator = np->accelerator;
 359 -       struct netfront_accel_hooks *hooks;
 360 +       unsigned long flags;
 361         int rc = 0;
 362
 363         if (np->accel_vif_state.hooks) {
 364 -               hooks = np->accel_vif_state.hooks;
 365 +               spin_lock_irqsave(&accelerator->vif_states_lock, flags);
 366
 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);
 370
 371                 spin_unlock_irqrestore(&accelerator->vif_states_lock,
 372 -                                      *lock_flags);
 373 +                                      flags);
 374
 375                 /*
 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);
 379
 380                 rc = accelerator->hooks->remove(dev);
 381 -
 382 -               spin_lock_irqsave(&accelerator->vif_states_lock, *lock_flags);
 383         }
 384
 385         return rc;
 386  }
 387
 388
 389 +/*
 390 + * Must be called with the accelerator mutex held.  Takes the
 391 + * vif_states_lock spinlock
 392 + */
 393  static int netfront_remove_accelerator(struct netfront_info *np,
 394                                        struct xenbus_device *dev)
 395  {
 396         struct netfront_accelerator *accelerator;
 397         struct netfront_accel_vif_state *tmp_vif_state;
 398 -       unsigned long flags;
 399         int rc = 0;
 400
 401         /* Check that we've got a device that was accelerated */
 402 @@ -640,8 +629,6 @@ static int netfront_remove_accelerator(s
 403
 404         accelerator = np->accelerator;
 405
 406 -       spin_lock_irqsave(&accelerator->vif_states_lock, flags);
 407 -
 408         list_for_each_entry(tmp_vif_state, &accelerator->vif_states,
 409                             link) {
 410                 if (tmp_vif_state == &np->accel_vif_state) {
 411 @@ -650,16 +637,18 @@ static int netfront_remove_accelerator(s
 412                 }
 413         }
 414
 415 -       rc = do_remove(np, dev, &flags);
 416 +       rc = do_remove(np, dev);
 417
 418         np->accelerator = NULL;
 419
 420 -       spin_unlock_irqrestore(&accelerator->vif_states_lock, flags);
 421 -
 422         return rc;
 423  }
 424
 425
 426 +/*
 427 + * No lock pre-requisites.  Takes the accelerator mutex and the
 428 + * vif_states_lock spinlock.
 429 + */
 430  int netfront_accelerator_call_remove(struct netfront_info *np,
 431                                      struct xenbus_device *dev)
 432  {
 433 @@ -671,11 +660,14 @@ int netfront_accelerator_call_remove(str
 434         return rc;
 435  }
 436
 437 -
 438 +
 439 +/*
 440 + * No lock pre-requisites.  Takes the accelerator mutex and the
 441 + * vif_states_lock spinlock.
 442 + */
 443  int netfront_accelerator_suspend(struct netfront_info *np,
 444                                  struct xenbus_device *dev)
 445  {
 446 -       unsigned long flags;
 447         int rc = 0;
 448
 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.
 453          */
 454 -       spin_lock_irqsave(&np->accelerator->vif_states_lock, flags);
 455 -
 456 -       rc = do_remove(np, dev, &flags);
 457 -
 458 -       spin_unlock_irqrestore(&np->accelerator->vif_states_lock, flags);
 459 +       rc = do_remove(np, dev);
 460   out:
 461         mutex_unlock(&accelerator_mutex);
 462         return rc;
 463 @@ -713,15 +701,16 @@ int netfront_accelerator_suspend_cancel(
 464                 netfront_accelerator_add_watch(np);
 465         return 0;
 466  }
 467 -
 468 -
 469 +
 470 +
 471 +/*
 472 + * No lock pre-requisites.  Takes the accelerator mutex
 473 + */
 474  void netfront_accelerator_resume(struct netfront_info *np,
 475                                  struct xenbus_device *dev)
 476  {
 477         struct netfront_accel_vif_state *accel_vif_state = NULL;
 478 -       spinlock_t *vif_states_lock;
 479 -       unsigned long flags;
 480 -
 481 +
 482         mutex_lock(&accelerator_mutex);
 483
 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);
 488
 489 -                       vif_states_lock = &np->accelerator->vif_states_lock;
 490 -                       spin_lock_irqsave(vif_states_lock, flags);
 491 -
 492                         /*
 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
 496                          */
 497                         list_del(&accel_vif_state->link);
 498                         np->accelerator = NULL;
 499 -
 500 -                       spin_unlock_irqrestore(vif_states_lock, flags);
 501 -
 502 +
 503                         break;
 504                 }
 505         }
 506 @@ -757,11 +741,13 @@ void netfront_accelerator_resume(struct
 507  }
 508
 509
 510 +/*
 511 + * No lock pre-requisites.  Takes the vif_states_lock spinlock
 512 + */
 513  int netfront_check_accelerator_queue_ready(struct net_device *dev,
 514                                            struct netfront_info *np)
 515  {
 516         struct netfront_accelerator *accelerator;
 517 -       struct netfront_accel_hooks *hooks;
 518         int rc = 1;
 519         unsigned long flags;
 520
 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);
 531         }
 532 @@ -780,11 +766,13 @@ int netfront_check_accelerator_queue_rea
 533  }
 534
 535
 536 +/*
 537 + * No lock pre-requisites.  Takes the vif_states_lock spinlock
 538 + */
 539  void netfront_accelerator_call_stop_napi_irq(struct netfront_info *np,
 540                                              struct net_device *dev)
 541  {
 542         struct netfront_accelerator *accelerator;
 543 -       struct netfront_accel_hooks *hooks;
 544         unsigned long flags;
 545
 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);
 557         }
 558  }
 559
 560
 561 +/*
 562 + * No lock pre-requisites.  Takes the vif_states_lock spinlock
 563 + */
 564  int netfront_accelerator_call_get_stats(struct netfront_info *np,
 565                                         struct net_device *dev)
 566  {
 567         struct netfront_accelerator *accelerator;
 568 -       struct netfront_accel_hooks *hooks;
 569         unsigned long flags;
 570         int rc = 0;
 571
 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,
 581                                                                   &np->stats);
 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)
 586  #ifdef CONFIG_INET
 587         unregister_inetaddr_notifier(&notifier_inetdev);
 588  #endif
 589 +       xenbus_unregister_driver(&netfront_driver);
 590
 591         netif_exit_accel();
 592 -
 593 -       return xenbus_unregister_driver(&netfront_driver);
 594  }
 595  module_exit(netif_exit);
 596