Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756535AbXI1SOU (ORCPT ); Fri, 28 Sep 2007 14:14:20 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1755644AbXI1SNl (ORCPT ); Fri, 28 Sep 2007 14:13:41 -0400 Received: from e5.ny.us.ibm.com ([32.97.182.145]:40865 "EHLO e5.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754849AbXI1SNj (ORCPT ); Fri, 28 Sep 2007 14:13:39 -0400 Subject: [RFC][PATCH 3/8] move mnt_want_write() out of may_open() To: linux-kernel@vger.kernel.org Cc: hch@infradead.org, miklos@szeredi.hu, Dave Hansen From: Dave Hansen Date: Fri, 28 Sep 2007 11:13:33 -0700 References: <20070928181330.27B367AC@kernel> In-Reply-To: <20070928181330.27B367AC@kernel> Message-Id: <20070928181333.F3073089@kernel> Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4183 Lines: 145 may_open() can fail in a lot of ways. It is also named such that it doesn't appear to be _taking_ action, just checking a bunch of conditions. So, it makes a poor place to take and release the mnt writer count. This moves the burder of taking the mnt writer counts into the callers. The callers have the advantage of much more visibility of the filp. Since we rely on the __fput(filp) to release the mnt writer count, this is a good thing. Signed-off-by: Dave Hansen --- lxc-dave/fs/namei.c | 41 +++++++++++++++++++++++------------------ lxc-dave/fs/nfsctl.c | 20 ++++++++++++++++++-- 2 files changed, 41 insertions(+), 20 deletions(-) diff -puN fs/namei.c~move-mnt_want_write-out-of-may_open fs/namei.c --- lxc/fs/namei.c~move-mnt_want_write-out-of-may_open 2007-09-28 11:03:20.000000000 -0700 +++ lxc-dave/fs/namei.c 2007-09-28 11:03:20.000000000 -0700 @@ -1621,14 +1621,6 @@ int may_open(struct nameidata *nd, int a return -EACCES; flag &= ~O_TRUNC; - } else if (flag & FMODE_WRITE) { - /* - * effectively: !special_file() - * balanced by __fput() - */ - error = mnt_want_write(nd->mnt); - if (error) - return error; } error = vfs_permission(nd, acc_mode); @@ -1687,10 +1679,8 @@ static int open_namei_create(struct name struct dentry *dir = nd->dentry; /* - * This ensures that the mnt stays writable - * over the vfs_create() call to may_open(), - * which takes a more persistent - * mnt_want_write(). + * This mnt_want_write() is potentially persistent, + * and balanced in __fput() */ error = mnt_want_write(nd->mnt); if (error) { @@ -1710,14 +1700,18 @@ static int open_namei_create(struct name /* Don't check for write permission, don't truncate */ error = may_open(nd, 0, flag & ~O_TRUNC); out: - /* - * if needed, may_open() has taken a write - * on the mnt for us, so we can release ours. - */ - mnt_drop_write(nd->mnt); + if (error) + mnt_drop_write(nd->mnt); return error; } +static inline int write_may_occur_to_file(struct inode *inode, int acc_mode) +{ + if (special_file(inode->i_mode)) + return 0; + return (acc_mode & FMODE_WRITE); +} + /* * open_namei() * @@ -1831,9 +1825,20 @@ do_last: if (path.dentry->d_inode && S_ISDIR(path.dentry->d_inode->i_mode)) goto exit; ok: - error = may_open(nd, acc_mode, flag); + /* + * This mnt_want_write() is potentially persistent, + * and balanced in __fput() + */ + if (write_may_occur_to_file(nd->dentry->d_inode, acc_mode)) + error = mnt_want_write(nd->mnt); if (error) goto exit; + error = may_open(nd, acc_mode, flag); + if (error) { + if (write_may_occur_to_file(nd->dentry->d_inode, acc_mode)) + mnt_drop_write(nd->mnt); + goto exit; + } return 0; exit_mutex_unlock: diff -puN fs/nfsctl.c~move-mnt_want_write-out-of-may_open fs/nfsctl.c --- lxc/fs/nfsctl.c~move-mnt_want_write-out-of-may_open 2007-09-28 11:03:20.000000000 -0700 +++ lxc-dave/fs/nfsctl.c 2007-09-28 11:03:20.000000000 -0700 @@ -22,6 +22,7 @@ static struct file *do_open(char *name, int flags) { + struct file *filp; struct nameidata nd; struct vfsmount *mnt; int error; @@ -35,14 +36,29 @@ static struct file *do_open(char *name, if (error) return ERR_PTR(error); + /* + * This is balanced when you __fput() the 'struct file' + * returned by dentry_open() below. If the may_open() + * ever stops containing FMODE_WRITE, this needs to + * be changed. + */ + error = mnt_want_write(mnt); + if (error) + return ERR_PTR(error); if (flags == O_RDWR) error = may_open(&nd,MAY_READ|MAY_WRITE,FMODE_READ|FMODE_WRITE); else error = may_open(&nd, MAY_WRITE, FMODE_WRITE); - if (!error) - return dentry_open(nd.dentry, nd.mnt, flags); + if (error) + goto out; + filp = dentry_open(nd.dentry, nd.mnt, flags); + if (!IS_ERR(filp)) + return filp; + error = PTR_ERR(error); +out: + mnt_drop_write(mnt); path_release(&nd); return ERR_PTR(error); } _ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/