WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [Patch 7/8] Netfront accelerator bug fixes

To: xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-devel] [Patch 7/8] Netfront accelerator bug fixes
From: Kieran Mansley <kmansley@xxxxxxxxxxxxxx>
Date: Tue, 30 Oct 2007 17:08:02 +0000
Delivery-date: Tue, 30 Oct 2007 10:13:38 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
simplify locking in netfront accelerator

Signed-off-by <kmansley@xxxxxxxxxxxxxx>

diff -r c75337e70043 drivers/xen/netfront/accel.c
--- a/drivers/xen/netfront/accel.c      Tue Oct 30 13:26:47 2007 +0000
+++ b/drivers/xen/netfront/accel.c      Tue Oct 30 16:31:38 2007 +0000
@@ -31,7 +31,7 @@
 #include <linux/netdevice.h>
 #include <linux/skbuff.h>
 #include <linux/list.h>
-#include <linux/kref.h>
+#include <linux/mutex.h>
 
 #include <xen/xenbus.h>
 
@@ -45,29 +45,17 @@
 #define WPRINTK(fmt, args...)                          \
        printk(KERN_WARNING "netfront/accel: " fmt, ##args)
 
-#if LINUX_VERSION_CODE <= KERNEL_VERSION(2, 6, 5)
-#define kref_init(x,y) kref_init(x,y)
-#define kref_put(x,y)  kref_put(x)
-#else
-#define kref_init(x,y) kref_init(x)
-#define kref_put(x,y)  kref_put(x,y)
-#endif
-
 /*
  * List of all netfront accelerator plugin modules available.  Each
  * list entry is of type struct netfront_accelerator.
  */ 
 static struct list_head accelerators_list;
 
-/*
- * Lock to protect access to accelerators_list
- */
+/* Lock to protect access to accelerators_list */
 static spinlock_t accelerators_lock;
 
-/* Forward declaration of kref cleanup functions */
-static void accel_kref_release(struct kref *ref);
-static void vif_kref_release(struct kref *ref);
-
+/* Mutex to prevent concurrent loads and suspends, etc. */
+DEFINE_MUTEX(accelerator_mutex);
 
 void netif_init_accel(void)
 {
@@ -105,9 +93,6 @@ void init_accelerator_vif(struct netfron
        /* It's assumed that these things don't change */
        np->accel_vif_state.np = np;
        np->accel_vif_state.dev = dev;
-
-       np->accel_vif_state.ready_for_probe = 1;
-       np->accel_vif_state.need_probe = NULL;
 }
 
 
@@ -130,6 +115,11 @@ static void add_accelerator_vif(struct n
 static void add_accelerator_vif(struct netfront_accelerator *accelerator,
                                struct netfront_info *np)
 {
+       unsigned flags;
+
+       /* Need lock to write list */
+       spin_lock_irqsave(&accelerator->vif_states_lock, flags);
+
        if (np->accelerator == NULL) {
                np->accelerator = accelerator;
                
@@ -142,6 +132,8 @@ static void add_accelerator_vif(struct n
                 */
                BUG_ON(np->accelerator != accelerator);
        }
+
+       spin_unlock_irqrestore(&accelerator->vif_states_lock, flags);
 }
 
 
@@ -175,9 +167,6 @@ static int init_accelerator(const char *
 
        accelerator->hooks = hooks;
 
-       accelerator->ready_for_probe = 1;
-       accelerator->need_probe = NULL;
-
        list_add(&accelerator->link, &accelerators_list);
 
        *result = accelerator;
@@ -193,17 +182,9 @@ static void
 static void 
 accelerator_set_vif_state_hooks(struct netfront_accel_vif_state *vif_state)
 {
-       /* This function must be called with the vif_state_lock held */
+       /* This function must be called with the vif_states_lock held */
 
        DPRINTK("%p\n",vif_state);
-
-       /*
-        * Take references to stop hooks disappearing.
-        * This persists until vif_kref gets to zero.
-        */
-       kref_get(&vif_state->np->accelerator->accel_kref);
-       /* This persists until vif_state->hooks are cleared */
-       kref_init(&vif_state->vif_kref, vif_kref_release);
 
        /* Make sure there are no data path operations going on */
        netif_poll_disable(vif_state->np->netdev);
@@ -221,47 +202,22 @@ static void accelerator_probe_new_vif(st
                                      struct netfront_accelerator *accelerator)
 {
        struct netfront_accel_hooks *hooks;
-       unsigned flags;
-       
+
        DPRINTK("\n");
 
-       spin_lock_irqsave(&accelerator->vif_states_lock, flags);
-       
-       /*
-        * Include this frontend device on the accelerator's list
-        */
+       /* Include this frontend device on the accelerator's list */
        add_accelerator_vif(accelerator, np);
        
        hooks = accelerator->hooks;
        
        if (hooks) {
-               if (np->accel_vif_state.ready_for_probe) {
-                       np->accel_vif_state.ready_for_probe = 0;
-                       
-                       kref_get(&accelerator->accel_kref);
-                       
-                       spin_unlock_irqrestore(&accelerator->vif_states_lock,
-                                              flags);
-                       
-                       hooks->new_device(np->netdev, dev);
-                       
-                       kref_put(&accelerator->accel_kref,
-                                accel_kref_release);
-                       /* 
-                        * Hooks will get linked into vif_state by a
-                        * future call by the accelerator to
-                        * netfront_accelerator_ready()
-                        */
-                       return;
-               } else {
-                       if (np->accel_vif_state.need_probe != NULL)
-                               DPRINTK("Probe request on vif awaiting 
probe\n");
-                       np->accel_vif_state.need_probe = hooks;
-               }
-       }
-               
-       spin_unlock_irqrestore(&accelerator->vif_states_lock,
-                              flags);
+               hooks->new_device(np->netdev, dev);
+               /* 
+                * Hooks will get linked into vif_state by a future
+                * call by the accelerator to netfront_accelerator_ready()
+                */
+       }
+
        return;
 }
 
@@ -275,10 +231,12 @@ int netfront_load_accelerator(struct net
                              const char *frontend)
 {
        struct netfront_accelerator *accelerator;
-       int rc;
+       int rc = 0;
        unsigned flags;
 
        DPRINTK(" %s\n", frontend);
+
+       mutex_lock(&accelerator_mutex);
 
        spin_lock_irqsave(&accelerators_lock, flags);
 
@@ -292,6 +250,7 @@ int netfront_load_accelerator(struct net
 
                        accelerator_probe_new_vif(np, dev, accelerator);
 
+                       mutex_unlock(&accelerator_mutex);
                        return 0;
                }
        }
@@ -299,15 +258,16 @@ int netfront_load_accelerator(struct net
        /* Couldn't find it, so create a new one and load the module */
        if ((rc = init_accelerator(frontend, &accelerator, NULL)) < 0) {
                spin_unlock_irqrestore(&accelerators_lock, flags);
+               mutex_unlock(&accelerator_mutex);
                return rc;
        }
 
        spin_unlock_irqrestore(&accelerators_lock, flags);
 
        /* Include this frontend device on the accelerator's list */
-       spin_lock_irqsave(&accelerator->vif_states_lock, flags);
        add_accelerator_vif(accelerator, np);
-       spin_unlock_irqrestore(&accelerator->vif_states_lock, flags);
+
+       mutex_unlock(&accelerator_mutex);
 
        DPRINTK("requesting module %s\n", frontend);
 
@@ -319,7 +279,7 @@ int netfront_load_accelerator(struct net
         * it's up and running, and we can continue from there 
         */
 
-       return 0;
+       return rc;
 }
 
 
@@ -331,21 +291,11 @@ int netfront_load_accelerator(struct net
  */
 static void 
 accelerator_probe_vifs(struct netfront_accelerator *accelerator,
-                      struct netfront_accel_hooks *hooks,
-                      unsigned *lock_flags)
+                      struct netfront_accel_hooks *hooks)
 {
        struct netfront_accel_vif_state *vif_state, *tmp;
 
-       /* Calling function must have taken the vif_states_lock */
-
        DPRINTK("%p\n", accelerator);
-
-       /* 
-        * kref_init() takes a single reference to the hooks that will
-        * persist until the accelerator hooks are removed (e.g. by
-        * accelerator module unload)
-        */
-       kref_init(&accelerator->accel_kref, accel_kref_release);
 
        /* 
         * Store the hooks for future calls to probe a new device, and
@@ -354,72 +304,23 @@ accelerator_probe_vifs(struct netfront_a
         */
        BUG_ON(hooks == NULL);
        accelerator->hooks = hooks;
-       
+
+       /* 
+        *  currently hold accelerator_mutex, so don't need
+        *  vif_states_lock to read the list
+        */
        list_for_each_entry_safe(vif_state, tmp, &accelerator->vif_states,
                                 link) {
                struct netfront_info *np = vif_state->np;
-
-               if (vif_state->ready_for_probe) {
-                       vif_state->ready_for_probe = 0;
-                       kref_get(&accelerator->accel_kref);
-
-                       /* 
-                        * drop lock before calling hook.  hooks are
-                        * protected by the kref
-                        */
-                       spin_unlock_irqrestore(&accelerator->vif_states_lock,
-                                              (*lock_flags));
-                       
-                       hooks->new_device(np->netdev, vif_state->dev);
-                       
-                       kref_put(&accelerator->accel_kref, accel_kref_release);
-
-                       /* Retake lock for next go round the loop */
-                       spin_lock_irqsave(&accelerator->vif_states_lock,
-                                         (*lock_flags));
-                       
-                       /*
-                        * Hooks will get linked into vif_state by a call to
-                        * netfront_accelerator_ready() once accelerator
-                        * plugin is ready for action
-                        */
-               } else {
-                       if (vif_state->need_probe != NULL)
-                               DPRINTK("Probe request on vif awaiting 
probe\n");
-                       vif_state->need_probe = hooks;
-               }
-       }
-       
-       /* Return with vif_states_lock held, as on entry */
-}
-
-
-/* 
- * Wrapper for accelerator_probe_vifs that checks now is a good time
- * to do the probe, and postpones till previous state cleared up if
- * necessary
- */
-static void 
-accelerator_probe_vifs_on_load(struct netfront_accelerator *accelerator,
-                              struct netfront_accel_hooks *hooks)
-{
-       unsigned flags;
-
-       DPRINTK("\n");
-
-       spin_lock_irqsave(&accelerator->vif_states_lock, flags);
-       
-       if (accelerator->ready_for_probe) {
-               accelerator->ready_for_probe = 0;
-               accelerator_probe_vifs(accelerator, hooks, &flags);
-       } else {
-               if (accelerator->need_probe)
-                       DPRINTK("Probe request on accelerator awaiting 
probe\n");
-               accelerator->need_probe = hooks;
-       }
-
-       spin_unlock_irqrestore(&accelerator->vif_states_lock,
-                              flags);
+               
+               hooks->new_device(np->netdev, vif_state->dev);
+               
+               /*
+                * Hooks will get linked into vif_state by a call to
+                * netfront_accelerator_ready() once accelerator
+                * plugin is ready for action
+                */
+       }
 }
 
 
@@ -447,6 +348,8 @@ int netfront_accelerator_loaded(int vers
                }
        }
 
+       mutex_lock(&accelerator_mutex);
+
        spin_lock_irqsave(&accelerators_lock, flags);
 
        /* 
@@ -457,9 +360,9 @@ int netfront_accelerator_loaded(int vers
                if (match_accelerator(frontend, accelerator)) {
                        spin_unlock_irqrestore(&accelerators_lock, flags);
 
-                       accelerator_probe_vifs_on_load(accelerator, hooks);
-
-                       return 0;
+                       accelerator_probe_vifs(accelerator, hooks);
+
+                       goto out;
                }
        }
 
@@ -474,6 +377,8 @@ int netfront_accelerator_loaded(int vers
 
        spin_unlock_irqrestore(&accelerators_lock, flags);
 
+ out:
+       mutex_unlock(&accelerator_mutex);
        return 0;
 }
 EXPORT_SYMBOL_GPL(netfront_accelerator_loaded);
@@ -497,6 +402,10 @@ void netfront_accelerator_ready(const ch
 
        list_for_each_entry(accelerator, &accelerators_list, link) {
                if (match_accelerator(frontend, accelerator)) {
+                       /* 
+                        * Mutex not held so need vif_states_lock for
+                        * list
+                        */
                        spin_lock_irqsave
                                (&accelerator->vif_states_lock, flags1);
 
@@ -509,11 +418,9 @@ void netfront_accelerator_ready(const ch
 
                        spin_unlock_irqrestore
                                (&accelerator->vif_states_lock, flags1);
-                       goto done;
+                       break;
                }
        }
-
- done:
        spin_unlock_irqrestore(&accelerators_lock, flags);
 }
 EXPORT_SYMBOL_GPL(netfront_accelerator_ready);
@@ -552,38 +459,24 @@ static void accelerator_remove_hooks(str
        struct netfront_accel_vif_state *vif_state, *tmp;
        unsigned flags;
 
-       spin_lock_irqsave(&accelerator->vif_states_lock, flags);
-
+       /* Mutex is held so don't need vif_states_lock to iterate list */
        list_for_each_entry_safe(vif_state, tmp,
                                 &accelerator->vif_states,
                                 link) {
+               spin_lock_irqsave(&accelerator->vif_states_lock, flags);
+
                BUG_ON(vif_state->hooks == NULL);
                hooks = vif_state->hooks;
                accelerator_remove_single_hook(accelerator, vif_state);
 
-               /* 
-                * Tell accelerator that this device is being removed,
-                * and remove the reference taken when the vif_state
-                * hooks were set; both must be called without lock
-                * held
-                */
                spin_unlock_irqrestore(&accelerator->vif_states_lock, flags);
 
                /* Last chance to get statistics from the accelerator */
                hooks->get_stats(vif_state->np->netdev, &vif_state->np->stats);
                hooks->remove(vif_state->dev);
-
-               kref_put(&vif_state->vif_kref, vif_kref_release);
-
-               spin_lock_irqsave(&accelerator->vif_states_lock, flags);
        }
        
        accelerator->hooks = NULL;
-
-       spin_unlock_irqrestore(&accelerator->vif_states_lock, flags);
-
-       /* Remove the reference taken when module loaded */ 
-       kref_put(&accelerator->accel_kref, accel_kref_release);
 }
 
 
@@ -597,82 +490,33 @@ void netfront_accelerator_stop(const cha
        struct netfront_accelerator *accelerator;
        unsigned flags;
 
+       mutex_lock(&accelerator_mutex);
        spin_lock_irqsave(&accelerators_lock, flags);
 
        list_for_each_entry(accelerator, &accelerators_list, link) {
                if (match_accelerator(frontend, accelerator)) {
                        spin_unlock_irqrestore(&accelerators_lock, flags);
 
-                       /* 
-                        * Use semaphore to ensure we know when all
-                        * uses of hooks are complete
-                        */
-                       sema_init(&accelerator->exit_semaphore, 0);
-
                        accelerator_remove_hooks(accelerator);
 
-                       /* Wait for hooks to be unused, then return */
-                       down(&accelerator->exit_semaphore);
-                       
-                       return;
+                       goto out;
                }
        }
        spin_unlock_irqrestore(&accelerators_lock, flags);
+ out:
+       mutex_unlock(&accelerator_mutex);
 }
 EXPORT_SYMBOL_GPL(netfront_accelerator_stop);
-
-
-
-int netfront_check_accelerator_queue_ready(struct net_device *dev,
-                                          struct netfront_info *np)
-{
-       struct netfront_accelerator *accelerator;
-       struct netfront_accel_hooks *hooks;
-       int rc = 1;
-       unsigned flags;
-
-       accelerator = np->accelerator;
-
-       /*
-        * Call the check ready accelerator hook. The use count for the
-        * accelerator's hooks is incremented for the duration of the
-        * call to prevent the accelerator being able to modify the
-        * hooks in the middle (by, for example, unloading)
-        */ 
-       if (np->accel_vif_state.hooks && accelerator) {
-               spin_lock_irqsave(&accelerator->vif_states_lock, flags); 
-               hooks = np->accel_vif_state.hooks;
-               if (hooks && np->accelerator == accelerator) {
-                       kref_get(&np->accel_vif_state.vif_kref);
-                       spin_unlock_irqrestore
-                               (&accelerator->vif_states_lock, flags);
-
-                       rc = np->accel_vif_state.hooks->check_ready(dev);
-                       
-                       kref_put(&np->accel_vif_state.vif_kref,
-                                vif_kref_release);
-               } else {
-                       spin_unlock_irqrestore
-                               (&accelerator->vif_states_lock, flags);
-               }
-       }
-
-       return rc;
-}
 
 
 /* Helper for call_remove and do_suspend */
 static int do_remove(struct netfront_info *np, struct xenbus_device *dev,
-                    unsigned lock_flags, int clear_accelerator)
+                    unsigned *lock_flags)
 {
        struct netfront_accelerator *accelerator = np->accelerator;
        struct netfront_accel_hooks *hooks;
-       unsigned flags;
        int rc = 0;
  
-       if(clear_accelerator)
-               np->accelerator = NULL;
-
        if (np->accel_vif_state.hooks) {
                hooks = np->accel_vif_state.hooks;
 
@@ -684,27 +528,15 @@ static int do_remove(struct netfront_inf
                accelerator_remove_single_hook(accelerator, 
                                               &np->accel_vif_state);
 
-               spin_unlock_irqrestore(&accelerator->vif_states_lock, 
-                                      lock_flags);
-
-               /* 
-                * No need to do kref_get before calling these hooks as we
-                * can borrow the one we have to drop after this call 
-                */ 
-
                /* Last chance to get statistics from the accelerator */
                hooks->get_stats(np->netdev, &np->stats);
 
+               spin_unlock_irqrestore(&accelerator->vif_states_lock, 
+                                      *lock_flags);
+
                rc = hooks->remove(dev);
-               
-               /* 
-                * Remove the reference taken when the vif_state hooks
-                * were set, must be called without lock held
-                */
-               kref_put(&np->accel_vif_state.vif_kref, vif_kref_release);
-       } else {
-               spin_unlock_irqrestore(&accelerator->vif_states_lock, 
-                                      lock_flags);     
+
+               spin_lock_irqsave(&accelerator->vif_states_lock, *lock_flags);
        }
 
  
@@ -715,16 +547,22 @@ int netfront_accelerator_call_remove(str
 int netfront_accelerator_call_remove(struct netfront_info *np,
                                     struct xenbus_device *dev)
 {
+       struct netfront_accelerator *accelerator;
        struct netfront_accel_vif_state *tmp_vif_state;
        unsigned flags;
+       int rc = 0; 
+
+       mutex_lock(&accelerator_mutex);
 
        /* Check that we've got a device that was accelerated */
        if (np->accelerator == NULL)
-               return 0;
-
-       spin_lock_irqsave(&np->accelerator->vif_states_lock, flags); 
-
-       list_for_each_entry(tmp_vif_state, &np->accelerator->vif_states,
+               goto out;
+
+       accelerator = np->accelerator;
+
+       spin_lock_irqsave(&accelerator->vif_states_lock, flags); 
+
+       list_for_each_entry(tmp_vif_state, &accelerator->vif_states,
                            link) {
                if (tmp_vif_state == &np->accel_vif_state) {
                        list_del(&np->accel_vif_state.link);
@@ -732,8 +570,14 @@ int netfront_accelerator_call_remove(str
                }
        }
 
-       /* do_remove drops the lock for us */
-       return do_remove(np, dev, flags, 1);
+       rc = do_remove(np, dev, &flags);
+
+       np->accelerator = NULL;
+
+       spin_unlock_irqrestore(&accelerator->vif_states_lock, flags); 
+ out:
+       mutex_unlock(&accelerator_mutex);
+       return rc;
 }
   
   
@@ -741,21 +585,26 @@ int netfront_accelerator_suspend(struct 
                                 struct xenbus_device *dev)
 {
        unsigned flags;
-
+       int rc = 0;
+
+       mutex_lock(&accelerator_mutex);
 
        /* Check that we've got a device that was accelerated */
        if (np->accelerator == NULL)
-               return 0;
+               goto out;
 
        /* 
         * Call the remove accelerator hook, but leave the vif_state
         * on the accelerator's list in case there is a suspend_cancel.
         */
-       spin_lock_irqsave(&np->accelerator->vif_states_lock,
-                         flags); 
+       spin_lock_irqsave(&np->accelerator->vif_states_lock, flags); 
        
-       /* do_remove drops the lock for us */
-       return do_remove(np, dev, flags, 0);
+       rc = do_remove(np, dev, &flags);
+
+       spin_unlock_irqrestore(&np->accelerator->vif_states_lock, flags); 
+ out:
+       mutex_unlock(&accelerator_mutex);
+       return rc;
 }
   
   
@@ -763,21 +612,18 @@ int netfront_accelerator_suspend_cancel(
                                        struct xenbus_device *dev)
 {
        struct netfront_accel_vif_state *accel_vif_state = NULL;
-       unsigned flags1, flags2;
- 
+ 
+       mutex_lock(&accelerator_mutex);
+
        /* Check that we've got a device that was accelerated */
        if (np->accelerator == NULL)
-               return 0;
-
-       spin_lock_irqsave(&np->accelerator->vif_states_lock, flags); 
+               goto out;
+
        /* Find the vif_state from the accelerator's list */
        list_for_each_entry(accel_vif_state, &np->accelerator->vif_states,
                            link) {
                if (accel_vif_state->dev == dev) {
                        BUG_ON(accel_vif_state != &np->accel_vif_state);
-                       
-                       
spin_unlock_irqrestore(&np->accelerator->vif_states_lock,
-                                              flags); 
  
                        /*
                         * Kick things off again to restore
@@ -785,12 +631,12 @@ int netfront_accelerator_suspend_cancel(
                         */
                        accelerator_probe_new_vif(np, dev, np->accelerator);
  
-                       return 0;
+                       break;
                }
        }
        
-       spin_unlock_irqrestore(&np->accelerator->vif_states_lock, flags); 
- 
+ out:
+       mutex_unlock(&accelerator_mutex);
        return 0;
 }
  
@@ -802,17 +648,20 @@ void netfront_accelerator_resume(struct 
        spinlock_t *vif_states_lock;
        unsigned flags;
  
-       /* Check that we've got a device that was accelerated */
+       mutex_lock(&accelerator_mutex);
+
+       /* Check that we've got a device that was accelerated */
        if(np->accelerator == NULL)
-               return;
- 
-       spin_lock_irqsave(&np->accelerator->vif_states_lock, flags); 
- 
+               goto out;
+
        /* Find the vif_state from the accelerator's list */
        list_for_each_entry(accel_vif_state, &np->accelerator->vif_states, 
                            link) {
                if (accel_vif_state->dev == dev) {
                        BUG_ON(accel_vif_state != &np->accel_vif_state);
+ 
+                       vif_states_lock = &np->accelerator->vif_states_lock;
+                       spin_lock_irqsave(vif_states_lock, flags); 
  
                        /* 
                         * Remove it from the accelerator's list so
@@ -820,15 +669,40 @@ void netfront_accelerator_resume(struct 
                         * when they get connected
                         */
                        list_del(&accel_vif_state->link);
-                       vif_states_lock = &np->accelerator->vif_states_lock;
                        np->accelerator = NULL;
  
                        spin_unlock_irqrestore(vif_states_lock, flags); 
                        
-                       return;
+                       break;
                }
        }
-       spin_unlock_irqrestore(&np->accelerator->vif_states_lock, flags); 
+
+ out:
+       mutex_unlock(&accelerator_mutex);
+       return;
+}
+
+
+int netfront_check_accelerator_queue_ready(struct net_device *dev,
+                                          struct netfront_info *np)
+{
+       struct netfront_accelerator *accelerator;
+       struct netfront_accel_hooks *hooks;
+       int rc = 1;
+       unsigned flags;
+
+       accelerator = np->accelerator;
+
+       /* Call the check_ready accelerator hook. */ 
+       if (np->accel_vif_state.hooks && accelerator) {
+               spin_lock_irqsave(&accelerator->vif_states_lock, flags); 
+               hooks = np->accel_vif_state.hooks;
+               if (hooks && np->accelerator == accelerator)
+                       rc = np->accel_vif_state.hooks->check_ready(dev);
+               spin_unlock_irqrestore(&accelerator->vif_states_lock, flags);
+       }
+
+       return rc;
 }
 
 
@@ -841,30 +715,13 @@ void netfront_accelerator_call_stop_napi
 
        accelerator = np->accelerator;
 
-       /* 
-        * Call the stop_napi_interrupts accelerator hook.  The use
-        * count for the accelerator's hooks is incremented for the
-        * duration of the call to prevent the accelerator being able
-        * to modify the hooks in the middle (by, for example,
-        * unloading)
-        */
-
+       /* Call the stop_napi_interrupts accelerator hook. */
        if (np->accel_vif_state.hooks && accelerator != NULL) {
                spin_lock_irqsave(&accelerator->vif_states_lock, flags); 
                hooks = np->accel_vif_state.hooks;
-               if (hooks && np->accelerator == accelerator) {
-                       kref_get(&np->accel_vif_state.vif_kref);
-                       spin_unlock_irqrestore
-                               (&accelerator->vif_states_lock, flags);
-
+               if (hooks && np->accelerator == accelerator)
                        np->accel_vif_state.hooks->stop_napi_irq(dev);
-               
-                       kref_put(&np->accel_vif_state.vif_kref,
-                                vif_kref_release);
-               } else {
-                       spin_unlock_irqrestore
-                               (&accelerator->vif_states_lock, flags);
-               }
+               spin_unlock_irqrestore(&accelerator->vif_states_lock, flags);
        }
 }
 
@@ -879,90 +736,15 @@ int netfront_accelerator_call_get_stats(
 
        accelerator = np->accelerator;
 
-       /* 
-        * Call the get_stats accelerator hook.  The use count for the
-        * accelerator's hooks is incremented for the duration of the
-        * call to prevent the accelerator being able to modify the
-        * hooks in the middle (by, for example, unloading)
-        */
-
+       /* Call the get_stats accelerator hook. */
        if (np->accel_vif_state.hooks && accelerator != NULL) {
-               spin_lock_irqsave(accelerator->vif_states_lock, flags); 
+               spin_lock_irqsave(&accelerator->vif_states_lock, flags); 
                hooks = np->accel_vif_state.hooks;
-               if (hooks && np->accelerator == accelerator) {
-                       kref_get(&np->accel_vif_state.vif_kref);
-                       spin_unlock_irqrestore
-                               (&accelerator->vif_states_lock, flags);
-
+               if (hooks && np->accelerator == accelerator)
                        rc = np->accel_vif_state.hooks->get_stats(dev,
                                                                  &np->stats);
-               
-                       kref_put(&np->accel_vif_state.vif_kref,
-                                vif_kref_release);
-               } else {
-                       spin_unlock_irqrestore
-                               (&accelerator->vif_states_lock, flags);
-               }
+               spin_unlock_irqrestore(&accelerator->vif_states_lock, flags);
        }
        return rc;
 }
 
-
-/* 
- * Once all users of hooks have kref_put()'d we can signal that it's
- * safe to unload
- */ 
-static void accel_kref_release(struct kref *ref)
-{
-       struct netfront_accelerator *accelerator =
-               container_of(ref, struct netfront_accelerator, accel_kref);
-       struct netfront_accel_hooks *hooks;
-       unsigned flags;
-
-       DPRINTK("%p\n", accelerator);
-
-       /* Signal that all users of hooks are done */
-       up(&accelerator->exit_semaphore);
-
-       spin_lock_irqsave(&accelerator->vif_states_lock, flags);
-       if (accelerator->need_probe) {
-               hooks = accelerator->need_probe;
-               accelerator->need_probe = NULL;
-               accelerator_probe_vifs(accelerator, hooks, &flags);
-       } 
-       else
-               accelerator->ready_for_probe = 1;
-
-       spin_unlock_irqrestore(&accelerator->vif_states_lock, flags);
-}
-
-
-static void vif_kref_release(struct kref *ref)
-{
-       struct netfront_accel_vif_state *vif_state = 
-               container_of(ref, struct netfront_accel_vif_state, vif_kref);
-       struct netfront_accel_hooks *hooks;
-       unsigned flags;
-
-       DPRINTK("%p\n", vif_state);
-
-       /* 
-        * Now that this vif has finished using the hooks, it can
-        * decrement the accelerator's global copy ref count 
-        */
-       kref_put(&vif_state->np->accelerator->accel_kref, accel_kref_release);
-
-       spin_lock_irqsave(&vif_state->np->accelerator->vif_states_lock, flags);
-       if (vif_state->need_probe) {
-               hooks = vif_state->need_probe;
-               vif_state->need_probe = NULL;
-               spin_unlock_irqrestore
-                       (&vif_state->np->accelerator->vif_states_lock, flags);
-               hooks->new_device(vif_state->np->netdev, vif_state->dev);
-       } else {
-               vif_state->ready_for_probe = 1;
-               spin_unlock_irqrestore
-                       (&vif_state->np->accelerator->vif_states_lock, flags);
-       }
-}
-
diff -r c75337e70043 drivers/xen/netfront/netfront.h
--- a/drivers/xen/netfront/netfront.h   Tue Oct 30 13:26:47 2007 +0000
+++ b/drivers/xen/netfront/netfront.h   Tue Oct 30 16:31:18 2007 +0000
@@ -37,7 +37,6 @@
 #include <linux/netdevice.h>
 #include <linux/skbuff.h>
 #include <linux/list.h>
-#include <linux/kref.h>
 
 #define NET_TX_RING_SIZE __RING_SIZE((struct netif_tx_sring *)0, PAGE_SIZE)
 #define NET_RX_RING_SIZE __RING_SIZE((struct netif_rx_sring *)0, PAGE_SIZE)
@@ -109,14 +108,6 @@ struct netfront_accel_vif_state {
        struct xenbus_device *dev;
        struct netfront_info *np;
        struct netfront_accel_hooks *hooks;
-
-       /* 
-        * Protect against removal of hooks while in use.  
-        */
-       struct kref vif_kref;
-
-       unsigned ready_for_probe;
-       struct netfront_accel_hooks *need_probe;
 }; 
 
 /* 
@@ -137,10 +128,7 @@ struct netfront_accelerator {
        char *frontend;
        /* The hooks into the accelerator plugin module */
        struct netfront_accel_hooks *hooks;
-       /* 
-        * Protect against removal of hooks while in use.  
-        */
-       struct kref accel_kref;
+
        /* 
         * List of per-netfront device state (struct
         * netfront_accel_vif_state) for each netfront device that is
@@ -148,14 +136,6 @@ struct netfront_accelerator {
         */
        struct list_head vif_states;
        spinlock_t vif_states_lock;
-       /* 
-        * Semaphore to signal that all users of this accelerator have
-        * finished using it before module is unloaded
-        */
-       struct semaphore exit_semaphore; 
-
-       unsigned ready_for_probe;
-       struct netfront_accel_hooks *need_probe;
 };
 
 struct netfront_info {

Attachment: simplify_accelerator_locking
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-devel] [Patch 7/8] Netfront accelerator bug fixes, Kieran Mansley <=