[PATCH, RFC] fasync() BKL pushdown

[PATCH, RFC] fasync() BKL pushdown

[Jonathan Corbet]

When I put together the bkl-removal tree, I didn't add Andi's fasync()
patches because the addition of an unlocked_fasync() function was not
universally accepted.  Recently I decided to take a look and see what would
be involved in just pushing the BKL down into fasync() implementations.  It
turns out that it wasn't too hard.

The majority of fasync() functions just call fasync_helper() with a pointer
to an fasync_struct reachable from the file structure.  Given that (1) the
struct file will not go away while fasync() is running, and (2) the
VFS-level fasync() stuff is protected with the Big Fasync Lock, I contend
that these simple implementations have no need for the BKL.  Once those are
filtered out, there's really only a half-dozen places which need to be
fixed.

(As an aside, fasync_helper() can return a positive value on success.  A
number of fasync() implementations remap that to zero, but quite a few
others return it directly.  That, in turn, will cause fcntl() to return
said positive value to user space, which is contrary to what the man page
says.  There is one user of the positive return value (tty_io.c), so maybe
some aspiring janitor would like to clean up the various fasync() functions
that just do "return fasync_helper(...);"?)

The set of patches is this:

Jonathan Corbet (7):
      mpt: fasync BKL pushdown
      i2o: fasync BKL pushdown
      tun: fasync BKL pushdown
      tty_io: fasync BKL pushdown
      Bluetooth VHCI: fasync BKL pushdown
      ecryptfs: fasync BKL pushdown
      Call fasync() functions without the BKL

And the actual patch is appended.  If nobody objects, I'll pull these into
the bkl-removal tree shortly.

jon

diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
index 7734bc9..d97700a 100644
--- a/drivers/bluetooth/hci_vhci.c
+++ b/drivers/bluetooth/hci_vhci.c
@@ -318,18 +318,21 @@ static int vhci_release(struct inode *inode, struct file *file)
 static int vhci_fasync(int fd, struct file *file, int on)
 {
 	struct vhci_data *data = file->private_data;
-	int err;
+	int err = 0;
 
+	lock_kernel();
 	err = fasync_helper(fd, file, on, &data->fasync);
 	if (err < 0)
-		return err;
+		goto out;
 
 	if (on)
 		data->flags |= VHCI_FASYNC;
 	else
 		data->flags &= ~VHCI_FASYNC;
 
-	return 0;
+out:
+	unlock_kernel();
+	return err;
 }
 
 static const struct file_operations vhci_fops = {
diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index fd182f2..eda2789 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -2909,15 +2909,16 @@ static int tty_fasync(int fd, struct file *filp, int on)
 {
 	struct tty_struct *tty;
 	unsigned long flags;
-	int retval;
+	int retval = 0;
 
+	lock_kernel();
 	tty = (struct tty_struct *)filp->private_data;
 	if (tty_paranoia_check(tty, filp->f_path.dentry->d_inode, "tty_fasync"))
-		return 0;
+		goto out;
 
 	retval = fasync_helper(fd, filp, on, &tty->fasync);
 	if (retval <= 0)
-		return retval;
+		goto out;
 
 	if (on) {
 		enum pid_type type;
@@ -2935,12 +2936,15 @@ static int tty_fasync(int fd, struct file *filp, int on)
 		spin_unlock_irqrestore(&tty->ctrl_lock, flags);
 		retval = __f_setown(filp, pid, type, 0);
 		if (retval)
-			return retval;
+			goto out;
 	} else {
 		if (!tty->fasync && !waitqueue_active(&tty->read_wait))
 			tty->minimum_to_wake = N_TTY_BUF_SIZE;
 	}
-	return 0;
+	retval = 0;
+out:
+	unlock_kernel();
+	return retval;
 }
 
 /**
diff --git a/drivers/message/fusion/mptctl.c b/drivers/message/fusion/mptctl.c
index e630b50..c594656 100644
--- a/drivers/message/fusion/mptctl.c
+++ b/drivers/message/fusion/mptctl.c
@@ -548,11 +548,15 @@ static int
 mptctl_fasync(int fd, struct file *filep, int mode)
 {
 	MPT_ADAPTER	*ioc;
+	int ret;
 
+	lock_kernel();
 	list_for_each_entry(ioc, &ioc_list, list)
 		ioc->aen_event_read_flag=0;
 
-	return fasync_helper(fd, filep, mode, &async_queue);
+	ret = fasync_helper(fd, filep, mode, &async_queue);
+	unlock_kernel();
+	return ret;
 }
 
 static int
diff --git a/drivers/message/i2o/i2o_config.c b/drivers/message/i2o/i2o_config.c
index 95b4c10..4238de9 100644
--- a/drivers/message/i2o/i2o_config.c
+++ b/drivers/message/i2o/i2o_config.c
@@ -1084,15 +1084,17 @@ static int cfg_fasync(int fd, struct file *fp, int on)
 {
 	ulong id = (ulong) fp->private_data;
 	struct i2o_cfg_info *p;
+	int ret = -EBADF;
 
+	lock_kernel();
 	for (p = open_files; p; p = p->next)
 		if (p->q_id == id)
 			break;
 
-	if (!p)
-		return -EBADF;
-
-	return fasync_helper(fd, fp, on, &p->fasync);
+	if (p)
+		ret = fasync_helper(fd, fp, on, &p->fasync);
+	unlock_kernel();
+	return ret;
 }
 
 static int cfg_release(struct inode *inode, struct file *file)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index ce5af2a..4c0c597 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -782,18 +782,21 @@ static int tun_chr_fasync(int fd, struct file *file, int on)
 
 	DBG(KERN_INFO "%s: tun_chr_fasync %d\n", tun->dev->name, on);
 
+	lock_kernel();
 	if ((ret = fasync_helper(fd, file, on, &tun->fasync)) < 0)
-		return ret;
+		goto out;
 
 	if (on) {
 		ret = __f_setown(file, task_pid(current), PIDTYPE_PID, 0);
 		if (ret)
-			return ret;
+			goto out;
 		tun->flags |= TUN_FASYNC;
 	} else
 		tun->flags &= ~TUN_FASYNC;
-
-	return 0;
+	ret = 0;
+out:
+	unlock_kernel();
+	return ret;
 }
 
 static int tun_chr_open(struct inode *inode, struct file * file)
diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index 2258b8f..24749bf 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -30,6 +30,7 @@
 #include <linux/security.h>
 #include <linux/compat.h>
 #include <linux/fs_stack.h>
+#include <linux/smp_lock.h>
 #include "ecryptfs_kernel.h"
 
 /**
@@ -277,9 +278,11 @@ static int ecryptfs_fasync(int fd, struct file *file, int flag)
 	int rc = 0;
 	struct file *lower_file = NULL;
 
+	lock_kernel();
 	lower_file = ecryptfs_file_to_lower(file);
 	if (lower_file->f_op && lower_file->f_op->fasync)
 		rc = lower_file->f_op->fasync(fd, lower_file, flag);
+	unlock_kernel();
 	return rc;
 }
 
diff --git a/fs/fcntl.c b/fs/fcntl.c
index bfd7765..330a7d7 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -12,7 +12,6 @@
 #include <linux/fdtable.h>
 #include <linux/capability.h>
 #include <linux/dnotify.h>
-#include <linux/smp_lock.h>
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/security.h>
@@ -227,7 +226,6 @@ static int setfl(int fd, struct file * filp, unsigned long arg)
 	if (error)
 		return error;
 
-	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);
@@ -238,7 +236,6 @@ static int setfl(int fd, struct file * filp, unsigned long arg)
 
 	filp->f_flags = (arg & SETFL_MASK) | (filp->f_flags & ~SETFL_MASK);
  out:
-	unlock_kernel();
 	return error;
 }
 

Re: [PATCH, RFC] fasync() BKL pushdown

[Andi Kleen]

Jonathan Corbet wrote:

> The majority of fasync() functions just call fasync_helper() with a pointer
> to an fasync_struct reachable from the file structure.  Given that (1) the
> struct file will not go away while fasync() is running, and (2) the
> VFS-level fasync() stuff is protected with the Big Fasync Lock, I contend
> that these simple implementations have no need for the BKL.  

Not necessarily true, they might require BKL still for fd live time issues.

-Andi

Re: [PATCH, RFC] fasync() BKL pushdown

[Jonathan Corbet]

On Fri, 20 Jun 2008 19:55:03 +0200
Andi Kleen <andi@firstfloor.org> wrote:

> Jonathan Corbet wrote:
> 
> > The majority of fasync() functions just call fasync_helper() with a
> > pointer to an fasync_struct reachable from the file structure.
> > Given that (1) the struct file will not go away while fasync() is
> > running, and (2) the VFS-level fasync() stuff is protected with the
> > Big Fasync Lock, I contend that these simple implementations have
> > no need for the BKL.  
> 
> Not necessarily true, they might require BKL still for fd live time
> issues.

Could you help me out a bit here?  I'm even slower than usual when it
comes to VFS stuff.  As far as I can tell, the given file cannot go
away during the call to fasync(), as sys_fcntl() holds a reference on
it.  Are you saying that something else can happen during that time?

Thanks,

jon

Re: [PATCH, RFC] fasync() BKL pushdown

[Andi Kleen]

Jonathan Corbet wrote:
> On Fri, 20 Jun 2008 19:55:03 +0200
> Andi Kleen <andi@firstfloor.org> wrote:
> 
>> Jonathan Corbet wrote:
>>
>>> The majority of fasync() functions just call fasync_helper() with a
>>> pointer to an fasync_struct reachable from the file structure.
>>> Given that (1) the struct file will not go away while fasync() is
>>> running, and (2) the VFS-level fasync() stuff is protected with the
>>> Big Fasync Lock, I contend that these simple implementations have
>>> no need for the BKL.  
>> Not necessarily true, they might require BKL still for fd live time
>> issues.
> 
> Could you help me out a bit here?  I'm even slower than usual when it
> comes to VFS stuff.  As far as I can tell, the given file cannot go
> away during the call to fasync(), as sys_fcntl() holds a reference on
> it.  Are you saying that something else can happen during that time?

Some devices do state change even when the reference count is > 0.
Would need to double check it's all ok with the fasync list.

Anyways I did this auditing for the cases where I used unlocked_ioctl
[but I think I wanted to redo it because i wasn't 100% sure anymore]
and I haven't done it at all for the cases that weren't converted.

-Andi

[PATCH, RFC] fasync() BKL pushdown (take 2)

[Jonathan Corbet]

On Fri, 20 Jun 2008 21:12:51 +0200
Andi Kleen <andi@firstfloor.org> wrote:

> Some devices do state change even when the reference count is > 0.
> Would need to double check it's all ok with the fasync list.

OK, I've gone over all of the fasync() definitions again with an eye
toward convincing myself that the fasync list would not get cleared,
freed, or otherwise molested if fasync() runs without BKL protection.
I focused especially on other code (open(), ioctl()) which might still
run with the BKL.  The result was two more pushdowns in spots where I
wasn't sure; chance are both are unnecessary.

The new fasync pushdown tree contains:

Jonathan Corbet (9):
      mpt: fasync BKL pushdown
      i2o: fasync BKL pushdown
      tun: fasync BKL pushdown
      tty_io: fasync BKL pushdown
      Bluetooth VHCI: fasync BKL pushdown
      ecryptfs: fasync BKL pushdown
      ipmi: fasync BKL pushdown
      snd/PCM: fasync BKL pushdown
      Call fasync() functions without the BKL

 drivers/bluetooth/hci_vhci.c     |    9 ++++++---
 drivers/char/ipmi/ipmi_devintf.c |    2 ++
 drivers/char/tty_io.c            |   14 +++++++++-----
 drivers/message/fusion/mptctl.c  |    6 +++++-
 drivers/message/i2o/i2o_config.c |   10 ++++++----
 drivers/net/tun.c                |   11 +++++++----
 fs/ecryptfs/file.c               |    3 +++
 fs/fcntl.c                       |    3 ---
 sound/core/pcm_native.c          |    8 ++++++--
 9 files changed, 44 insertions(+), 22 deletions(-)

Once again, the combined patch is small, so I'll just attach it.
Anybody have a reason why I should not fold this work into the
bkl-removal tree?

Thanks,

jon


diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
index 7734bc9..d97700a 100644
--- a/drivers/bluetooth/hci_vhci.c
+++ b/drivers/bluetooth/hci_vhci.c
@@ -318,18 +318,21 @@ static int vhci_release(struct inode *inode,
struct file *file) static int vhci_fasync(int fd, struct file *file,
int on) {
 	struct vhci_data *data = file->private_data;
-	int err;
+	int err = 0;
 
+	lock_kernel();
 	err = fasync_helper(fd, file, on, &data->fasync);
 	if (err < 0)
-		return err;
+		goto out;
 
 	if (on)
 		data->flags |= VHCI_FASYNC;
 	else
 		data->flags &= ~VHCI_FASYNC;
 
-	return 0;
+out:
+	unlock_kernel();
+	return err;
 }
 
 static const struct file_operations vhci_fops = {
diff --git a/drivers/char/ipmi/ipmi_devintf.c
b/drivers/char/ipmi/ipmi_devintf.c index c816656..c11a404 100644
--- a/drivers/char/ipmi/ipmi_devintf.c
+++ b/drivers/char/ipmi/ipmi_devintf.c
@@ -101,7 +101,9 @@ static int ipmi_fasync(int fd, struct file *file,
int on) struct ipmi_file_private *priv = file->private_data;
 	int                      result;
 
+	lock_kernel(); /* could race against open() otherwise */
 	result = fasync_helper(fd, file, on, &priv->fasync_queue);
+	unlock_kernel();
 
 	return (result);
 }
diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index fd182f2..eda2789 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -2909,15 +2909,16 @@ static int tty_fasync(int fd, struct file
*filp, int on) {
 	struct tty_struct *tty;
 	unsigned long flags;
-	int retval;
+	int retval = 0;
 
+	lock_kernel();
 	tty = (struct tty_struct *)filp->private_data;
 	if (tty_paranoia_check(tty, filp->f_path.dentry->d_inode,
"tty_fasync"))
-		return 0;
+		goto out;
 
 	retval = fasync_helper(fd, filp, on, &tty->fasync);
 	if (retval <= 0)
-		return retval;
+		goto out;
 
 	if (on) {
 		enum pid_type type;
@@ -2935,12 +2936,15 @@ static int tty_fasync(int fd, struct file
*filp, int on) spin_unlock_irqrestore(&tty->ctrl_lock, flags);
 		retval = __f_setown(filp, pid, type, 0);
 		if (retval)
-			return retval;
+			goto out;
 	} else {
 		if (!tty->fasync && !waitqueue_active(&tty->read_wait))
 			tty->minimum_to_wake = N_TTY_BUF_SIZE;
 	}
-	return 0;
+	retval = 0;
+out:
+	unlock_kernel();
+	return retval;
 }
 
 /**
diff --git a/drivers/message/fusion/mptctl.c
b/drivers/message/fusion/mptctl.c index e630b50..c594656 100644
--- a/drivers/message/fusion/mptctl.c
+++ b/drivers/message/fusion/mptctl.c
@@ -548,11 +548,15 @@ static int
 mptctl_fasync(int fd, struct file *filep, int mode)
 {
 	MPT_ADAPTER	*ioc;
+	int ret;
 
+	lock_kernel();
 	list_for_each_entry(ioc, &ioc_list, list)
 		ioc->aen_event_read_flag=0;
 
-	return fasync_helper(fd, filep, mode, &async_queue);
+	ret = fasync_helper(fd, filep, mode, &async_queue);
+	unlock_kernel();
+	return ret;
 }
 
 static int
diff --git a/drivers/message/i2o/i2o_config.c
b/drivers/message/i2o/i2o_config.c index 95b4c10..4238de9 100644
--- a/drivers/message/i2o/i2o_config.c
+++ b/drivers/message/i2o/i2o_config.c
@@ -1084,15 +1084,17 @@ static int cfg_fasync(int fd, struct file *fp,
int on) {
 	ulong id = (ulong) fp->private_data;
 	struct i2o_cfg_info *p;
+	int ret = -EBADF;
 
+	lock_kernel();
 	for (p = open_files; p; p = p->next)
 		if (p->q_id == id)
 			break;
 
-	if (!p)
-		return -EBADF;
-
-	return fasync_helper(fd, fp, on, &p->fasync);
+	if (p)
+		ret = fasync_helper(fd, fp, on, &p->fasync);
+	unlock_kernel();
+	return ret;
 }
 
 static int cfg_release(struct inode *inode, struct file *file)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index ce5af2a..4c0c597 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -782,18 +782,21 @@ static int tun_chr_fasync(int fd, struct file
*file, int on) 
 	DBG(KERN_INFO "%s: tun_chr_fasync %d\n", tun->dev->name, on);
 
+	lock_kernel();
 	if ((ret = fasync_helper(fd, file, on, &tun->fasync)) < 0)
-		return ret;
+		goto out;
 
 	if (on) {
 		ret = __f_setown(file, task_pid(current), PIDTYPE_PID,
0); if (ret)
-			return ret;
+			goto out;
 		tun->flags |= TUN_FASYNC;
 	} else
 		tun->flags &= ~TUN_FASYNC;
-
-	return 0;
+	ret = 0;
+out:
+	unlock_kernel();
+	return ret;
 }
 
 static int tun_chr_open(struct inode *inode, struct file * file)
diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index 2258b8f..24749bf 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -30,6 +30,7 @@
 #include <linux/security.h>
 #include <linux/compat.h>
 #include <linux/fs_stack.h>
+#include <linux/smp_lock.h>
 #include "ecryptfs_kernel.h"
 
 /**
@@ -277,9 +278,11 @@ static int ecryptfs_fasync(int fd, struct file
*file, int flag) int rc = 0;
 	struct file *lower_file = NULL;
 
+	lock_kernel();
 	lower_file = ecryptfs_file_to_lower(file);
 	if (lower_file->f_op && lower_file->f_op->fasync)
 		rc = lower_file->f_op->fasync(fd, lower_file, flag);
+	unlock_kernel();
 	return rc;
 }
 
diff --git a/fs/fcntl.c b/fs/fcntl.c
index bfd7765..330a7d7 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -12,7 +12,6 @@
 #include <linux/fdtable.h>
 #include <linux/capability.h>
 #include <linux/dnotify.h>
-#include <linux/smp_lock.h>
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/security.h>
@@ -227,7 +226,6 @@ static int setfl(int fd, struct file * filp,
unsigned long arg) if (error)
 		return error;
 
-	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); @@ -238,7 +236,6 @@ static int setfl(int fd, struct file
* filp, unsigned long arg) 
 	filp->f_flags = (arg & SETFL_MASK) | (filp->f_flags &
~SETFL_MASK); out:
-	unlock_kernel();
 	return error;
 }
 
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 61f5d42..c49b9d9 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -22,6 +22,7 @@
 #include <linux/mm.h>
 #include <linux/file.h>
 #include <linux/slab.h>
+#include <linux/smp_lock.h>
 #include <linux/time.h>
 #include <linux/pm_qos_params.h>
 #include <linux/uio.h>
@@ -3249,14 +3250,17 @@ static int snd_pcm_fasync(int fd, struct file *
file, int on) struct snd_pcm_file * pcm_file;
 	struct snd_pcm_substream *substream;
 	struct snd_pcm_runtime *runtime;
-	int err;
+	int err = -ENXIO;
 
+	lock_kernel();
 	pcm_file = file->private_data;
 	substream = pcm_file->substream;
-	snd_assert(substream != NULL, return -ENXIO);
+	snd_assert(substream != NULL, goto out);
 	runtime = substream->runtime;
 
 	err = fasync_helper(fd, file, on, &runtime->fasync);
+out:
+	unlock_kernel();
 	if (err < 0)
 		return err;
 	return 0;

Re: [PATCH, RFC] fasync() BKL pushdown (take 2)

[Andi Kleen]

Jonathan Corbet wrote:
> On Fri, 20 Jun 2008 21:12:51 +0200
> Andi Kleen <andi@firstfloor.org> wrote:
> 
>> Some devices do state change even when the reference count is > 0.
>> Would need to double check it's all ok with the fasync list.
> 
> OK, I've gone over all of the fasync() definitions again with an eye
> toward convincing myself that the fasync list would not get cleared,
> freed, or otherwise molested if fasync() runs without BKL protection.
> I focused especially on other code (open(), ioctl()) which might still
> run with the BKL.  The result was two more pushdowns in spots where I
> wasn't sure; chance are both are unnecessary.

Ok fine for me then. I haven't read it again in detail, but it sounds
good now.

I still think it would be better to somehow compile break external users,
but that would be only the icing on the cake.

Acked-by: Andi Kleen <ak@linux.intel.com>

-Andi

Re: [PATCH, RFC] fasync() BKL pushdown

[Arjan van de Ven]

On Fri, 20 Jun 2008 11:29:14 -0600
Jonathan Corbet <corbet@lwn.net> wrote:
> 
> The majority of fasync() functions just call fasync_helper() with a
> pointer to an fasync_struct reachable from the file structure.  Given
> that (1) the struct file will not go away while fasync() is running,
> and (2) the VFS-level fasync() stuff is protected with the Big Fasync
> Lock, I contend that these simple implementations have no need for
> the BKL.  Once those are filtered out, there's really only a
> half-dozen places which need to be fixed.


> Jonathan Corbet (7):
>       mpt: fasync BKL pushdown
>       i2o: fasync BKL pushdown
>       tun: fasync BKL pushdown
>       tty_io: fasync BKL pushdown
>       Bluetooth VHCI: fasync BKL pushdown
>       ecryptfs: fasync BKL pushdown
>       Call fasync() functions without the BKL
> 
> And the actual patch is appended.  If nobody objects, I'll pull these
> into the bkl-removal tree shortly.
> 

looks good to me

only question is if this one is really needed:
+	lock_kernel();
 	tty = (struct tty_struct *)filp->private_data;
 	if (tty_paranoia_check(tty, filp->f_path.dentry->d_inode,
"tty_fasync"))
-		return 0;
+		goto out;
 
 	retval = fasync_helper(fd, filp, on, &tty->fasync);
 	if (retval <= 0)

(given that it just calls the helper.. mostly and that Alan has been
busy removing the BKL from the tty layer)


-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH, RFC] fasync() BKL pushdown

[Jonathan Corbet]

On Fri, 20 Jun 2008 13:58:40 -0700
Arjan van de Ven <arjan@infradead.org> wrote:

> only question is if this one is really needed:
> +	lock_kernel();
>  	tty = (struct tty_struct *)filp->private_data;
>  	if (tty_paranoia_check(tty, filp->f_path.dentry->d_inode,
> "tty_fasync"))
> -		return 0;
> +		goto out;
>  
>  	retval = fasync_helper(fd, filp, on, &tty->fasync);
>  	if (retval <= 0)
> 
> (given that it just calls the helper.. mostly and that Alan has been
> busy removing the BKL from the tty layer)

There's enough other stuff going on there, including manipulations of
the tty_struct, waitqueue_active() checks, etc., that I figured I should
really leave the BKL in there and let Alan figure it out.  I've learned
that it's best to tread with great care in that part of the kernel...

jon


Jonathan Corbet / LWN.net / corbet@lwn.net