linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] Another set of ioctl bkl pushdown, almost the end
@ 2010-05-19 17:24 Frederic Weisbecker
  2010-05-19 17:24 ` [PATCH 1/8] ecryptfs: Pushdown the bkl from ioctl Frederic Weisbecker
                   ` (8 more replies)
  0 siblings, 9 replies; 22+ messages in thread
From: Frederic Weisbecker @ 2010-05-19 17:24 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Thomas Gleixner, John Kacur,
	Arnd Bergmann, Tyler Hicks, Dustin Kirkland, H . Peter Anvin,
	Ian Kent, J . Bruce Fields, Neil Brown, Jeff Dike, Mikael Starvik,
	Jesper Nilsson, Tony Luck, Fenghua Yu, Jan Kara, Joern Engel,
	Mauro Carvalho Chehab

We are very close to the end of struct file_operations::ioctl

What remains:
	sound/oss: coming soon
	v4l: Waiting for inclusion or approval in v4l tree
	udf: Approved but not yet included in udf tree
	logfs: Waiting for inclusion or approval in logfs tree

If you prefer some of the following patches to go through your tree,
please tell me quickly so that I drop them before pushing
for .35

Thanks.

---
Frederic Weisbecker (8):
  ecryptfs: Pushdown the bkl from ioctl
  autofs: Pushdown the bkl from ioctl
  autofs4: Pushdown the bkl from ioctl
  sunrpc: Pushdown the bkl from ioctl
  sunrpc: Pushdown the bkl from sunrpc cache ioctl
  uml: Pushdown the bkl from harddog_kern ioctl
  cris: Pushdown the bkl from ioctl
  ia64: Use unlocked_ioctl from perfmon

 arch/cris/arch-v10/drivers/gpio.c         |   30 ++++++++++++++++++---------
 arch/cris/arch-v10/drivers/i2c.c          |   24 ++++++++++++++++-----
 arch/cris/arch-v10/drivers/sync_serial.c  |   32 +++++++++++++++++++---------
 arch/cris/arch-v32/drivers/cryptocop.c    |   24 ++++++++++++++++-----
 arch/cris/arch-v32/drivers/mach-a3/gpio.c |   28 +++++++++++++++++--------
 arch/cris/arch-v32/drivers/mach-fs/gpio.c |   29 +++++++++++++++++---------
 arch/cris/arch-v32/drivers/sync_serial.c  |   30 +++++++++++++++++++--------
 arch/ia64/kernel/perfmon.c                |   22 ++++++++++----------
 arch/um/drivers/harddog_kern.c            |   18 +++++++++++++--
 fs/autofs/root.c                          |   19 ++++++++++++++--
 fs/autofs4/root.c                         |   22 ++++++++++++++++---
 fs/ecryptfs/file.c                        |   23 ++++++++++++++++----
 net/sunrpc/cache.c                        |   13 +++++++++--
 net/sunrpc/rpc_pipe.c                     |   18 +++++++++++++--
 14 files changed, 240 insertions(+), 92 deletions(-)


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/8] ecryptfs: Pushdown the bkl from ioctl
  2010-05-19 17:24 [PATCH 0/8] Another set of ioctl bkl pushdown, almost the end Frederic Weisbecker
@ 2010-05-19 17:24 ` Frederic Weisbecker
  2010-05-20 23:39   ` Tyler Hicks
  2010-05-19 17:24 ` [PATCH 2/8] autofs: Pushdown the bkl from ioctl Frederic Weisbecker
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Frederic Weisbecker @ 2010-05-19 17:24 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Tyler Hicks, Dustin Kirkland, Ecryptfs,
	Thomas Gleixner, John Kacur, Arnd Bergmann

Pushdown the bkl to ecryptfs_ioctl.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Tyler Hicks <tyhicks@linux.vnet.ibm.com>
Cc: Dustin Kirkland <kirkland@canonical.com>
Cc: Ecryptfs <ecryptfs-devel@lists.launchpad.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Kacur <jkacur@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>
---
 fs/ecryptfs/file.c |   23 ++++++++++++++++++-----
 1 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index e7440a6..9352613 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -294,12 +294,12 @@ static int ecryptfs_fasync(int fd, struct file *file, int flag)
 	return rc;
 }
 
-static int ecryptfs_ioctl(struct inode *inode, struct file *file,
+static long ecryptfs_ioctl(struct file *file,
 			  unsigned int cmd, unsigned long arg);
 
 const struct file_operations ecryptfs_dir_fops = {
 	.readdir = ecryptfs_readdir,
-	.ioctl = ecryptfs_ioctl,
+	.unlocked_ioctl = ecryptfs_ioctl,
 	.open = ecryptfs_open,
 	.flush = ecryptfs_flush,
 	.release = ecryptfs_release,
@@ -315,7 +315,7 @@ const struct file_operations ecryptfs_main_fops = {
 	.write = do_sync_write,
 	.aio_write = generic_file_aio_write,
 	.readdir = ecryptfs_readdir,
-	.ioctl = ecryptfs_ioctl,
+	.unlocked_ioctl = ecryptfs_ioctl,
 	.mmap = generic_file_mmap,
 	.open = ecryptfs_open,
 	.flush = ecryptfs_flush,
@@ -326,8 +326,8 @@ const struct file_operations ecryptfs_main_fops = {
 };
 
 static int
-ecryptfs_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
-	       unsigned long arg)
+ecryptfs_ioctl_unlocked(struct inode *inode, struct file *file,
+			unsigned int cmd, unsigned long arg)
 {
 	int rc = 0;
 	struct file *lower_file = NULL;
@@ -341,3 +341,16 @@ ecryptfs_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
 		rc = -ENOTTY;
 	return rc;
 }
+
+static long ecryptfs_ioctl(struct file *file, unsigned int cmd,
+			   unsigned long arg)
+{
+	long ret;
+	struct inode *inode = file->f_dentry->d_inode;
+
+	lock_kernel();
+	ret = ecryptfs_ioctl_unlocked(inode, file, cmd, arg);
+	unlock_kernel();
+
+	return ret;
+}
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/8] autofs: Pushdown the bkl from ioctl
  2010-05-19 17:24 [PATCH 0/8] Another set of ioctl bkl pushdown, almost the end Frederic Weisbecker
  2010-05-19 17:24 ` [PATCH 1/8] ecryptfs: Pushdown the bkl from ioctl Frederic Weisbecker
