Removing the BKL from FASYNC handling ran into the challenge of keeping the setting of the FASYNC bit in filp->f_flags atomic with regard to calls to the underlying fasync() function. Andi Kleen suggested moving the handling of that bit into fasync(); this patch does exactly that.
As it happens, every fasync() implementation in the kernel with one exception calls fasync_helper(). So, if we make fasync_helper() set the FASYNC bit, we can avoid making any changes to the other fasync() functions - as long as those functions, themselves, have proper locking. Most fasync() implementations do nothing but call fasync_helper() - which has its own lock - so they are easily verified as correct. The BKL had already been pushed down into the few exceptions. The networking code has its own version of fasync_helper(), so that code has been augmented with explicit FASYNC bit handling. Signed-off-by: Jonathan Corbet <[email protected]> --- fs/fcntl.c | 21 ++++++++------------- fs/ioctl.c | 11 +---------- net/socket.c | 2 ++ 3 files changed, 11 insertions(+), 23 deletions(-) diff --git a/fs/fcntl.c b/fs/fcntl.c index 756119e..7b641d9 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -185,19 +185,14 @@ static int setfl(int fd, struct file * filp, unsigned long arg) return error; /* - * We still need a lock here for now to keep multiple FASYNC calls - * from racing with each other. + * ->fasync() is responsible for setting the FASYNC bit. */ - lock_kernel(); - if ((arg ^ filp->f_flags) & FASYNC) { - if (filp->f_op && filp->f_op->fasync) { - error = filp->f_op->fasync(fd, filp, (arg & FASYNC) != 0); - if (error < 0) - goto out; - } - change_bit(NR_FASYNC, &filp->f_flags); + if (((arg ^ filp->f_flags) & FASYNC) && filp->f_op && + filp->f_op->fasync) { + error = filp->f_op->fasync(fd, filp, (arg & FASYNC) != 0); + if (error < 0) + goto out; } - /* * Now that we use bitops we have to tweak each bit individually. */ @@ -207,7 +202,6 @@ static int setfl(int fd, struct file * filp, unsigned long arg) tweak_flags_bit(NR_O_DIRECT, arg & O_DIRECT, &filp->f_flags); tweak_flags_bit(NR_O_NOATIME, arg & O_NOATIME, &filp->f_flags); out: - unlock_kernel(); return error; } @@ -532,7 +526,7 @@ static DEFINE_RWLOCK(fasync_lock); static struct kmem_cache *fasync_cache __read_mostly; /* - * fasync_helper() is used by some character device drivers (mainly mice) + * fasync_helper() is used by almost all character device drivers * to set up the fasync queue. It returns negative on error, 0 if it did * no changes and positive if it added/deleted the entry. */ @@ -548,6 +542,7 @@ int fasync_helper(int fd, struct file * filp, int on, struct fasync_struct **fap return -ENOMEM; } write_lock_irq(&fasync_lock); + tweak_flags_bit(NR_FASYNC, on, &filp->f_flags); for (fp = fapp; (fa = *fp) != NULL; fp = &fa->fa_next) { if (fa->fa_file == filp) { if(on) { diff --git a/fs/ioctl.c b/fs/ioctl.c index fc0db36..bc32cb2 100644 --- a/fs/ioctl.c +++ b/fs/ioctl.c @@ -427,17 +427,11 @@ static int ioctl_fioasync(unsigned int fd, struct file *filp, /* Did FASYNC state change ? */ if ((flag ^ filp->f_flags) & FASYNC) { if (filp->f_op && filp->f_op->fasync) + /* fasync() adjusts filp->f_flags */ error = filp->f_op->fasync(fd, filp, on); else error = -ENOTTY; } - if (error) - return error; - - if (on) - set_bit(NR_FASYNC, &filp->f_flags); - else - clear_bit(NR_FASYNC, &filp->f_flags); return error; } @@ -505,10 +499,7 @@ int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd, break; case FIOASYNC: - /* BKL needed to avoid races tweaking f_flags */ - lock_kernel(); error = ioctl_fioasync(fd, filp, argp); - unlock_kernel(); break; case FIOQSIZE: diff --git a/net/socket.c b/net/socket.c index 35dd737..2b1e380 100644 --- a/net/socket.c +++ b/net/socket.c @@ -1037,6 +1037,7 @@ static int sock_fasync(int fd, struct file *filp, int on) break; if (on) { + set_bit(NR_FASYNC, &filp->f_flags); if (fa != NULL) { write_lock_bh(&sk->sk_callback_lock); fa->fa_fd = fd; @@ -1053,6 +1054,7 @@ static int sock_fasync(int fd, struct file *filp, int on) sock->fasync_list = fna; write_unlock_bh(&sk->sk_callback_lock); } else { + clear_bit(NR_FASYNC, &filp->f_flags); if (fa != NULL) { write_lock_bh(&sk->sk_callback_lock); *prev = fa->fa_next; -- 1.6.1.1 -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
