Sophie

Sophie

distrib > Scientific%20Linux > 5x > x86_64 > by-pkgid > fc11cd6e1c513a17304da94a5390f3cd > files > 927

kernel-2.6.18-194.11.1.el5.src.rpm

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;
 }