@ 2010-05-19 17:24 ` Frederic Weisbecker
  2010-05-19 18:02   ` H. Peter Anvin
  2010-05-19 17:24 ` [PATCH 3/8] autofs4: " Frederic Weisbecker
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Frederic Weisbecker @ 2010-05-19 17:24 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, H. Peter Anvin, Autofs,
	Thomas Gleixner, John Kacur, Arnd Bergmann

Pushdown the bkl to autofs_root_ioctl.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Autofs <autofs@linux.kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Kacur <jkacur@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>
---
 fs/autofs/root.c |   19 ++++++++++++++++---
 1 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/fs/autofs/root.c b/fs/autofs/root.c
index 8713c7c..dc16b3c 100644
--- a/fs/autofs/root.c
+++ b/fs/autofs/root.c
@@ -25,12 +25,12 @@ static int autofs_root_symlink(struct inode *,struct dentry *,const char *);
 static int autofs_root_unlink(struct inode *,struct dentry *);
 static int autofs_root_rmdir(struct inode *,struct dentry *);
 static int autofs_root_mkdir(struct inode *,struct dentry *,int);
-static int autofs_root_ioctl(struct inode *, struct file *,unsigned int,unsigned long);
+static long autofs_root_ioctl(struct file *, unsigned int, unsigned long);
 
 const struct file_operations autofs_root_operations = {
 	.read		= generic_read_dir,
 	.readdir	= autofs_root_readdir,
-	.ioctl		= autofs_root_ioctl,
+	.unlocked_ioctl	= autofs_root_ioctl,
 };
 
 const struct inode_operations autofs_root_inode_operations = {
@@ -545,7 +545,7 @@ static inline int autofs_expire_run(struct super_block *sb,
  * ioctl()'s on the root directory is the chief method for the daemon to
  * generate kernel reactions
  */
-static int autofs_root_ioctl(struct inode *inode, struct file *filp,
+static int autofs_root_ioctl_unlocked(struct inode *inode, struct file *filp,
 			     unsigned int cmd, unsigned long arg)
 {
 	struct autofs_sb_info *sbi = autofs_sbi(inode->i_sb);
@@ -579,3 +579,16 @@ static int autofs_root_ioctl(struct inode *inode, struct file *filp,
 		return -ENOSYS;
 	}
 }
+
+static long autofs_root_ioctl(struct file *filp,
+			     unsigned int cmd, unsigned long arg)
+{
+	long ret;
+	struct inode *inode = filp->f_dentry->d_inode;
+
+	lock_kernel();
+	ret = autofs_root_ioctl_unlocked(inode, filp, cmd, arg);
+	unlock_kernel();
+
+	return ret;
+}
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/8] autofs4: Pushdown the bkl from ioctl
  2010-05-19 17:24 [PATCH 0/8] Another set of ioctl bkl pushdown, almost the end Frederic Weisbecker
  2010-05-19 17:24 ` [PATCH 1/8] ecryptfs: Pushdown the bkl from ioctl Frederic Weisbecker
  2010-05-19 17:24 ` [PATCH 2/8] autofs: Pushdown the bkl from ioctl Frederic Weisbecker
@ 2010-05-19 17:24 ` Frederic Weisbecker
  2010-05-19 17:24 ` [PATCH 4/8] sunrpc: " Frederic Weisbecker
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Frederic Weisbecker @ 2010-05-19 17:24 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Ian Kent, Autofs, Thomas Gleixner,
	John Kacur, Arnd Bergmann

Pushdown the bkl to autofs4_root_ioctl.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ian Kent <raven@themaw.net>
Cc: Autofs <autofs@linux.kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Kacur <jkacur@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>
---
 fs/autofs4/root.c |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index e8e5e63..db4117e 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -18,13 +18,14 @@
 #include <linux/slab.h>
 #include <linux/param.h>
 #include <linux/time.h>
+#include <linux/smp_lock.h>
 #include "autofs_i.h"
 
 static int autofs4_dir_symlink(struct inode *,struct dentry *,const char *);
 static int autofs4_dir_unlink(struct inode *,struct dentry *);
 static int autofs4_dir_rmdir(struct inode *,struct dentry *);
 static int autofs4_dir_mkdir(struct inode *,struct dentry *,int);
-static int autofs4_root_ioctl(struct inode *, struct file *,unsigned int,unsigned long);
+static long autofs4_root_ioctl(struct file *,unsigned int,unsigned long);
 static int autofs4_dir_open(struct inode *inode, struct file *file);
 static struct dentry *autofs4_lookup(struct inode *,struct dentry *, struct nameidata *);
 static void *autofs4_follow_link(struct dentry *, struct nameidata *);
@@ -38,7 +39,7 @@ const struct file_operations autofs4_root_operations = {
 	.read		= generic_read_dir,
 	.readdir	= dcache_readdir,
 	.llseek		= dcache_dir_lseek,
-	.ioctl		= autofs4_root_ioctl,
+	.unlocked_ioctl	= autofs4_root_ioctl,
 };
 
 const struct file_operations autofs4_dir_operations = {
@@ -902,8 +903,8 @@ int is_autofs4_dentry(struct dentry *dentry)
  * ioctl()'s on the root directory is the chief method for the daemon to
  * generate kernel reactions
  */
-static int autofs4_root_ioctl(struct inode *inode, struct file *filp,
-			     unsigned int cmd, unsigned long arg)
+static int autofs4_root_ioctl_unlocked(struct inode *inode, struct file *filp,
+				       unsigned int cmd, unsigned long arg)
 {
 	struct autofs_sb_info *sbi = autofs4_sbi(inode->i_sb);
 	void __user *p = (void __user *)arg;
@@ -947,3 +948,16 @@ static int autofs4_root_ioctl(struct inode *inode, struct file *filp,
 		return -ENOSYS;
 	}
 }
+
+static long autofs4_root_ioctl(struct file *filp,
+			       unsigned int cmd, unsigned long arg)
+{
+	long ret;
+	struct inode *inode = filp->f_dentry->d_inode;
+
+	lock_kernel();
+	ret = autofs4_root_ioctl_unlocked(inode, filp, cmd, arg);
+	unlock_kernel();
+
+	return ret;
+}
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 4/8] sunrpc: Pushdown the bkl from ioctl
  2010-05-19 17:24 [PATCH 0/8] Another set of ioctl bkl pushdown, almost the end Frederic Weisbecker
                   ` (2 preceding siblings ...)
  2010-05-19 17:24 ` [PATCH 3/8] autofs4: " Frederic Weisbecker
@ 2010-05-19 17:24 ` Frederic Weisbecker
  2010-05-19 17:24 ` [PATCH 5/8] sunrpc: Pushdown the bkl from sunrpc cache ioctl Frederic Weisbecker
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Frederic Weisbecker @ 2010-05-19 17:24 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, J. Bruce Fields, Neil Brown, Nfs,
	Thomas Gleixner, John Kacur, Arnd Bergmann

Pushdown the bkl to rpc_pipe_ioctl.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Cc: Neil Brown <neilb@suse.de>
Cc: Nfs <linux-nfs@vger.kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Kacur <jkacur@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>
---
 net/sunrpc/rpc_pipe.c |   18 +++++++++++++++---
 1 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index 20e30c6..95ccbcf 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -27,6 +27,7 @@
 #include <linux/workqueue.h>
 #include <linux/sunrpc/rpc_pipe_fs.h>
 #include <linux/sunrpc/cache.h>
+#include <linux/smp_lock.h>
 
 static struct vfsmount *rpc_mount __read_mostly;
 static int rpc_mount_count;
@@ -309,8 +310,7 @@ rpc_pipe_poll(struct file *filp, struct poll_table_struct *wait)
 }
 
 static int
-rpc_pipe_ioctl(struct inode *ino, struct file *filp,
-		unsigned int cmd, unsigned long arg)
+rpc_pipe_ioctl_unlocked(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	struct rpc_inode *rpci = RPC_I(filp->f_path.dentry->d_inode);
 	int len;
@@ -331,13 +331,25 @@ rpc_pipe_ioctl(struct inode *ino, struct file *filp,
 	}
 }
 
+static long
+rpc_pipe_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+	long ret;
+
+	lock_kernel();
+	ret = rpc_pipe_ioctl_unlocked(filp, cmd, arg);
+	unlock_kernel();
+
+	return ret;
+}
+
 static const struct file_operations rpc_pipe_fops = {
 	.owner		= THIS_MODULE,
 	.llseek		= no_llseek,
 	.read		= rpc_pipe_read,
 	.write		= rpc_pipe_write,
 	.poll		= rpc_pipe_poll,
-	.ioctl		= rpc_pipe_ioctl,
+	.unlocked_ioctl	= rpc_pipe_ioctl,
 	.open		= rpc_pipe_open,
 	.release	= rpc_pipe_release,
 };
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 5/8] sunrpc: Pushdown the bkl from sunrpc cache ioctl
  2010-05-19 17:24 [PATCH 0/8] Another set of ioctl bkl pushdown, almost the end Frederic Weisbecker
                   ` (3 preceding siblings ...)
  2010-05-19 17:24 ` [PATCH 4/8] sunrpc: " Frederic Weisbecker
@ 2010-05-19 17:24 ` Frederic Weisbecker
  2010-05-19 17:24 ` [PATCH 6/8] uml: Pushdown the bkl from harddog_kern ioctl Frederic Weisbecker
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Frederic Weisbecker @ 2010-05-19 17:24 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, J. Bruce Fields, Neil Brown, Nfs,
	Thomas Gleixner, John Kacur, Arnd Bergmann

Pushdown the bkl to cache_ioctl_pipefs.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Cc: Neil Brown <neilb@suse.de>
Cc: Nfs <linux-nfs@vger.kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Kacur <jkacur@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>
---
 net/sunrpc/cache.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 39bddba..91b1ade 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -33,6 +33,7 @@
 #include <linux/sunrpc/cache.h>
 #include <linux/sunrpc/stats.h>
 #include <linux/sunrpc/rpc_pipe_fs.h>
+#include <linux/smp_lock.h>
 
 #define	 RPCDBG_FACILITY RPCDBG_CACHE
 
@@ -1525,12 +1526,18 @@ static unsigned int cache_poll_pipefs(struct file *filp, poll_table *wait)
 	return cache_poll(filp, wait, cd);
 }
 
-static int cache_ioctl_pipefs(struct inode *inode, struct file *filp,
+static long cache_ioctl_pipefs(struct file *filp,
 			      unsigned int cmd, unsigned long arg)
 {
+	struct inode *inode = filp->f_dentry->d_inode;
 	struct cache_detail *cd = RPC_I(inode)->private;
+	long ret;
 
-	return cache_ioctl(inode, filp, cmd, arg, cd);
+	lock_kernel();
+	ret = cache_ioctl(inode, filp, cmd, arg, cd);
+	unlock_kernel();
+
+	return ret;
 }
 
 static int cache_open_pipefs(struct inode *inode, struct file *filp)
@@ -1553,7 +1560,7 @@ const struct file_operations cache_file_operations_pipefs = {
 	.read		= cache_read_pipefs,
 	.write		= cache_write_pipefs,
 	.poll		= cache_poll_pipefs,
-	.ioctl		= cache_ioctl_pipefs, /* for FIONREAD */
+	.unlocked_ioctl	= cache_ioctl_pipefs, /* for FIONREAD */
 	.open		= cache_open_pipefs,
 	.release	= cache_release_pipefs,
 };
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 6/8] uml: Pushdown the bkl from harddog_kern ioctl
  2010-05-19 17:24 [PATCH 0/8] Another set of ioctl bkl pushdown, almost the end Frederic Weisbecker
                   ` (4 preceding siblings ...)
  2010-05-19 17:24 ` [PATCH 5/8] sunrpc: Pushdown the bkl from sunrpc cache ioctl Frederic Weisbecker
@ 2010-05-19 17:24 ` Frederic Weisbecker
  2010-05-19 17:24 ` [PATCH 7/8] cris: Pushdown the bkl from ioctl Frederic Weisbecker
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Frederic Weisbecker @ 2010-05-19 17:24 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Jeff Dike, Uml, Thomas Gleixner,
	John Kacur, Arnd Bergmann

Pushdown the bkl to harddog_ioctl.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jeff Dike <jdike@addtoit.com>
Cc: Uml <user-mode-linux-devel@lists.sourceforge.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Kacur <jkacur@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>
---
 arch/um/drivers/harddog_kern.c |   18 +++++++++++++++---
 1 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/arch/um/drivers/harddog_kern.c b/arch/um/drivers/harddog_kern.c
index d332503..cfcac1f 100644
--- a/arch/um/drivers/harddog_kern.c
+++ b/arch/um/drivers/harddog_kern.c
@@ -124,8 +124,8 @@ static ssize_t harddog_write(struct file *file, const char __user *data, size_t
 	return 0;
 }
 
-static int harddog_ioctl(struct inode *inode, struct file *file,
-			 unsigned int cmd, unsigned long arg)
+static int harddog_ioctl_unlocked(struct file *file,
+				  unsigned int cmd, unsigned long arg)
 {
 	void __user *argp= (void __user *)arg;
 	static struct watchdog_info ident = {
@@ -148,10 +148,22 @@ static int harddog_ioctl(struct inode *inode, struct file *file,
 	}
 }
 
+static long harddog_ioctl(struct file *file,
+			  unsigned int cmd, unsigned long arg)
+{
+	long ret;
+
+	lock_kernel();
+	ret = harddog_ioctl_unlocked(file, cmd, arg);
+	unlock_kernel();
+
+	return ret;
+}
+
 static const struct file_operations harddog_fops = {
 	.owner		= THIS_MODULE,
 	.write		= harddog_write,
-	.ioctl		= harddog_ioctl,
+	.unlocked_ioctl	= harddog_ioctl,
 	.open		= harddog_open,
 	.release	= harddog_release,
 };
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 7/8] cris: Pushdown the bkl from ioctl
  2010-05-19 17:24 [PATCH 0/8] Another set of ioctl bkl pushdown, almost the end Frederic Weisbecker
                   ` (5 preceding siblings ...)
  2010-05-19 17:24 ` [PATCH 6/8] uml: Pushdown the bkl from harddog_kern ioctl Frederic Weisbecker
@ 2010-05-19 17:24 ` Frederic Weisbecker
  2010-05-19 17:24 ` [PATCH 8/8] ia64: Use unlocked_ioctl from perfmon Frederic Weisbecker
  2010-05-20 11:26 ` [PATCH 0/8] Another set of ioctl bkl pushdown, almost the end Jan Kara
  8 siblings, 0 replies; 22+ messages in thread
