releases/2.6.31.12/fasync-split-fasync_helper-into-separate-add-remove-functions.patch

   1 From 53281b6d34d44308372d16acb7fb5327609f68b6 Mon Sep 17 00:00:00 2001
   2 From: Linus Torvalds <torvalds@linux-foundation.org>
   3 Date: Wed, 16 Dec 2009 08:23:37 -0800
   4 Subject: fasync: split 'fasync_helper()' into separate add/remove functions
   5
   6 From: Linus Torvalds <torvalds@linux-foundation.org>
   7
   8 commit 53281b6d34d44308372d16acb7fb5327609f68b6 upstream.
   9
  10 Yes, the add and remove cases do share the same basic loop and the
  11 locking, but the compiler can inline and then CSE some of the end result
  12 anyway.  And splitting it up makes the code way easier to follow,
  13 and makes it clearer exactly what the semantics are.
  14
  15 In particular, we must make sure that the FASYNC flag in file->f_flags
  16 exactly matches the state of "is this file on any fasync list", since
  17 not only is that flag visible to user space (F_GETFL), but we also use
  18 that flag to check whether we need to remove any fasync entries on file
  19 close.
  20
  21 We got that wrong for the case of a mixed use of file locking (which
  22 tries to remove any fasync entries for file leases) and fasync.
  23
  24 Splitting the function up also makes it possible to do some future
  25 optimizations without making the function even messier.  In particular,
  26 since the FASYNC flag has to match the state of "is this on a list", we
  27 can do the following future optimizations:
  28
  29  - on remove, we don't even need to get the locks and traverse the list
  30    if FASYNC isn't set, since we can know a priori that there is no
  31    point (this is effectively the same optimization that we already do
  32    in __fput() wrt removing fasync on file close)
  33
  34  - on add, we can use the FASYNC flag to decide whether we are changing
  35    an existing entry or need to allocate a new one.
  36
  37 but this is just the cleanup + fix for the FASYNC flag.
  38
  39 Acked-by: Al Viro <viro@ZenIV.linux.org.uk>
  40 Tested-by: Tavis Ormandy <taviso@google.com>
  41 Cc: Jeff Dike <jdike@addtoit.com>
  42 Cc: Matt Mackall <mpm@selenic.com>
  43 Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  44 Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
  45
  46 ---
  47  fs/fcntl.c |  102 +++++++++++++++++++++++++++++++++++++++----------------------
  48  1 file changed, 66 insertions(+), 36 deletions(-)
  49
  50 --- a/fs/fcntl.c
  51 +++ b/fs/fcntl.c
  52 @@ -526,60 +526,90 @@ static DEFINE_RWLOCK(fasync_lock);
  53  static struct kmem_cache *fasync_cache __read_mostly;
  54
  55  /*
  56 - * fasync_helper() is used by almost all character device drivers
  57 - * to set up the fasync queue. It returns negative on error, 0 if it did
  58 - * no changes and positive if it added/deleted the entry.
  59 + * Remove a fasync entry. If successfully removed, return
  60 + * positive and clear the FASYNC flag. If no entry exists,
  61 + * do nothing and return 0.
  62 + *
  63 + * NOTE! It is very important that the FASYNC flag always
  64 + * match the state "is the filp on a fasync list".
  65 + *
  66 + * We always take the 'filp->f_lock', in since fasync_lock
  67 + * needs to be irq-safe.
  68   */
  69 -int fasync_helper(int fd, struct file * filp, int on, struct fasync_struct **fapp)
  70 +static int fasync_remove_entry(struct file *filp, struct fasync_struct **fapp)
  71  {
  72         struct fasync_struct *fa, **fp;
  73 -       struct fasync_struct *new = NULL;
  74         int result = 0;
  75
  76 -       if (on) {
  77 -               new = kmem_cache_alloc(fasync_cache, GFP_KERNEL);
  78 -               if (!new)
  79 -                       return -ENOMEM;
  80 +       spin_lock(&filp->f_lock);
  81 +       write_lock_irq(&fasync_lock);
  82 +       for (fp = fapp; (fa = *fp) != NULL; fp = &fa->fa_next) {
  83 +               if (fa->fa_file != filp)
  84 +                       continue;
  85 +               *fp = fa->fa_next;
  86 +               kmem_cache_free(fasync_cache, fa);
  87 +               filp->f_flags &= ~FASYNC;
  88 +               result = 1;
  89 +               break;
  90         }
  91 +       write_unlock_irq(&fasync_lock);
  92 +       spin_unlock(&filp->f_lock);
  93 +       return result;
  94 +}
  95 +
  96 +/*
  97 + * Add a fasync entry. Return negative on error, positive if
  98 + * added, and zero if did nothing but change an existing one.
  99 + *
 100 + * NOTE! It is very important that the FASYNC flag always
 101 + * match the state "is the filp on a fasync list".
 102 + */
 103 +static int fasync_add_entry(int fd, struct file *filp, struct fasync_struct **fapp)
 104 +{
 105 +       struct fasync_struct *new, *fa, **fp;
 106 +       int result = 0;
 107 +
 108 +       new = kmem_cache_alloc(fasync_cache, GFP_KERNEL);
 109 +       if (!new)
 110 +               return -ENOMEM;
 111
 112 -       /*
 113 -        * We need to take f_lock first since it's not an IRQ-safe
 114 -        * lock.
 115 -        */
 116         spin_lock(&filp->f_lock);
 117         write_lock_irq(&fasync_lock);
 118         for (fp = fapp; (fa = *fp) != NULL; fp = &fa->fa_next) {
 119 -               if (fa->fa_file == filp) {
 120 -                       if(on) {
 121 -                               fa->fa_fd = fd;
 122 -                               kmem_cache_free(fasync_cache, new);
 123 -                       } else {
 124 -                               *fp = fa->fa_next;
 125 -                               kmem_cache_free(fasync_cache, fa);
 126 -                               result = 1;
 127 -                       }
 128 -                       goto out;
 129 -               }
 130 +               if (fa->fa_file != filp)
 131 +                       continue;
 132 +               fa->fa_fd = fd;
 133 +               kmem_cache_free(fasync_cache, new);
 134 +               goto out;
 135         }
 136
 137 -       if (on) {
 138 -               new->magic = FASYNC_MAGIC;
 139 -               new->fa_file = filp;
 140 -               new->fa_fd = fd;
 141 -               new->fa_next = *fapp;
 142 -               *fapp = new;
 143 -               result = 1;
 144 -       }
 145 +       new->magic = FASYNC_MAGIC;
 146 +       new->fa_file = filp;
 147 +       new->fa_fd = fd;
 148 +       new->fa_next = *fapp;
 149 +       *fapp = new;
 150 +       result = 1;
 151 +       filp->f_flags |= FASYNC;
 152 +
 153  out:
 154 -       if (on)
 155 -               filp->f_flags |= FASYNC;
 156 -       else
 157 -               filp->f_flags &= ~FASYNC;
 158         write_unlock_irq(&fasync_lock);
 159         spin_unlock(&filp->f_lock);
 160         return result;
 161  }
 162
 163 +/*
 164 + * fasync_helper() is used by almost all character device drivers
 165 + * to set up the fasync queue, and for regular files by the file
 166 + * lease code. It returns negative on error, 0 if it did no changes
 167 + * and positive if it added/deleted the entry.
 168 + */
 169 +int fasync_helper(int fd, struct file * filp, int on, struct fasync_struct **fapp)
 170 +{
 171 +       if (!on)
 172 +               return fasync_remove_entry(filp, fapp);
 173 +       return fasync_add_entry(fd, filp, fapp);
 174 +}
 175 +
 176  EXPORT_SYMBOL(fasync_helper);
 177
 178  void __kill_fasync(struct fasync_struct *fa, int sig, int band)