From: Lachlan McIlroy <lmcilroy@redhat.com> Date: Fri, 26 Feb 2010 02:06:30 -0500 Subject: [fs] remove unneccessary f_ep_lock from fasync_helper Message-id: <663627576.2382971267149990537.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com> Patchwork-id: 23441 O-Subject: [RHEL5.6 PATCH] Remove unneccessary use of f_ep_lock from fasync_helper code Bugzilla: 567479 RH-Acked-by: Eric Sandeen <sandeen@redhat.com> RH-Acked-by: Danny Feng <dfeng@redhat.com> RH-Acked-by: Josef Bacik <josef@redhat.com> This patch fixes BZ567479: https://bugzilla.redhat.com/show_bug.cgi?id=567479 The problem here is that a previous patch from BZ548657 to split fasync_helper() into separate add/remove functions erroneously included additional redundant locking from upstream. This extra locking on the file pointer exposed an issue with IBM's GPFS that uses file locks with a NULL file pointer in the fl_file field. Regardless of whether or not a NULL fl_file is okay there is a genuine bug in RHEL code. In upstream code the f_lock is obtained where the f_ep_lock is used here. The f_lock is needed upstream because these functions may modify the f_flags field. In RHEL5.x we don't have an f_lock to protect the f_flags field but instead use the BKL which should already be held by all callers of these functions. Either way the f_ep_lock is not used to protect f_flags anywhere else so it's redundant here. This patch has been verified by IBM's GPFS developers. Lachlan Signed-off-by: Jarod Wilson <jarod@redhat.com> diff --git a/fs/fcntl.c b/fs/fcntl.c index 139b6d1..31030dd 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -551,7 +551,7 @@ static int fasync_remove_entry(struct file *filp, struct fasync_struct **fapp) { struct fasync_struct *fa, **fp; int result = 0; - spin_lock(&filp->f_ep_lock); + write_lock_irq(&fasync_lock); for (fp = fapp; (fa = *fp) != NULL; fp = &fa->fa_next) { if (fa->fa_file != filp) @@ -563,7 +563,6 @@ static int fasync_remove_entry(struct file *filp, struct fasync_struct **fapp) break; } write_unlock_irq(&fasync_lock); - spin_unlock(&filp->f_ep_lock); return result; } @@ -584,7 +583,6 @@ static int fasync_add_entry(int fd, struct file *filp, if (!new) return -ENOMEM; - spin_lock(&filp->f_ep_lock); write_lock_irq(&fasync_lock); for (fp = fapp; (fa = *fp) != NULL; fp = &fa->fa_next) { if (fa->fa_file != filp) @@ -604,7 +602,6 @@ static int fasync_add_entry(int fd, struct file *filp, out: write_unlock_irq(&fasync_lock); - spin_unlock(&filp->f_ep_lock); return result; }