From: Frederic Weisbecker @ 2010-05-19 17:24 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Mikael Starvik, Jesper Nilsson, Cris,
	Thomas Gleixner, John Kacur, Arnd Bergmann

Pushdown the bkl to the remaining drivers using the
deprecated .ioctl.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mikael Starvik <starvik@axis.com>
Cc: Jesper Nilsson <jesper.nilsson@axis.com>
Cc: Cris <linux-cris-kernel@axis.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Kacur <jkacur@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>
---
 arch/cris/arch-v10/drivers/gpio.c         |   30 ++++++++++++++++++---------
 arch/cris/arch-v10/drivers/i2c.c          |   24 ++++++++++++++++-----
 arch/cris/arch-v10/drivers/sync_serial.c  |   32 +++++++++++++++++++---------
 arch/cris/arch-v32/drivers/cryptocop.c    |   24 ++++++++++++++++-----
 arch/cris/arch-v32/drivers/mach-a3/gpio.c |   28 +++++++++++++++++--------
 arch/cris/arch-v32/drivers/mach-fs/gpio.c |   29 +++++++++++++++++---------
 arch/cris/arch-v32/drivers/sync_serial.c  |   30 +++++++++++++++++++--------
 7 files changed, 137 insertions(+), 60 deletions(-)

diff --git a/arch/cris/arch-v10/drivers/gpio.c b/arch/cris/arch-v10/drivers/gpio.c
index 4b0f65f..300bd37 100644
--- a/arch/cris/arch-v10/drivers/gpio.c
+++ b/arch/cris/arch-v10/drivers/gpio.c
@@ -46,8 +46,7 @@ static char gpio_name[] = "etrax gpio";
 static wait_queue_head_t *gpio_wq;
 #endif
 
-static int gpio_ioctl(struct inode *inode, struct file *file,
-	unsigned int cmd, unsigned long arg);
+static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
 static ssize_t gpio_write(struct file *file, const char __user *buf,
 	size_t count, loff_t *off);
 static int gpio_open(struct inode *inode, struct file *filp);
@@ -505,8 +504,7 @@ static int
 gpio_leds_ioctl(unsigned int cmd, unsigned long arg);
 
 static int
