fasync: RCU and fine grained locking

kill_fasync() uses a central rwlock, candidate for RCU conversion, to
avoid cache line ping pongs on SMP.

fasync_remove_entry() and fasync_add_entry() can disable IRQS on a short
section instead during whole list scan.

Use a spinlock per fasync_struct to synchronize kill_fasync_rcu() and
fasync_{remove|add}_entry(). This spinlock is IRQ safe, so sock_fasync()
doesnt need its own implementation and can use fasync_helper(), to
reduce code size and complexity.

We can remove __kill_fasync() direct use in net/socket.c, and rename it
to kill_fasync_rcu().

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

1 files changed, 42 insertions, 24 deletions

diff --git a/fs/fcntl.c b/fs/fcntl.c
index 452d02f9075..0a140741b39 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -614,9 +614,15 @@ int send_sigurg(struct fown_struct *fown)
         return ret;
 }
 
-static DEFINE_RWLOCK(fasync_lock);
+static DEFINE_SPINLOCK(fasync_lock);
 static struct kmem_cache *fasync_cache __read_mostly;
 
+static void fasync_free_rcu(struct rcu_head *head)
+{
+        kmem_cache_free(fasync_cache,
+                        container_of(head, struct fasync_struct, fa_rcu));
+}
+
 /*
  * Remove a fasync entry. If successfully removed, return
  * positive and clear the FASYNC flag. If no entry exists,
@@ -625,8 +631,6 @@ static struct kmem_cache *fasync_cache __read_mostly;
  * NOTE! It is very important that the FASYNC flag always
  * match the state "is the filp on a fasync list".
  *
- * We always take the 'filp->f_lock', in since fasync_lock
- * needs to be irq-safe.
  */
 static int fasync_remove_entry(struct file *filp, struct fasync_struct **fapp)
 {
@@ -634,17 +638,22 @@ static int fasync_remove_entry(struct file *filp, struct fasync_struct **fapp)
         int result = 0;
 
         spin_lock(&filp->f_lock);
-        write_lock_irq(&fasync_lock);
+        spin_lock(&fasync_lock);
         for (fp = fapp; (fa = *fp) != NULL; fp = &fa->fa_next) {
                 if (fa->fa_file != filp)
                         continue;
+
+                spin_lock_irq(&fa->fa_lock);
+                fa->fa_file = NULL;
+                spin_unlock_irq(&fa->fa_lock);
+
                 *fp = fa->fa_next;
-                kmem_cache_free(fasync_cache, fa);
+                call_rcu(&fa->fa_rcu, fasync_free_rcu);
                 filp->f_flags &= ~FASYNC;
                 result = 1;
                 break;
         }
-        write_unlock_irq(&fasync_lock);
+        spin_unlock(&fasync_lock);
         spin_unlock(&filp->f_lock);
         return result;
 }
@@ -666,25 +675,30 @@ static int fasync_add_entry(int fd, struct file *filp, struct fasync_struct **fa
                 return -ENOMEM;
 
         spin_lock(&filp->f_lock);
-        write_lock_irq(&fasync_lock);
+        spin_lock(&fasync_lock);
         for (fp = fapp; (fa = *fp) != NULL; fp = &fa->fa_next) {
                 if (fa->fa_file != filp)
                         continue;
+
+                spin_lock_irq(&fa->fa_lock);
                 fa->fa_fd = fd;
+                spin_unlock_irq(&fa->fa_lock);
+
                 kmem_cache_free(fasync_cache, new);
                 goto out;
         }
 
+        spin_lock_init(&new->fa_lock);
         new->magic = FASYNC_MAGIC;
         new->fa_file = filp;
         new->fa_fd = fd;
         new->fa_next = *fapp;
-        *fapp = new;
+        rcu_assign_pointer(*fapp, new);
         result = 1;
         filp->f_flags |= FASYNC;
 
 out:
-        write_unlock_irq(&fasync_lock);
+        spin_unlock(&fasync_lock);
         spin_unlock(&filp->f_lock);
         return result;
 }
@@ -704,37 +718,41 @@ int fasync_helper(int fd, struct file * filp, int on, struct fasync_struct **fap
 
 EXPORT_SYMBOL(fasync_helper);
 
-void __kill_fasync(struct fasync_struct *fa, int sig, int band)
+/*
+ * rcu_read_lock() is held
+ */
+static void kill_fasync_rcu(struct fasync_struct *fa, int sig, int band)
 {
         while (fa) {
-                struct fown_struct * fown;
+                struct fown_struct *fown;
                 if (fa->magic != FASYNC_MAGIC) {
                         printk(KERN_ERR "kill_fasync: bad magic number in "
                                "fasync_struct!\n");
                         return;
                 }
-                fown = &fa->fa_file->f_owner;
-                /* Don't send SIGURG to processes which have not set a
-                   queued signum: SIGURG has its own default signalling
-                   mechanism. */
-                if (!(sig == SIGURG && fown->signum == 0))
-                        send_sigio(fown, fa->fa_fd, band);
-                fa = fa->fa_next;
+                spin_lock(&fa->fa_lock);
+                if (fa->fa_file) {
+                        fown = &fa->fa_file->f_owner;
+                        /* Don't send SIGURG to processes which have not set a
+                           queued signum: SIGURG has its own default signalling
+                           mechanism. */
+                        if (!(sig == SIGURG && fown->signum == 0))
+                                send_sigio(fown, fa->fa_fd, band);
+                }
+                spin_unlock(&fa->fa_lock);
+                fa = rcu_dereference(fa->fa_next);
         }
 }
 
-EXPORT_SYMBOL(__kill_fasync);
-
 void kill_fasync(struct fasync_struct **fp, int sig, int band)
 {
         /* First a quick test without locking: usually
          * the list is empty.
          */
         if (*fp) {
-                read_lock(&fasync_lock);
-                /* reread *fp after obtaining the lock */
-                __kill_fasync(*fp, sig, band);
-                read_unlock(&fasync_lock);
+                rcu_read_lock();
+                kill_fasync_rcu(rcu_dereference(*fp), sig, band);
+                rcu_read_unlock();
         }
 }
 EXPORT_SYMBOL(kill_fasync);