-gpio_ioctl(struct inode *inode, struct file *file,
-	   unsigned int cmd, unsigned long arg)
+gpio_ioctl_unlocked(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	unsigned long flags;
 	unsigned long val;
@@ -684,6 +682,18 @@ gpio_ioctl(struct inode *inode, struct file *file,
 }
 
 static int
+gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	long ret;
+
+	lock_kernel();
+	ret = gpio_ioctl_unlocked(file, cmd, arg);
+	unlock_kernel();
+
+	return ret;
+}
+
+static int
 gpio_leds_ioctl(unsigned int cmd, unsigned long arg)
 {
 	unsigned char green;
@@ -713,12 +723,12 @@ gpio_leds_ioctl(unsigned int cmd, unsigned long arg)
 }
 
 static const struct file_operations gpio_fops = {
-	.owner       = THIS_MODULE,
-	.poll        = gpio_poll,
-	.ioctl       = gpio_ioctl,
-	.write       = gpio_write,
-	.open        = gpio_open,
-	.release     = gpio_release,
+	.owner		= THIS_MODULE,
+	.poll		= gpio_poll,
+	.unlocked_ioctl	= gpio_ioctl,
+	.write		= gpio_write,
+	.open		= gpio_open,
+	.release	= gpio_release,
 };
 
 static void ioif_watcher(const unsigned int gpio_in_available,
diff --git a/arch/cris/arch-v10/drivers/i2c.c b/arch/cris/arch-v10/drivers/i2c.c
index a8737a8..6da8580 100644
--- a/arch/cris/arch-v10/drivers/i2c.c
+++ b/arch/cris/arch-v10/drivers/i2c.c
@@ -580,8 +580,7 @@ i2c_release(struct inode *inode, struct file *filp)
  */
 
 static int
-i2c_ioctl(struct inode *inode, struct file *file,
-	  unsigned int cmd, unsigned long arg)
+i2c_ioctl_unlocked(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	if(_IOC_TYPE(cmd) != ETRAXI2C_IOCTYPE) {
 		return -EINVAL;
@@ -617,11 +616,24 @@ i2c_ioctl(struct inode *inode, struct file *file,
 	return 0;
 }
 
+static long
+i2c_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	long ret;
+
+	lock_kernel();
+	ret = i2c_ioctl_unlocked(file, cmd, arg);
+	unlock_kernel();
+
+	return ret;
+}
+
+
 static const struct file_operations i2c_fops = {
-	.owner    = THIS_MODULE,
-	.ioctl    = i2c_ioctl,
-	.open     = i2c_open,
-	.release  = i2c_release,
+	.owner    	= THIS_MODULE,
+	.unlocked_ioctl = i2c_ioctl,
+	.open		= i2c_open,
+	.release	= i2c_release,
 };
 
 int __init
diff --git a/arch/cris/arch-v10/drivers/sync_serial.c b/arch/cris/arch-v10/drivers/sync_serial.c
index 109dcd8..5295485 100644
--- a/arch/cris/arch-v10/drivers/sync_serial.c
+++ b/arch/cris/arch-v10/drivers/sync_serial.c
@@ -157,7 +157,7 @@ static int sync_serial_open(struct inode *inode, struct file *file);
 static int sync_serial_release(struct inode *inode, struct file *file);
 static unsigned int sync_serial_poll(struct file *filp, poll_table *wait);
 
-static int sync_serial_ioctl(struct inode *inode, struct file *file,
+static long sync_serial_ioctl(struct file *file,
 	unsigned int cmd, unsigned long arg);
 static ssize_t sync_serial_write(struct file *file, const char *buf,
 	size_t count, loff_t *ppos);
@@ -244,13 +244,13 @@ static unsigned sync_serial_prescale_shadow;
 #define NUMBER_OF_PORTS 2
 
 static const struct file_operations sync_serial_fops = {
-	.owner   = THIS_MODULE,
-	.write   = sync_serial_write,
-	.read    = sync_serial_read,
-	.poll    = sync_serial_poll,
-	.ioctl   = sync_serial_ioctl,
-	.open    = sync_serial_open,
-	.release = sync_serial_release
+	.owner		= THIS_MODULE,
+	.write		= sync_serial_write,
+	.read		= sync_serial_read,
+	.poll		= sync_serial_poll,
+	.unlocked_ioctl = sync_serial_ioctl,
+	.open		= sync_serial_open,
+	.release	= sync_serial_release
 };
 
 static int __init etrax_sync_serial_init(void)
@@ -678,8 +678,8 @@ static unsigned int sync_serial_poll(struct file *file, poll_table *wait)
 	return mask;
 }
 
-static int sync_serial_ioctl(struct inode *inode, struct file *file,
-		  unsigned int cmd, unsigned long arg)
+static int sync_serial_ioctl_unlocked(struct file *file,
+				      unsigned int cmd, unsigned long arg)
 {
 	int return_val = 0;
 	unsigned long flags;
@@ -956,6 +956,18 @@ static int sync_serial_ioctl(struct inode *inode, struct file *file,
 	return return_val;
 }
 
+static long sync_serial_ioctl(struct file *file,
+			      unsigned int cmd, unsigned long arg)
+{
+	long ret;
+
+	lock_kernel();
+	ret = sync_serial_ioctl_unlocked(file, cmd, arg);
+	unlock_kernel();
+
+	return ret;
+}
+
 
 static ssize_t sync_serial_write(struct file *file, const char *buf,
 	size_t count, loff_t *ppos)
diff --git a/arch/cris/arch-v32/drivers/cryptocop.c b/arch/cris/arch-v32/drivers/cryptocop.c
index b70fb34..1070bf2 100644
--- a/arch/cris/arch-v32/drivers/cryptocop.c
+++ b/arch/cris/arch-v32/drivers/cryptocop.c
@@ -217,7 +217,7 @@ static int cryptocop_open(struct inode *, struct file *);
 
 static int cryptocop_release(struct inode *, struct file *);
 
-static int cryptocop_ioctl(struct inode *inode, struct file *file,
+static long cryptocop_ioctl(struct file *file,
 			   unsigned int cmd, unsigned long arg);
 
 static void cryptocop_start_job(void);
@@ -279,10 +279,10 @@ static void print_user_dma_lists(struct cryptocop_dma_list_operation *dma_op);
 
 
 const struct file_operations cryptocop_fops = {
-	.owner =	THIS_MODULE,
-	.open =		cryptocop_open,
-	.release =	cryptocop_release,
-	.ioctl =	cryptocop_ioctl
+	.owner		= THIS_MODULE,
+	.open		= cryptocop_open,
+	.release	= cryptocop_release,
+	.unlocked_ioctl = cryptocop_ioctl,
 };
 
 
@@ -3102,7 +3102,7 @@ static int cryptocop_ioctl_create_session(struct inode *inode, struct file *filp
 	return 0;
 }
 
-static int cryptocop_ioctl(struct inode *inode, struct file *filp, unsigned int cmd, unsigned long arg)
+static int cryptocop_ioctl_unlocked(struct inode *inode, struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	int err = 0;
 	if (_IOC_TYPE(cmd) != ETRAXCRYPTOCOP_IOCTYPE) {
@@ -3134,6 +3134,18 @@ static int cryptocop_ioctl(struct inode *inode, struct file *filp, unsigned int
 	return 0;
 }
 
+static long cryptocop_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+	struct inode *inode = file->f_path.dentry->d_inode;
+	long ret;
+
+	lock_kernel();
+	ret = cryptocop_ioctl_unlocked(inode, filp, cmd, arg);
+	unlock_kernel();
+
+	return ret;
+}
+
 
 #ifdef LDEBUG
 static void print_dma_descriptors(struct cryptocop_int_operation *iop)
diff --git a/arch/cris/arch-v32/drivers/mach-a3/gpio.c b/arch/cris/arch-v32/drivers/mach-a3/gpio.c
index 97357cf..4dcfae3 100644
--- a/arch/cris/arch-v32/drivers/mach-a3/gpio.c
+++ b/arch/cris/arch-v32/drivers/mach-a3/gpio.c
@@ -72,8 +72,7 @@ static char gpio_name[] = "etrax gpio";
 static int virtual_gpio_ioctl(struct file *file, unsigned int cmd,
 			      unsigned long arg);
 #endif
-static int gpio_ioctl(struct inode *inode, struct file *file,
-	unsigned int cmd, unsigned long arg);
+static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
 static ssize_t gpio_write(struct file *file, const char __user *buf,
 	size_t count, loff_t *off);
 static int gpio_open(struct inode *inode, struct file *filp);
@@ -521,7 +520,7 @@ static inline unsigned long setget_output(struct gpio_private *priv,
 	return dir_shadow;
 } /* setget_output */
 
-static int gpio_ioctl(struct inode *inode, struct file *file,
+static int gpio_ioctl_unlocked(struct file *file,
 	unsigned int cmd, unsigned long arg)
 {
 	unsigned long flags;
@@ -664,6 +663,17 @@ static int gpio_ioctl(struct inode *inode, struct file *file,
 	return 0;
 }
 
+static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	long ret;
+
+	lock_kernel();
+	ret = gpio_ioctl_unlocked(file, cmd, arg);
+	unlock_kernel();
+
+	return ret;
+}
+
 #ifdef CONFIG_ETRAX_VIRTUAL_GPIO
 static int virtual_gpio_ioctl(struct file *file, unsigned int cmd,
 	unsigned long arg)
@@ -877,12 +887,12 @@ static int gpio_pwm_ioctl(struct gpio_private *priv, unsigned int cmd,
 }
 
 static const struct file_operations gpio_fops = {
-	.owner       = THIS_MODULE,
-	.poll        = gpio_poll,
-	.ioctl       = gpio_ioctl,
-	.write       = gpio_write,
-	.open        = gpio_open,
-	.release     = gpio_release,
+	.owner		= THIS_MODULE,
+	.poll		= gpio_poll,
+	.unlocked_ioctl = gpio_ioctl,
+	.write		= gpio_write,
+	.open		= gpio_open,
+	.release	= gpio_release,
 };
 
 #ifdef CONFIG_ETRAX_VIRTUAL_GPIO
diff --git a/arch/cris/arch-v32/drivers/mach-fs/gpio.c b/arch/cris/arch-v32/drivers/mach-fs/gpio.c
index d89ab80..d2e184c 100644
--- a/arch/cris/arch-v32/drivers/mach-fs/gpio.c
+++ b/arch/cris/arch-v32/drivers/mach-fs/gpio.c
@@ -74,8 +74,7 @@ static wait_queue_head_t *gpio_wq;
 static int virtual_gpio_ioctl(struct file *file, unsigned int cmd,
 	unsigned long arg);
 #endif
-static int gpio_ioctl(struct inode *inode, struct file *file,
-	unsigned int cmd, unsigned long arg);
+static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
 static ssize_t gpio_write(struct file *file, const char *buf, size_t count,
 	loff_t *off);
 static int gpio_open(struct inode *inode, struct file *filp);
@@ -561,8 +560,7 @@ static int
 gpio_leds_ioctl(unsigned int cmd, unsigned long arg);
 
 static int
-gpio_ioctl(struct inode *inode, struct file *file,
-	   unsigned int cmd, unsigned long arg)
+gpio_ioctl_unlocked(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	unsigned long flags;
 	unsigned long val;
@@ -707,6 +705,17 @@ gpio_ioctl(struct inode *inode, struct file *file,
 	return 0;
 }
 
+static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	long ret;
+
+	lock_kernel();
+	ret = gpio_ioctl_unlocked(file, cmd, arg);
+	unlock_kernel();
+
+	return ret;
+}
+
 #ifdef CONFIG_ETRAX_VIRTUAL_GPIO
 static int
 virtual_gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
@@ -856,12 +865,12 @@ gpio_leds_ioctl(unsigned int cmd, unsigned long arg)
 }
 
 static const struct file_operations gpio_fops = {
-	.owner       = THIS_MODULE,
-	.poll        = gpio_poll,
-	.ioctl       = gpio_ioctl,
-	.write       = gpio_write,
-	.open        = gpio_open,
-	.release     = gpio_release,
+	.owner		= THIS_MODULE,
+	.poll		= gpio_poll,
+	.unlocked_ioctl	= gpio_ioctl,
+	.write		= gpio_write,
+	.open		= gpio_open,
+	.release	= gpio_release,
 };
 
 #ifdef CONFIG_ETRAX_VIRTUAL_GPIO
diff --git a/arch/cris/arch-v32/drivers/sync_serial.c b/arch/cris/arch-v32/drivers/sync_serial.c
index 4889f19..bf6d2f9 100644
--- a/arch/cris/arch-v32/drivers/sync_serial.c
+++ b/arch/cris/arch-v32/drivers/sync_serial.c
@@ -153,7 +153,7 @@ static int sync_serial_open(struct inode *, struct file*);
 static int sync_serial_release(struct inode*, struct file*);
 static unsigned int sync_serial_poll(struct file *filp, poll_table *wait);
 
-static int sync_serial_ioctl(struct inode*, struct file*,
+static long sync_serial_ioctl(struct file *,
 			     unsigned int cmd, unsigned long arg);
 static ssize_t sync_serial_write(struct file * file, const char * buf,
 				 size_t count, loff_t *ppos);
@@ -241,13 +241,13 @@ static struct sync_port ports[]=
 #define NBR_PORTS ARRAY_SIZE(ports)
 
 static const struct file_operations sync_serial_fops = {
-	.owner   = THIS_MODULE,
-	.write   = sync_serial_write,
-	.read    = sync_serial_read,
-	.poll    = sync_serial_poll,
-	.ioctl   = sync_serial_ioctl,
-	.open    = sync_serial_open,
-	.release = sync_serial_release
+	.owner		= THIS_MODULE,
+	.write		= sync_serial_write,
+	.read		= sync_serial_read,
+	.poll		= sync_serial_poll,
+	.unlocked_ioctl = sync_serial_ioctl,
+	.open		= sync_serial_open,
+	.release	= sync_serial_release
 };
 
 static int __init etrax_sync_serial_init(void)
@@ -650,7 +650,7 @@ static unsigned int sync_serial_poll(struct file *file, poll_table *wait)
 	return mask;
 }
 
-static int sync_serial_ioctl(struct inode *inode, struct file *file,
+static int sync_serial_ioctl_unlocked(struct file *file,
 		  unsigned int cmd, unsigned long arg)
 {
 	int return_val = 0;
@@ -961,6 +961,18 @@ static int sync_serial_ioctl(struct inode *inode, struct file *file,
 	return return_val;
 }
 
+static long sync_serial_ioctl(struct file *file,
+			      unsigned int cmd, unsigned long arg)
+{
+	long ret;
+
+	lock_kernel();
+	ret = sync_serial_ioctl_unlocked(file, cmd, arg);
+	unlock_kernel();
+
+	return ret;
+}
+
 /* NOTE: sync_serial_write does not support concurrency */
 static ssize_t sync_serial_write(struct file *file, const char *buf,
 				 size_t count, loff_t *ppos)
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 8/8] ia64: Use unlocked_ioctl from perfmon
  2010-05-19 17:24 [PATCH 0/8] Another set of ioctl bkl pushdown, almost the end Frederic Weisbecker
                   ` (6 preceding siblings ...)
  2010-05-19 17:24 ` [PATCH 7/8] cris: Pushdown the bkl from ioctl Frederic Weisbecker
@ 2010-05-19 17:24 ` Frederic Weisbecker
  2010-05-20 11:26 ` [PATCH 0/8] Another set of ioctl bkl pushdown, almost the end Jan Kara
  8 siblings, 0 replies; 22+ messages in thread
From: Frederic Weisbecker @ 2010-05-19 17:24 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Tony Luck, Fenghua Yu, IA64,
	Thomas Gleixner, John Kacur, Arnd Bergmann

pfm_ioctl() obviously doesn't need the bkl just to print a
message.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: IA64 <linux-ia64@vger.kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Kacur <jkacur@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>
---
 arch/ia64/kernel/perfmon.c |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
index ab985f7..7443290 100644
--- a/arch/ia64/kernel/perfmon.c
+++ b/arch/ia64/kernel/perfmon.c
@@ -1696,8 +1696,8 @@ pfm_poll(struct file *filp, poll_table * wait)
 	return mask;
 }
 
-static int
-pfm_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
+static long
+pfm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	DPRINT(("pfm_ioctl called\n"));
 	return -EINVAL;
@@ -2174,15 +2174,15 @@ pfm_no_open(struct inode *irrelevant, struct file *dontcare)
 
 
 static const struct file_operations pfm_file_ops = {
-	.llseek   = no_llseek,
-	.read     = pfm_read,
-	.write    = pfm_write,
-	.poll     = pfm_poll,
-	.ioctl    = pfm_ioctl,
-	.open     = pfm_no_open,	/* special open code to disallow open via /proc */
-	.fasync   = pfm_fasync,
-	.release  = pfm_close,
-	.flush	  = pfm_flush
+	.llseek		= no_llseek,
+	.read		= pfm_read,
+	.write		= pfm_write,
+	.poll		= pfm_poll,
+	.unlocked_ioctl = pfm_ioctl,
+	.open		= pfm_no_open,	/* special open code to disallow open via /proc */
+	.fasync		= pfm_fasync,
+	.release	= pfm_close,
+	.flush		= pfm_flush
 };
 
 static int
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/8] autofs: Pushdown the bkl from ioctl
  2010-05-19 17:24 ` [PATCH 2/8] autofs: Pushdown the bkl from ioctl Frederic Weisbecker
@ 2010-05-19 18:02   ` H. Peter Anvin
  2010-05-19 18:08     ` Frederic Weisbecker
  0 siblings, 1 reply; 22+ messages in thread
From: H. Peter Anvin @ 2010-05-19 18:02 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Ingo Molnar, LKML, Autofs, Thomas Gleixner, John Kacur,
	Arnd Bergmann

On 05/19/2010 10:24 AM, Frederic Weisbecker wrote:
>   * generate kernel reactions
>   */
> -static int autofs_root_ioctl(struct inode *inode, struct file *filp,
> +static int autofs_root_ioctl_unlocked(struct inode *inode, struct file *filp,
>  			     unsigned int cmd, unsigned long arg)
>  {
>  	struct autofs_sb_info *sbi = autofs_sbi(inode->i_sb);
> @@ -579,3 +579,16 @@ static int autofs_root_ioctl(struct inode *inode, struct file *filp,
>  		return -ENOSYS;
>  	}
>  }
> +
> +static long autofs_root_ioctl(struct file *filp,
> +			     unsigned int cmd, unsigned long arg)
> +{

The choice of naming here seems reverse in my opinion.

	-hpa

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/8] autofs: Pushdown the bkl from ioctl
  2010-05-19 18:02   ` H. Peter Anvin
@ 2010-05-19 18:08     ` Frederic Weisbecker
  2010-05-19 18:13       ` H. Peter Anvin
  0 siblings, 1 reply; 22+ messages in thread
From: Frederic Weisbecker @ 2010-05-19 18:08 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Ingo Molnar, LKML, Autofs, Thomas Gleixner, John Kacur,
	Arnd Bergmann

On Wed, May 19, 2010 at 11:02:04AM -0700, H. Peter Anvin wrote:
> On 05/19/2010 10:24 AM, Frederic Weisbecker wrote:
> >   * generate kernel reactions
> >   */
> > -static int autofs_root_ioctl(struct inode *inode, struct file *filp,
> > +static int autofs_root_ioctl_unlocked(struct inode *inode, struct file *filp,
> >  			     unsigned int cmd, unsigned long arg)
> >  {
> >  	struct autofs_sb_info *sbi = autofs_sbi(inode->i_sb);
> > @@ -579,3 +579,16 @@ static int autofs_root_ioctl(struct inode *inode, struct file *filp,
> >  		return -ENOSYS;
> >  	}
> >  }
> > +
> > +static long autofs_root_ioctl(struct file *filp,
> > +			     unsigned int cmd, unsigned long arg)
> > +{
> 
> The choice of naming here seems reverse in my opinion.


Oh, why?

The function that holds the bkl calls its unlocked version.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/8] autofs: Pushdown the bkl from ioctl
  2010-05-19 18:08     ` Frederic Weisbecker
@ 2010-05-19 18:13       ` H. Peter Anvin
  2010-05-19 18:22         ` Frederic Weisbecker
  2010-05-19 19:03         ` Frederic Weisbecker
  0 siblings, 2 replies; 22+ messages in thread
From: H. Peter Anvin @ 2010-05-19 18:13 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Ingo Molnar, LKML, Autofs, Thomas Gleixner, John Kacur,
	Arnd Bergmann

On 05/19/2010 11:08 AM, Frederic Weisbecker wrote:
> On Wed, May 19, 2010 at 11:02:04AM -0700, H. Peter Anvin wrote:
>> On 05/19/2010 10:24 AM, Frederic Weisbecker wrote:
>>>   * generate kernel reactions
>>>   */
>>> -static int autofs_root_ioctl(struct inode *inode, struct file *filp,
>>> +static int autofs_root_ioctl_unlocked(struct inode *inode, struct file *filp,
>>>  			     unsigned int cmd, unsigned long arg)
>>>  {
>>>  	struct autofs_sb_info *sbi = autofs_sbi(inode->i_sb);
>>> @@ -579,3 +579,16 @@ static int autofs_root_ioctl(struct inode *inode, struct file *filp,
>>>  		return -ENOSYS;
>>>  	}
>>>  }
>>> +
>>> +static long autofs_root_ioctl(struct file *filp,
>>> +			     unsigned int cmd, unsigned long arg)
>>> +{
>>
>> The choice of naming here seems reverse in my opinion.
> 
> 
> Oh, why?
> 
> The function that holds the bkl calls its unlocked version.
> 

But it's not ... it is locked at that point.  It's not lock*ing*, but it
is not *unlocked*, either.  Furthermore, it is directly contradicting
the naming scheme of the ops structure.

	-hpa


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/8] autofs: Pushdown the bkl from ioctl
  2010-05-19 18:13       ` H. Peter Anvin
@ 2010-05-19 18:22         ` Frederic Weisbecker
  2010-05-19 19:03         ` Frederic Weisbecker
  1 sibling, 0 replies; 22+ messages in thread
From: Frederic Weisbecker @ 2010-05-19 18:22 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Ingo Molnar, LKML, Autofs, Thomas Gleixner, John Kacur,
	Arnd Bergmann

On Wed, May 19, 2010 at 11:13:50AM -0700, H. Peter Anvin wrote:
> On 05/19/2010 11:08 AM, Frederic Weisbecker wrote:
> > On Wed, May 19, 2010 at 11:02:04AM -0700, H. Peter Anvin wrote:
> >> On 05/19/2010 10:24 AM, Frederic Weisbecker wrote:
> >>>   * generate kernel reactions
> >>>   */
> >>> -static int autofs_root_ioctl(struct inode *inode, struct file *filp,
> >>> +static int autofs_root_ioctl_unlocked(struct inode *inode, struct file *filp,
> >>>  			     unsigned int cmd, unsigned long arg)
> >>>  {
> >>>  	struct autofs_sb_info *sbi = autofs_sbi(inode->i_sb);
> >>> @@ -579,3 +579,16 @@ static int autofs_root_ioctl(struct inode *inode, struct file *filp,
> >>>  		return -ENOSYS;
> >>>  	}
> >>>  }
> >>> +
> >>> +static long autofs_root_ioctl(struct file *filp,
> >>> +			     unsigned int cmd, unsigned long arg)
> >>> +{
> >>
> >> The choice of naming here seems reverse in my opinion.
> > 
> > 
> > Oh, why?
> > 
> > The function that holds the bkl calls its unlocked version.
> > 
> 
> But it's not ... it is locked at that point.  It's not lock*ing*, but it
> is not *unlocked*, either.  Furthermore, it is directly contradicting
> the naming scheme of the ops structure.


It depends on the point of view. The function itself doesn't lock, it is the
"naked point", so if you take it, you need to lock before, that's what the
name wants to tell.

But may be that's the opposite point of view than the common one, for
which I wouldn't be suprised as my brain tends to be upside down...


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/8] autofs: Pushdown the bkl from ioctl
  2010-05-19 18:13       ` H. Peter Anvin
  2010-05-19 18:22         ` Frederic Weisbecker
@ 2010-05-19 19:03         ` Frederic Weisbecker
  2010-05-19 20:04           ` H. Peter Anvin
  1 sibling, 1 reply; 22+ messages in thread
From: Frederic Weisbecker @ 2010-05-19 19:03 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Ingo Molnar, LKML, Autofs, Thomas Gleixner, John Kacur,
	Arnd Bergmann

On Wed, May 19, 2010 at 11:13:50AM -0700, H. Peter Anvin wrote:
> On 05/19/2010 11:08 AM, Frederic Weisbecker wrote:
> > On Wed, May 19, 2010 at 11:02:04AM -0700, H. Peter Anvin wrote:
> >> On 05/19/2010 10:24 AM, Frederic Weisbecker wrote:
> >>>   * generate kernel reactions
> >>>   */
> >>> -static int autofs_root_ioctl(struct inode *inode, struct file *filp,
> >>> +static int autofs_root_ioctl_unlocked(struct inode *inode, struct file *filp,
> >>>  			     unsigned int cmd, unsigned long arg)
> >>>  {
> >>>  	struct autofs_sb_info *sbi = autofs_sbi(inode->i_sb);
> >>> @@ -579,3 +579,16 @@ static int autofs_root_ioctl(struct inode *inode, struct file *filp,
> >>>  		return -ENOSYS;
> >>>  	}
> >>>  }
> >>> +
> >>> +static long autofs_root_ioctl(struct file *filp,
> >>> +			     unsigned int cmd, unsigned long arg)
> >>> +{
> >>
> >> The choice of naming here seems reverse in my opinion.
> > 
> > 
> > Oh, why?
> > 
> > The function that holds the bkl calls its unlocked version.
> > 
> 
> But it's not ... it is locked at that point.  It's not lock*ing*, but it
> is not *unlocked*, either.  Furthermore, it is directly contradicting
> the naming scheme of the ops structure.
> 
> 	-hpa
> 


Would you prefer me to resend a patch?


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/8] autofs: Pushdown the bkl from ioctl
  2010-05-19 19:03         ` Frederic Weisbecker
@ 2010-05-19 20:04           ` H. Peter Anvin
  2010-05-20 11:35             ` Ian Kent
  0 siblings, 1 reply; 22+ messages in thread
From: H. Peter Anvin @ 2010-05-19 20:04 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Ingo Molnar, LKML, Autofs, Thomas Gleixner, John Kacur,
	Arnd Bergmann, Ian Kent

On 05/19/2010 12:03 PM, Frederic Weisbecker wrote:
>>
>> But it's not ... it is locked at that point.  It's not lock*ing*, but it
>> is not *unlocked*, either.  Furthermore, it is directly contradicting
>> the naming scheme of the ops structure.
>>
> 
> Would you prefer me to resend a patch?
> 

Yes, that would be my preference.

We really need to set a date to kill off autofs3 completely -- I don't
care if we rename anything or not -- and add it to feature-removal.txt.

	-hpa

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/8] Another set of ioctl bkl pushdown, almost the end
  2010-05-19 17:24 [PATCH 0/8] Another set of ioctl bkl pushdown, almost the end Frederic Weisbecker
                   ` (7 preceding siblings ...)
  2010-05-19 17:24 ` [PATCH 8/8] ia64: Use unlocked_ioctl from perfmon Frederic Weisbecker
@ 2010-05-20 11:26 ` Jan Kara
  2010-05-20 11:28   ` Frederic Weisbecker
  8 siblings, 1 reply; 22+ messages in thread
From: Jan Kara @ 2010-05-20 11:26 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Ingo Molnar, LKML, Thomas Gleixner, John Kacur, Arnd Bergmann,
	Tyler Hicks, Dustin Kirkland, H . Peter Anvin, Ian Kent,
	J . Bruce Fields, Neil Brown, Jeff Dike, Mikael Starvik,
	Jesper Nilsson, Tony Luck, Fenghua Yu, Jan Kara, Joern Engel,
	Mauro Carvalho Chehab

On Wed 19-05-10 19:24:07, Frederic Weisbecker wrote:
> We are very close to the end of struct file_operations::ioctl
> 
> What remains:
...
> 	udf: Approved but not yet included in udf tree
  It is included in udf tree and I will push it to Linus in this merge
window...

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/8] Another set of ioctl bkl pushdown, almost the end
  2010-05-20 11:26 ` [PATCH 0/8] Another set of ioctl bkl pushdown, almost the end Jan Kara
@ 2010-05-20 11:28   ` Frederic Weisbecker
  0 siblings, 0 replies; 22+ messages in thread
From: Frederic Weisbecker @ 2010-05-20 11:28 UTC (permalink / raw)
  To: Jan Kara
  Cc: Ingo Molnar, LKML, Thomas Gleixner, John Kacur, Arnd Bergmann,
	Tyler Hicks, Dustin Kirkland, H . Peter Anvin, Ian Kent,
	J . Bruce Fields, Neil Brown, Jeff Dike, Mikael Starvik,
	Jesper Nilsson, Tony Luck, Fenghua Yu, Joern Engel,
	Mauro Carvalho Chehab

On Thu, May 20, 2010 at 01:26:56PM +0200, Jan Kara wrote:
> On Wed 19-05-10 19:24:07, Frederic Weisbecker wrote:
> > We are very close to the end of struct file_operations::ioctl
> > 
> > What remains:
> ...
> > 	udf: Approved but not yet included in udf tree
>   It is included in udf tree and I will push it to Linus in this merge
> window...


Thanks!


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/8] autofs: Pushdown the bkl from ioctl
  2010-05-19 20:04           ` H. Peter Anvin
@ 2010-05-20 11:35             ` Ian Kent
  0 siblings, 0 replies; 22+ messages in thread
From: Ian Kent @ 2010-05-20 11:35 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Frederic Weisbecker, Ingo Molnar, LKML, Autofs, Thomas Gleixner,
	John Kacur, Arnd Bergmann

On Wed, 2010-05-19 at 13:04 -0700, H. Peter Anvin wrote:
> On 05/19/2010 12:03 PM, Frederic Weisbecker wrote:
> >>
> >> But it's not ... it is locked at that point.  It's not lock*ing*, but it
> >> is not *unlocked*, either.  Furthermore, it is directly contradicting
> >> the naming scheme of the ops structure.
> >>
> > 
> > Would you prefer me to resend a patch?
> > 
> 
> Yes, that would be my preference.
> 
> We really need to set a date to kill off autofs3 completely -- I don't
> care if we rename anything or not -- and add it to feature-removal.txt.

Yes, my bad.
I have to get onto that, sorry.

Ian


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/8] ecryptfs: Pushdown the bkl from ioctl
  2010-05-19 17:24 ` [PATCH 1/8] ecryptfs: Pushdown the bkl from ioctl Frederic Weisbecker
@ 2010-05-20 23:39   ` Tyler Hicks
  2010-05-20 23:42     ` [PATCH] vfs/eCryptfs: Handle ioctl calls with unlocked and compat functions Tyler Hicks
  0 siblings, 1 reply; 22+ messages in thread
From: Tyler Hicks @ 2010-05-20 23:39 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Ingo Molnar, LKML, Dustin Kirkland, Ecryptfs, Thomas Gleixner,
	John Kacur, Arnd Bergmann, Alexander Viro

On Wed May 19, 2010 at 07:24:08PM +0200, Frederic Weisbecker (fweisbec@gmail.com) was quoted:
> Pushdown the bkl to ecryptfs_ioctl.

I've been sitting on a bug fix for a while that cleans up the eCryptfs
ioctl code. eCryptfs doesn't need the BKL itself and can leave that
decision up to the lower file sytem. The patch I have written should
cover what you're trying to do here, too. I'll respond with the patch
and if you think it looks good, I'll carry it in my tree and ask Linus
to pull soon.

I reuse vfs_ioctl(), which must be exported, so I should get Al's ack.

> 
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Tyler Hicks <tyhicks@linux.vnet.ibm.com>
> Cc: Dustin Kirkland <kirkland@canonical.com>
> Cc: Ecryptfs <ecryptfs-devel@lists.launchpad.net>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: John Kacur <jkacur@redhat.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> ---
>  fs/ecryptfs/file.c |   23 ++++++++++++++++++-----
>  1 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
> index e7440a6..9352613 100644
> --- a/fs/ecryptfs/file.c
> +++ b/fs/ecryptfs/file.c
> @@ -294,12 +294,12 @@ static int ecryptfs_fasync(int fd, struct file *file, int flag)
>  	return rc;
>  }
> 
> -static int ecryptfs_ioctl(struct inode *inode, struct file *file,
> +static long ecryptfs_ioctl(struct file *file,
>  			  unsigned int cmd, unsigned long arg);
> 
>  const struct file_operations ecryptfs_dir_fops = {
>  	.readdir = ecryptfs_readdir,
> -	.ioctl = ecryptfs_ioctl,
> +	.unlocked_ioctl = ecryptfs_ioctl,
>  	.open = ecryptfs_open,
>  	.flush = ecryptfs_flush,
>  	.release = ecryptfs_release,
> @@ -315,7 +315,7 @@ const struct file_operations ecryptfs_main_fops = {
>  	.write = do_sync_write,
>  	.aio_write = generic_file_aio_write,
>  	.readdir = ecryptfs_readdir,
> -	.ioctl = ecryptfs_ioctl,
> +	.unlocked_ioctl = ecryptfs_ioctl,
>  	.mmap = generic_file_mmap,
>  	.open = ecryptfs_open,
>  	.flush = ecryptfs_flush,
> @@ -326,8 +326,8 @@ const struct file_operations ecryptfs_main_fops = {
>  };
> 
>  static int
> -ecryptfs_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
> -	       unsigned long arg)
> +ecryptfs_ioctl_unlocked(struct inode *inode, struct file *file,
> +			unsigned int cmd, unsigned long arg)
>  {
>  	int rc = 0;
>  	struct file *lower_file = NULL;
> @@ -341,3 +341,16 @@ ecryptfs_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
>  		rc = -ENOTTY;
>  	return rc;
>  }
> +
> +static long ecryptfs_ioctl(struct file *file, unsigned int cmd,
> +			   unsigned long arg)
> +{
> +	long ret;
> +	struct inode *inode = file->f_dentry->d_inode;
> +
> +	lock_kernel();
> +	ret = ecryptfs_ioctl_unlocked(inode, file, cmd, arg);
> +	unlock_kernel();
> +
> +	return ret;
> +}
> -- 
> 1.6.2.3
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH] vfs/eCryptfs: Handle ioctl calls with unlocked and compat functions
  2010-05-20 23:39   ` Tyler Hicks
@ 2010-05-20 23:42     ` Tyler Hicks
  2010-05-21  6:25       ` Arnd Bergmann
  0 siblings, 1 reply; 22+ messages in thread
From: Tyler Hicks @ 2010-05-20 23:42 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Ingo Molnar, LKML, Dustin Kirkland, Ecryptfs, Thomas Gleixner,
	John Kacur, Arnd Bergmann, Alexander Viro

Lower filesystems that only implement unlocked_ioctl aren't being
passed ioctl calls because eCryptfs only checked for
lower_file->f_op->ioctl and returned -ENOTTY if it was NULL.

eCryptfs shouldn't implement ioctl(), since it doesn't require the BKL.
Instead, unlocked_ioctl() should be used and vfs_ioctl() can be called
on the lower file since it handles locking, if necessary.  This requires
vfs_ioctl() to be exported.

Also implements compat_ioctl() function by simply passing the call on
to the lower filesystem's compat_ioctl() function.

Reported-by: James Dupin <james.dupin@gmail.com>
Signed-off-by: Tyler Hicks <tyhicks@linux.vnet.ibm.com>
---
 fs/ecryptfs/file.c |   56 ++++++++++++++++++++++++++++++++-------------------
 fs/ioctl.c         |    4 +-
 include/linux/fs.h |    1 +
 3 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index e7440a6..4632ac8 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -294,12 +294,40 @@ static int ecryptfs_fasync(int fd, struct file *file, int flag)
 	return rc;
 }
 
-static int ecryptfs_ioctl(struct inode *inode, struct file *file,
-			  unsigned int cmd, unsigned long arg);
+static long
+ecryptfs_unlocked_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	long rc = -ENOTTY;
+	struct file *lower_file = NULL;
+
+	if (ecryptfs_file_to_private(file))
+		lower_file = ecryptfs_file_to_lower(file);
+	if (lower_file)
+		rc = vfs_ioctl(lower_file, cmd, arg);
+	return rc;
+}
+
+#ifdef CONFIG_COMPAT
+static long
+ecryptfs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	long rc = -ENOTTY;
+	struct file *lower_file = NULL;
+
+	if (ecryptfs_file_to_private(file))
+		lower_file = ecryptfs_file_to_lower(file);
+	if (lower_file && lower_file->f_op && lower_file->f_op->compat_ioctl)
+		rc = lower_file->f_op->compat_ioctl(lower_file, cmd, arg);
+	return rc;
+}
+#endif
 
 const struct file_operations ecryptfs_dir_fops = {
 	.readdir = ecryptfs_readdir,
-	.ioctl = ecryptfs_ioctl,
+	.unlocked_ioctl = ecryptfs_unlocked_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl = ecryptfs_compat_ioctl,
+#endif
 	.open = ecryptfs_open,
 	.flush = ecryptfs_flush,
 	.release = ecryptfs_release,
@@ -315,7 +343,10 @@ const struct file_operations ecryptfs_main_fops = {
 	.write = do_sync_write,
 	.aio_write = generic_file_aio_write,
 	.readdir = ecryptfs_readdir,
-	.ioctl = ecryptfs_ioctl,
+	.unlocked_ioctl = ecryptfs_unlocked_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl = ecryptfs_compat_ioctl,
+#endif
 	.mmap = generic_file_mmap,
 	.open = ecryptfs_open,
 	.flush = ecryptfs_flush,
@@ -324,20 +355,3 @@ const struct file_operations ecryptfs_main_fops = {
 	.fasync = ecryptfs_fasync,
 	.splice_read = generic_file_splice_read,
 };
-
-static int
-ecryptfs_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
-	       unsigned long arg)
-{
-	int rc = 0;
-	struct file *lower_file = NULL;
-
-	if (ecryptfs_file_to_private(file))
-		lower_file = ecryptfs_file_to_lower(file);
-	if (lower_file && lower_file->f_op && lower_file->f_op->ioctl)
-		rc = lower_file->f_op->ioctl(ecryptfs_inode_to_lower(inode),
-					     lower_file, cmd, arg);
-	else
-		rc = -ENOTTY;
-	return rc;
-}
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 7faefb4..549b8e9 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -34,8 +34,7 @@
  *
  * Returns 0 on success, -errno on error.
  */
-static long vfs_ioctl(struct file *filp, unsigned int cmd,
-		      unsigned long arg)
+long vfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	int error = -ENOTTY;
 
@@ -57,6 +56,7 @@ static long vfs_ioctl(struct file *filp, unsigned int cmd,
  out:
 	return error;
 }
+EXPORT_SYMBOL(vfs_ioctl);
 
 static int ioctl_fibmap(struct file *filp, int __user *p)
 {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 44f35ae..d862c81 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1922,6 +1922,7 @@ extern char * getname(const char __user *);
 /* fs/ioctl.c */
 
 extern int ioctl_preallocate(struct file *filp, void __user *argp);
+extern long vfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
 
 /* fs/dcache.c */
 extern void __init vfs_caches_init_early(void);
-- 
1.7.0.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] vfs/eCryptfs: Handle ioctl calls with unlocked and compat functions
  2010-05-20 23:42     ` [PATCH] vfs/eCryptfs: Handle ioctl calls with unlocked and compat functions Tyler Hicks
@ 2010-05-21  6:25       ` Arnd Bergmann
  2010-05-21  7:03         ` Frederic Weisbecker
  0 siblings, 1 reply; 22+ messages in thread
From: Arnd Bergmann @ 2010-05-21  6:25 UTC (permalink / raw)
  To: Tyler Hicks
  Cc: Frederic Weisbecker, Ingo Molnar, LKML, Dustin Kirkland, Ecryptfs,
	Thomas Gleixner, John Kacur, Alexander Viro

On Friday 21 May 2010, Tyler Hicks wrote:
> Lower filesystems that only implement unlocked_ioctl aren't being
> passed ioctl calls because eCryptfs only checked for
> lower_file->f_op->ioctl and returned -ENOTTY if it was NULL.
> 
> eCryptfs shouldn't implement ioctl(), since it doesn't require the BKL.
> Instead, unlocked_ioctl() should be used and vfs_ioctl() can be called
> on the lower file since it handles locking, if necessary.  This requires
> vfs_ioctl() to be exported.

Calling vfs_ioctl doesn't help you at all here, you could simply call
the ->unlocked_ioctl function of the lower fs directly to do the same,
because ->ioctl will be gone soon.

You are howevers still missing a few calls that are done through do_vfs_ioctl
or file_ioctl.  To implement these, you need to add the file and super operations
that these call and forward the functions to the lower fs.

> +#ifdef CONFIG_COMPAT
> +static long
> +ecryptfs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> +	long rc = -ENOTTY;
> +	struct file *lower_file = NULL;
> +
> +	if (ecryptfs_file_to_private(file))
> +		lower_file = ecryptfs_file_to_lower(file);
> +	if (lower_file && lower_file->f_op && lower_file->f_op->compat_ioctl)
> +		rc = lower_file->f_op->compat_ioctl(lower_file, cmd, arg);
> +	return rc;
> +}
> +#endif

You need to return -ENOIOCTLCMD here, not ENOTTY to cover the case where
the lower file system does not have a ->compat_ioctl function but has its
calls listed in fs/compat_ioctl.c.

	Arnd

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] vfs/eCryptfs: Handle ioctl calls with unlocked and compat functions
  2010-05-21  6:25       ` Arnd Bergmann
@ 2010-05-21  7:03         ` Frederic Weisbecker
  0 siblings, 0 replies; 22+ messages in thread
From: Frederic Weisbecker @ 2010-05-21  7:03 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Tyler Hicks, Ingo Molnar, LKML, Dustin Kirkland, Ecryptfs,
	Thomas Gleixner, John Kacur, Alexander Viro

On Fri, May 21, 2010 at 08:25:32AM +0200, Arnd Bergmann wrote:
> On Friday 21 May 2010, Tyler Hicks wrote:
> > Lower filesystems that only implement unlocked_ioctl aren't being
> > passed ioctl calls because eCryptfs only checked for
> > lower_file->f_op->ioctl and returned -ENOTTY if it was NULL.
> > 
> > eCryptfs shouldn't implement ioctl(), since it doesn't require the BKL.
> > Instead, unlocked_ioctl() should be used and vfs_ioctl() can be called
> > on the lower file since it handles locking, if necessary.  This requires
> > vfs_ioctl() to be exported.
> 
> Calling vfs_ioctl doesn't help you at all here, you could simply call
> the ->unlocked_ioctl function of the lower fs directly to do the same,
> because ->ioctl will be gone soon.



Yeah. Nothing is left pending in the fs tree wrt ioctl pushdown so this
is safe.



> You are howevers still missing a few calls that are done through do_vfs_ioctl
> or file_ioctl.  To implement these, you need to add the file and super operations
> that these call and forward the functions to the lower fs.
> 
> > +#ifdef CONFIG_COMPAT
> > +static long
> > +ecryptfs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> > +{
> > +	long rc = -ENOTTY;
> > +	struct file *lower_file = NULL;
> > +
> > +	if (ecryptfs_file_to_private(file))
> > +		lower_file = ecryptfs_file_to_lower(file);
> > +	if (lower_file && lower_file->f_op && lower_file->f_op->compat_ioctl)
> > +		rc = lower_file->f_op->compat_ioctl(lower_file, cmd, arg);
> > +	return rc;
> > +}
> > +#endif
> 
> You need to return -ENOIOCTLCMD here, not ENOTTY to cover the case where
> the lower file system does not have a ->compat_ioctl function but has its
> calls listed in fs/compat_ioctl.c.
> 
> 	Arnd



Right.

So I'll drop the pushdown from my tree and let Tyler handle that.

Thanks.


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2010-05-21  7:03 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-19 17:24 [PATCH 0/8] Another set of ioctl bkl pushdown, almost the end Frederic Weisbecker
2010-05-19 17:24 ` [PATCH 1/8] ecryptfs: Pushdown the bkl from ioctl Frederic Weisbecker
2010-05-20 23:39   ` Tyler Hicks
2010-05-20 23:42     ` [PATCH] vfs/eCryptfs: Handle ioctl calls with unlocked and compat functions Tyler Hicks
2010-05-21  6:25       ` Arnd Bergmann
2010-05-21  7:03         ` Frederic Weisbecker
2010-05-19 17:24 ` [PATCH 2/8] autofs: Pushdown the bkl from ioctl Frederic Weisbecker
2010-05-19 18:02   ` H. Peter Anvin
2010-05-19 18:08     ` Frederic Weisbecker
2010-05-19 18:13       ` H. Peter Anvin
2010-05-19 18:22         ` Frederic Weisbecker
2010-05-19 19:03         ` Frederic Weisbecker
2010-05-19 20:04           ` H. Peter Anvin
2010-05-20 11:35             ` Ian Kent
2010-05-19 17:24 ` [PATCH 3/8] autofs4: " Frederic Weisbecker
2010-05-19 17:24 ` [PATCH 4/8] sunrpc: " Frederic Weisbecker
2010-05-19 17:24 ` [PATCH 5/8] sunrpc: Pushdown the bkl from sunrpc cache ioctl Frederic Weisbecker
2010-05-19 17:24 ` [PATCH 6/8] uml: Pushdown the bkl from harddog_kern ioctl Frederic Weisbecker
2010-05-19 17:24 ` [PATCH 7/8] cris: Pushdown the bkl from ioctl Frederic Weisbecker
2010-05-19 17:24 ` [PATCH 8/8] ia64: Use unlocked_ioctl from perfmon Frederic Weisbecker
2010-05-20 11:26 ` [PATCH 0/8] Another set of ioctl bkl pushdown, almost the end Jan Kara
2010-05-20 11:28   ` Frederic Weisbecker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).