Mount writer count and read-only bind mounts (v4)

RFC PATCH In review Dave Hansen 2 participants 29 messages Jul 12, 2006 – Jul 13, 2006

Subsystems (21)

Networking Memory management VFS USB +17 more subsystems

lore

Share & tools

lore

Dave Hansen

author

2006-07-12 18:17 UTC #0 lore raw

cover Mount writer count and read-only bind mounts (v4)

Tries to incorporate comments from Al:
http://article.gmane.org/gmane.linux.kernel/421029

Quoted 2 lines · Al

b) figuring out what (if anything) should be done with propagation when we have shared subtrees... (not trivial at all)

Talked with Ram: Shared subtrees are about having identical views into the filesystem. Changing the mount permissions doesn't affect the view of the filesystem, so it should not be propagated.

The things that probably need the heaviest review in here are the i_nlink monitoring patch (including the inode state flag patches 03 and 06) and the new MNT_SB_WRITABLE flag (patch 05).

---

The following series implements read-only bind mounts. This feature allows a read-only view into a read-write filesystem. In the process of doing that, it also provides infrastructure for keeping track of the number of writers to any given mount. In this version, if there are writers on a superblock, the filesystem may not be remounted r/o. The same goes for MS_BIND mounts, and writers on a vfsmount.

This has a number of uses. It allows chroots to have parts of filesystems writable. It will be useful for containers in the future and is intended to replace patches that vserver has had out of the tree for several years. It allows security enhancement by making sure that parts of your filesystem read-only, when you don't want to have entire new filesystems mounted, or when you want atime selectively updated.

This set makes no attempt to keep the return codes for these r/o bind mounts the same as for a real r/o filesystem or device. It would require significantly more code and be quite a bit more invasive.

Using this feature requires two steps:

	mount --bind /source /dest
	mount -o remount,ro  /dest

Signed-off-by Dave Hansen <[email protected]>

Dave Hansen

author

2006-07-12 18:24 UTC #1 lore raw

RFC PATCH 1/27 prepare for write access checks: collapse if()

We're shortly going to be adding a bunch more permission checks in these functions. That requires adding either a bunch of new if() conditions, or some gotos. This patch collapses existing if()s and uses gotos instead to prepare for the upcoming changes.

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/namei.c |   93 +++++++++++++++++++++++++++-------------------------
 lxc-dave/fs/open.c  |   64 ++++++++++++++++++++---------------
 2 files changed, 87 insertions(+), 70 deletions(-)

fs/namei.c renamed from fs/namei.c~B-prepwork-collapse-ifs +50 ~~-43~~

diff -puN fs/namei.c~B-prepwork-collapse-ifs fs/namei.c
@@ -1911,30 +1911,32 @@ asmlinkage long sys_mkdirat(int dfd, con
 {
 	int error = 0;
 	char * tmp;
+	struct dentry *dentry;
+	struct nameidata nd;
 
 	tmp = getname(pathname);
 	error = PTR_ERR(tmp);
-	if (!IS_ERR(tmp)) {
-		struct dentry *dentry;
-		struct nameidata nd;
+	if (IS_ERR(tmp))
+		goto out_err;
 
-		error = do_path_lookup(dfd, tmp, LOOKUP_PARENT, &nd);
-		if (error)
-			goto out;
-		dentry = lookup_create(&nd, 1);
-		error = PTR_ERR(dentry);
-		if (!IS_ERR(dentry)) {
-			if (!IS_POSIXACL(nd.dentry->d_inode))
-				mode &= ~current->fs->umask;
-			error = vfs_mkdir(nd.dentry->d_inode, dentry, mode);
-			dput(dentry);
-		}
-		mutex_unlock(&nd.dentry->d_inode->i_mutex);
-		path_release(&nd);
-out:
-		putname(tmp);
-	}
+	error = do_path_lookup(dfd, tmp, LOOKUP_PARENT, &nd);
+	if (error)
+		goto out;
+	dentry = lookup_create(&nd, 1);
+	error = PTR_ERR(dentry);
+	if (IS_ERR(dentry))
+		goto out_unlock;
 
+	if (!IS_POSIXACL(nd.dentry->d_inode))
+		mode &= ~current->fs->umask;
+	error = vfs_mkdir(nd.dentry->d_inode, dentry, mode);
+	dput(dentry);
+out_unlock:
+	mutex_unlock(&nd.dentry->d_inode->i_mutex);
+	path_release(&nd);
+out:
+	putname(tmp);
+out_err:
 	return error;
 }

@@ -2033,10 +2035,11 @@ static long do_rmdir(int dfd, const char
 	mutex_lock_nested(&nd.dentry->d_inode->i_mutex, I_MUTEX_PARENT);
 	dentry = lookup_hash(&nd);
 	error = PTR_ERR(dentry);
-	if (!IS_ERR(dentry)) {
-		error = vfs_rmdir(nd.dentry->d_inode, dentry);
-		dput(dentry);
-	}
+	if (IS_ERR(dentry))
+		goto exit2;
+	error = vfs_rmdir(nd.dentry->d_inode, dentry);
+	dput(dentry);
+exit2:
 	mutex_unlock(&nd.dentry->d_inode->i_mutex);
 exit1:
 	path_release(&nd);

@@ -2176,30 +2179,33 @@ asmlinkage long sys_symlinkat(const char
 	int error = 0;
 	char * from;
 	char * to;
+	struct dentry *dentry;
+	struct nameidata nd;
 
 	from = getname(oldname);
 	if(IS_ERR(from))
 		return PTR_ERR(from);
 	to = getname(newname);
 	error = PTR_ERR(to);
-	if (!IS_ERR(to)) {
-		struct dentry *dentry;
-		struct nameidata nd;
+	if (IS_ERR(to))
+		goto out_putname;
 
-		error = do_path_lookup(newdfd, to, LOOKUP_PARENT, &nd);
-		if (error)
-			goto out;
-		dentry = lookup_create(&nd, 0);
-		error = PTR_ERR(dentry);
-		if (!IS_ERR(dentry)) {
-			error = vfs_symlink(nd.dentry->d_inode, dentry, from, S_IALLUGO);
-			dput(dentry);
-		}
-		mutex_unlock(&nd.dentry->d_inode->i_mutex);
-		path_release(&nd);
+	error = do_path_lookup(newdfd, to, LOOKUP_PARENT, &nd);
+	if (error)
+		goto out;
+	dentry = lookup_create(&nd, 0);
+	error = PTR_ERR(dentry);
+	if (IS_ERR(dentry))
+		goto out_unlock;
+
+	error = vfs_symlink(nd.dentry->d_inode, dentry, from, S_IALLUGO);
+	dput(dentry);
+out_unlock:
+	mutex_unlock(&nd.dentry->d_inode->i_mutex);
+	path_release(&nd);
 out:
-		putname(to);
-	}
+	putname(to);
+out_putname:
 	putname(from);
 	return error;
 }

@@ -2285,10 +2291,11 @@ asmlinkage long sys_linkat(int olddfd, c
 		goto out_release;
 	new_dentry = lookup_create(&nd, 0);
 	error = PTR_ERR(new_dentry);
-	if (!IS_ERR(new_dentry)) {
-		error = vfs_link(old_nd.dentry, nd.dentry->d_inode, new_dentry);
-		dput(new_dentry);
-	}
+	if (IS_ERR(new_dentry))
+		goto out_unlock;
+	error = vfs_link(old_nd.dentry, nd.dentry->d_inode, new_dentry);
+	dput(new_dentry);
+out_unlock:
 	mutex_unlock(&nd.dentry->d_inode->i_mutex);
 out_release:
 	path_release(&nd);

fs/open.c renamed from fs/open.c~B-prepwork-collapse-ifs +37 ~~-27~~

97 lines not on this page · raw message

Dave Hansen

author

2006-07-12 18:17 UTC #2 lore raw

RFC PATCH 2/27 r/o bind mount prepwork: move open_namei()'s vfs_create()

In reply to Dave Hansen

The code around vfs_create() in open_namei() is getting a bit too complex. Right now, there is at least the reference count on the dentry, and the i_mutex to worry about. Soon, we'll also have mnt_writecount.

So, break the vfs_create() call out of open_namei(), and into a helper function. This duplicates the call to may_open(), but that isn't such a bad thing since the arguments (acc_mode and flag) were being heavily massaged anyway.

Later in the series, we'll add the mnt_writecount handling around this new function call.

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/namei.c |   30 ++++++++++++++++++++----------
 1 files changed, 20 insertions(+), 10 deletions(-)

fs/namei.c renamed from fs/namei.c~B-prepwork-cleanup-open_namei +20 ~~-10~~

diff -puN fs/namei.c~B-prepwork-cleanup-open_namei fs/namei.c
@@ -1580,6 +1580,24 @@ int may_open(struct nameidata *nd, int a
 	return 0;
 }
 
+static int open_namei_create(struct nameidata *nd, struct path *path,
+				int flag, int mode)
+{
+	int error;
+	struct dentry *dir = nd->dentry;
+
+	if (!IS_POSIXACL(dir->d_inode))
+		mode &= ~current->fs->umask;
+	error = vfs_create(dir->d_inode, path->dentry, mode, nd);
+	mutex_unlock(&dir->d_inode->i_mutex);
+	dput(nd->dentry);
+	nd->dentry = path->dentry;
+	if (error)
+		return error;
+	/* Don't check for write permission, don't truncate */
+	return may_open(nd, 0, flag & ~O_TRUNC);
+}
+
 /*
  *	open_namei()
  *

@@ -1661,18 +1679,10 @@ do_last:
 
 	/* Negative dentry, just create the file */
 	if (!path.dentry->d_inode) {
-		if (!IS_POSIXACL(dir->d_inode))
-			mode &= ~current->fs->umask;
-		error = vfs_create(dir->d_inode, path.dentry, mode, nd);
-		mutex_unlock(&dir->d_inode->i_mutex);
-		dput(nd->dentry);
-		nd->dentry = path.dentry;
+		error = open_namei_create(nd, &path, flag, mode);
 		if (error)
 			goto exit;
-		/* Don't check for write permission, don't truncate */
-		acc_mode = 0;
-		flag &= ~O_TRUNC;
-		goto ok;
+		return 0;
 	}
 
 	/*

Dave Hansen

author

2006-07-12 18:26 UTC #3 lore raw

RFC PATCH 3/27 unlink: monitor i_nlink

In reply to Dave Hansen

When a filesystem decrements i_nlink to zero, it means that a write must be performed in order to drop the inode from the filesystem.

We're shortly going to have keep filesystems from being remounted r/o between the time that this i_nlink decrement and that write occurs.

So, add a little helper function to do the decrements. We'll tie into it in a bit to note when i_nlink hits zero. Should we also be checking all of the places where i_nlink is explicitly set to 0 in the unlink paths?

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/drivers/usb/core/inode.c |    4 ++--
 lxc-dave/fs/autofs/root.c         |    2 +-
 lxc-dave/fs/autofs4/root.c        |    2 +-
 lxc-dave/fs/bfs/dir.c             |    6 +++---
 lxc-dave/fs/cifs/inode.c          |   10 +++++-----
 lxc-dave/fs/coda/dir.c            |    4 ++--
 lxc-dave/fs/ext2/namei.c          |    2 +-
 lxc-dave/fs/ext3/namei.c          |   14 +++++++-------
 lxc-dave/fs/hfs/dir.c             |    2 +-
 lxc-dave/fs/hfsplus/dir.c         |    2 +-
 lxc-dave/fs/hpfs/namei.c          |    6 +++---
 lxc-dave/fs/jffs2/dir.c           |    6 +++---
 lxc-dave/fs/jfs/namei.c           |   12 ++++++------
 lxc-dave/fs/libfs.c               |   15 ++++++++++-----
 lxc-dave/fs/nfs/dir.c             |    6 +++---
 lxc-dave/fs/ocfs2/namei.c         |    4 ++--
 lxc-dave/fs/qnx4/namei.c          |    6 ++----
 lxc-dave/fs/reiserfs/namei.c      |    2 +-
 lxc-dave/fs/sysv/namei.c          |    2 +-
 lxc-dave/fs/udf/namei.c           |    6 +++---
 lxc-dave/fs/ufs/namei.c           |    2 +-
 lxc-dave/fs/vfat/namei.c          |    6 +++---
 lxc-dave/include/linux/fs.h       |    1 +
 lxc-dave/ipc/mqueue.c             |    2 +-
 lxc-dave/mm/shmem.c               |   10 +++++-----
 25 files changed, 69 insertions(+), 65 deletions(-)

drivers/usb/core/inode.c renamed from drivers/usb/core/inode.c~B-unlink-monitor-i_nlink-0 +2 -2

diff -puN drivers/usb/core/inode.c~B-unlink-monitor-i_nlink-0 drivers/usb/core/inode.c
@@ -332,7 +332,7 @@ static int usbfs_unlink (struct inode *d
 {
 	struct inode *inode = dentry->d_inode;
 	mutex_lock(&inode->i_mutex);
-	dentry->d_inode->i_nlink--;
+	inode_drop_nlink(dentry->d_inode);
 	dput(dentry);
 	mutex_unlock(&inode->i_mutex);
 	d_delete(dentry);

@@ -350,7 +350,7 @@ static int usbfs_rmdir(struct inode *dir
 		dentry->d_inode->i_nlink -= 2;
 		dput(dentry);
 		inode->i_flags |= S_DEAD;
-		dir->i_nlink--;
+		inode_drop_nlink(dir);
 		error = 0;
 	}
 	mutex_unlock(&inode->i_mutex);

fs/autofs/root.c renamed from fs/autofs/root.c~B-unlink-monitor-i_nlink-0 +1 -1

diff -puN fs/autofs/root.c~B-unlink-monitor-i_nlink-0 fs/autofs/root.c
@@ -414,7 +414,7 @@ static int autofs_root_rmdir(struct inod
 
 	dentry->d_time = (unsigned long)(struct autofs_dir_ent *)NULL;
 	autofs_hash_delete(ent);
-	dir->i_nlink--;
+	inode_drop_nlink(dir);
 	d_drop(dentry);
 	unlock_kernel();

fs/autofs4/root.c renamed from fs/autofs4/root.c~B-unlink-monitor-i_nlink-0 +1 -1

diff -puN fs/autofs4/root.c~B-unlink-monitor-i_nlink-0 fs/autofs4/root.c
@@ -676,7 +676,7 @@ static int autofs4_dir_rmdir(struct inod
 	dentry->d_inode->i_nlink = 0;
 
 	if (dir->i_nlink)
-		dir->i_nlink--;
+		inode_drop_nlink(dir);
 
 	return 0;
 }

fs/bfs/dir.c renamed from fs/bfs/dir.c~B-unlink-monitor-i_nlink-0 +3 -3

diff -puN fs/bfs/dir.c~B-unlink-monitor-i_nlink-0 fs/bfs/dir.c
@@ -117,7 +117,7 @@ static int bfs_create(struct inode * dir
 
 	err = bfs_add_entry(dir, dentry->d_name.name, dentry->d_name.len, inode->i_ino);
 	if (err) {
-		inode->i_nlink--;
+		inode_drop_nlink(inode);
 		mark_inode_dirty(inode);
 		iput(inode);
 		unlock_kernel();

@@ -196,7 +196,7 @@ static int bfs_unlink(struct inode * dir
 	mark_buffer_dirty(bh);
 	dir->i_ctime = dir->i_mtime = CURRENT_TIME_SEC;
 	mark_inode_dirty(dir);
-	inode->i_nlink--;
+	inode_drop_nlink(inode);
 	inode->i_ctime = dir->i_ctime;
 	mark_inode_dirty(inode);
 	error = 0;

@@ -249,7 +249,7 @@ static int bfs_rename(struct inode * old
 	old_dir->i_ctime = old_dir->i_mtime = CURRENT_TIME_SEC;
 	mark_inode_dirty(old_dir);
 	if (new_inode) {
-		new_inode->i_nlink--;
+		inode_drop_nlink(new_inode);
 		new_inode->i_ctime = CURRENT_TIME_SEC;
 		mark_inode_dirty(new_inode);
 	}

fs/cifs/inode.c renamed from fs/cifs/inode.c~B-unlink-monitor-i_nlink-0 +5 -5

diff -puN fs/cifs/inode.c~B-unlink-monitor-i_nlink-0 fs/cifs/inode.c
@@ -591,7 +591,7 @@ int cifs_unlink(struct inode *inode, str
 
 	if (!rc) {
 		if (direntry->d_inode)
-			direntry->d_inode->i_nlink--;
+			inode_drop_nlink(direntry->d_inode);
 	} else if (rc == -ENOENT) {
 		d_drop(direntry);
 	} else if (rc == -ETXTBSY) {

@@ -610,7 +610,7 @@ int cifs_unlink(struct inode *inode, str
 						CIFS_MOUNT_MAP_SPECIAL_CHR);
 			CIFSSMBClose(xid, pTcon, netfid);
 			if (direntry->d_inode)
-				direntry->d_inode->i_nlink--;
+				inode_drop_nlink(direntry->d_inode);
 		}
 	} else if (rc == -EACCES) {
 		/* try only if r/o attribute set in local lookup data? */

@@ -664,7 +664,7 @@ int cifs_unlink(struct inode *inode, str
 						CIFS_MOUNT_MAP_SPECIAL_CHR);
 			if (!rc) {
 				if (direntry->d_inode)
-					direntry->d_inode->i_nlink--;
+					inode_drop_nlink(direntry->d_inode);
 			} else if (rc == -ETXTBSY) {
 				int oplock = FALSE;
 				__u16 netfid;

@@ -685,7 +685,7 @@ int cifs_unlink(struct inode *inode, str
 						    CIFS_MOUNT_MAP_SPECIAL_CHR);
 					CIFSSMBClose(xid, pTcon, netfid);
 					if (direntry->d_inode)
-			                        direntry->d_inode->i_nlink--;
+						inode_drop_nlink(direntry->d_inode);
 				}
 			/* BB if rc = -ETXTBUSY goto the rename logic BB */
 			}

@@ -817,7 +817,7 @@ int cifs_rmdir(struct inode *inode, stru
 			  cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
 
 	if (!rc) {
-		inode->i_nlink--;
+		inode_drop_nlink(inode);
 		i_size_write(direntry->d_inode,0);
 		direntry->d_inode->i_nlink = 0;
 	}

fs/coda/dir.c renamed from fs/coda/dir.c~B-unlink-monitor-i_nlink-0 +2 -2

diff -puN fs/coda/dir.c~B-unlink-monitor-i_nlink-0 fs/coda/dir.c
@@ -367,7 +367,7 @@ int coda_unlink(struct inode *dir, struc
         }
 
 	coda_dir_changed(dir, 0);
-	de->d_inode->i_nlink--;
+	inode_drop_nlink(de->d_inode);
 	unlock_kernel();
 
         return 0;

@@ -394,7 +394,7 @@ int coda_rmdir(struct inode *dir, struct
         }
 
 	coda_dir_changed(dir, -1);
-	de->d_inode->i_nlink--;
+	inode_drop_nlink(de->d_inode);
 	d_delete(de);
 	unlock_kernel();

fs/ext2/namei.c renamed from fs/ext2/namei.c~B-unlink-monitor-i_nlink-0 +1 -1

diff -puN fs/ext2/namei.c~B-unlink-monitor-i_nlink-0 fs/ext2/namei.c
@@ -326,7 +326,7 @@ static int ext2_rename (struct inode * o
 		ext2_set_link(new_dir, new_de, new_page, old_inode);
 		new_inode->i_ctime = CURRENT_TIME_SEC;
 		if (dir_de)
-			new_inode->i_nlink--;
+			inode_drop_nlink(new_inode);
 		inode_dec_link_count(new_inode);
 	} else {
 		if (dir_de) {

fs/ext3/namei.c renamed from fs/ext3/namei.c~B-unlink-monitor-i_nlink-0 +7 -7

diff -puN fs/ext3/namei.c~B-unlink-monitor-i_nlink-0 fs/ext3/namei.c
@@ -1609,7 +1609,7 @@ static inline void ext3_inc_count(handle
 
 static inline void ext3_dec_count(handle_t *handle, struct inode *inode)
 {
-	inode->i_nlink--;
+	inode_drop_nlink(inode);
 }
 
 static int ext3_add_nondir(handle_t *handle,

@@ -1731,7 +1731,7 @@ retry:
 	inode->i_size = EXT3_I(inode)->i_disksize = inode->i_sb->s_blocksize;
 	dir_block = ext3_bread (handle, inode, 0, 1, &err);
 	if (!dir_block) {
-		inode->i_nlink--; /* is this nlink == 0? */
+		inode_drop_nlink(inode); /* is this nlink == 0? */
 		ext3_mark_inode_dirty(handle, inode);
 		iput (inode);
 		goto out_stop;

@@ -2041,7 +2041,7 @@ static int ext3_rmdir (struct inode * di
 	ext3_orphan_add(handle, inode);
 	inode->i_ctime = dir->i_ctime = dir->i_mtime = CURRENT_TIME_SEC;
 	ext3_mark_inode_dirty(handle, inode);
-	dir->i_nlink--;
+	inode_drop_nlink(dir);
 	ext3_update_dx_flag(dir);
 	ext3_mark_inode_dirty(handle, dir);

@@ -2092,7 +2092,7 @@ static int ext3_unlink(struct inode * di
 	dir->i_ctime = dir->i_mtime = CURRENT_TIME_SEC;
 	ext3_update_dx_flag(dir);
 	ext3_mark_inode_dirty(handle, dir);
-	inode->i_nlink--;
+	inode_drop_nlink(inode);
 	if (!inode->i_nlink)
 		ext3_orphan_add(handle, inode);
 	inode->i_ctime = dir->i_ctime;

@@ -2314,7 +2314,7 @@ static int ext3_rename (struct inode * o
 	}
 
 	if (new_inode) {
-		new_inode->i_nlink--;
+		inode_drop_nlink(new_inode);
 		new_inode->i_ctime = CURRENT_TIME_SEC;
 	}
 	old_dir->i_ctime = old_dir->i_mtime = CURRENT_TIME_SEC;

@@ -2325,9 +2325,9 @@ static int ext3_rename (struct inode * o
 		PARENT_INO(dir_bh->b_data) = cpu_to_le32(new_dir->i_ino);
 		BUFFER_TRACE(dir_bh, "call ext3_journal_dirty_metadata");
 		ext3_journal_dirty_metadata(handle, dir_bh);
-		old_dir->i_nlink--;
+		inode_drop_nlink(old_dir);
 		if (new_inode) {
-			new_inode->i_nlink--;
+			inode_drop_nlink(new_inode);
 		} else {
 			new_dir->i_nlink++;
 			ext3_update_dx_flag(new_dir);

fs/hfs/dir.c renamed from fs/hfs/dir.c~B-unlink-monitor-i_nlink-0 +1 -1

diff -puN fs/hfs/dir.c~B-unlink-monitor-i_nlink-0 fs/hfs/dir.c
@@ -246,7 +246,7 @@ static int hfs_unlink(struct inode *dir,
 	if (res)
 		return res;
 
-	inode->i_nlink--;
+	inode_drop_nlink(inode);
 	hfs_delete_inode(inode);
 	inode->i_ctime = CURRENT_TIME_SEC;
 	mark_inode_dirty(inode);

fs/hfsplus/dir.c renamed from fs/hfsplus/dir.c~B-unlink-monitor-i_nlink-0 +1 -1

diff -puN fs/hfsplus/dir.c~B-unlink-monitor-i_nlink-0 fs/hfsplus/dir.c
@@ -338,7 +338,7 @@ static int hfsplus_unlink(struct inode *
 		return res;
 
 	if (inode->i_nlink > 0)
-		inode->i_nlink--;
+		inode_drop_nlink(inode);
 	hfsplus_delete_inode(inode);
 	if (inode->i_ino != cnid && !inode->i_nlink) {
 		if (!atomic_read(&HFSPLUS_I(inode).opencnt)) {

fs/hpfs/namei.c renamed from fs/hpfs/namei.c~B-unlink-monitor-i_nlink-0 +3 -3

diff -puN fs/hpfs/namei.c~B-unlink-monitor-i_nlink-0 fs/hpfs/namei.c
@@ -434,7 +434,7 @@ again:
 		unlock_kernel();
 		return -ENOSPC;
 	default:
-		inode->i_nlink--;
+		inode_drop_nlink(inode);
 		err = 0;
 	}
 	goto out;

@@ -494,7 +494,7 @@ static int hpfs_rmdir(struct inode *dir,
 		err = -ENOSPC;
 		break;
 	default:
-		dir->i_nlink--;
+		inode_drop_nlink(dir);
 		inode->i_nlink = 0;
 		err = 0;
 	}

@@ -636,7 +636,7 @@ static int hpfs_rename(struct inode *old
 	hpfs_i(i)->i_parent_dir = new_dir->i_ino;
 	if (S_ISDIR(i->i_mode)) {
 		new_dir->i_nlink++;
-		old_dir->i_nlink--;
+		inode_drop_nlink(old_dir);
 	}
 	if ((fnode = hpfs_map_fnode(i->i_sb, i->i_ino, &bh))) {
 		fnode->up = new_dir->i_ino;

fs/jffs2/dir.c renamed from fs/jffs2/dir.c~B-unlink-monitor-i_nlink-0 +3 -3

diff -puN fs/jffs2/dir.c~B-unlink-monitor-i_nlink-0 fs/jffs2/dir.c
@@ -615,7 +615,7 @@ static int jffs2_rmdir (struct inode *di
 	}
 	ret = jffs2_unlink(dir_i, dentry);
 	if (!ret)
-		dir_i->i_nlink--;
+		inode_drop_nlink(dir_i);
 	return ret;
 }

@@ -823,7 +823,7 @@ static int jffs2_rename (struct inode *o
 
 	if (victim_f) {
 		/* There was a victim. Kill it off nicely */
-		new_dentry->d_inode->i_nlink--;
+		inode_drop_nlink(new_dentry->d_inode);
 		/* Don't oops if the victim was a dirent pointing to an
 		   inode which didn't exist. */
 		if (victim_f->inocache) {

@@ -862,7 +862,7 @@ static int jffs2_rename (struct inode *o
 	}
 
 	if (S_ISDIR(old_dentry->d_inode->i_mode))
-		old_dir_i->i_nlink--;
+		inode_drop_nlink(old_dir_i);
 
 	new_dir_i->i_mtime = new_dir_i->i_ctime = old_dir_i->i_mtime = old_dir_i->i_ctime = ITIME(now);

fs/jfs/namei.c renamed from fs/jfs/namei.c~B-unlink-monitor-i_nlink-0 +6 -6

diff -puN fs/jfs/namei.c~B-unlink-monitor-i_nlink-0 fs/jfs/namei.c
@@ -393,7 +393,7 @@ static int jfs_rmdir(struct inode *dip, 
 	/* update parent directory's link count corresponding
 	 * to ".." entry of the target directory deleted
 	 */
-	dip->i_nlink--;
+	inode_drop_nlink(dip);
 	dip->i_ctime = dip->i_mtime = CURRENT_TIME;
 	mark_inode_dirty(dip);

@@ -515,7 +515,7 @@ static int jfs_unlink(struct inode *dip,
 	mark_inode_dirty(dip);
 
 	/* update target's inode */
-	ip->i_nlink--;
+	inode_drop_nlink(ip);
 	mark_inode_dirty(ip);
 
 	/*

@@ -835,7 +835,7 @@ static int jfs_link(struct dentry *old_d
 	rc = txCommit(tid, 2, &iplist[0], 0);
 
 	if (rc) {
-		ip->i_nlink--;
+		ip->i_nlink--; /* never instantiated */
 		iput(ip);
 	} else
 		d_instantiate(dentry, ip);

@@ -1155,9 +1155,9 @@ static int jfs_rename(struct inode *old_
 			      old_ip->i_ino, JFS_RENAME);
 		if (rc)
 			goto out4;
-		new_ip->i_nlink--;
+		inode_drop_nlink(new_ip);
 		if (S_ISDIR(new_ip->i_mode)) {
-			new_ip->i_nlink--;
+			inode_drop_nlink(new_ip);
 			if (new_ip->i_nlink) {
 				mutex_unlock(&JFS_IP(new_ip)->commit_mutex);
 				if (old_dir != new_dir)

@@ -1223,7 +1223,7 @@ static int jfs_rename(struct inode *old_
 		goto out4;
 	}
 	if (S_ISDIR(old_ip->i_mode)) {
-		old_dir->i_nlink--;
+		inode_drop_nlink(old_dir);
 		if (old_dir != new_dir) {
 			/*
 			 * Change inode number of parent for moved directory

fs/libfs.c renamed from fs/libfs.c~B-unlink-monitor-i_nlink-0 +10 -5

diff -puN fs/libfs.c~B-unlink-monitor-i_nlink-0 fs/libfs.c
@@ -270,12 +270,17 @@ out:
 	return ret;
 }
 
+void inode_drop_nlink(struct inode *inode)
+{
+	inode->i_nlink--;
+}
+
 int simple_unlink(struct inode *dir, struct dentry *dentry)
 {
 	struct inode *inode = dentry->d_inode;
 
 	inode->i_ctime = dir->i_ctime = dir->i_mtime = CURRENT_TIME;
-	inode->i_nlink--;
+	inode_drop_nlink(inode);
 	dput(dentry);
 	return 0;
 }

@@ -285,9 +290,9 @@ int simple_rmdir(struct inode *dir, stru
 	if (!simple_empty(dentry))
 		return -ENOTEMPTY;
 
-	dentry->d_inode->i_nlink--;
+	inode_drop_nlink(dentry->d_inode);
 	simple_unlink(dir, dentry);
-	dir->i_nlink--;
+	inode_drop_nlink(dir);
 	return 0;
 }

@@ -303,9 +308,9 @@ int simple_rename(struct inode *old_dir,
 	if (new_dentry->d_inode) {
 		simple_unlink(new_dir, new_dentry);
 		if (they_are_dirs)
-			old_dir->i_nlink--;
+			inode_drop_nlink(old_dir);
 	} else if (they_are_dirs) {
-		old_dir->i_nlink--;
+		inode_drop_nlink(old_dir);
 		new_dir->i_nlink++;
 	}

fs/nfs/dir.c renamed from fs/nfs/dir.c~B-unlink-monitor-i_nlink-0 +3 -3

diff -puN fs/nfs/dir.c~B-unlink-monitor-i_nlink-0 fs/nfs/dir.c
@@ -841,7 +841,7 @@ static void nfs_dentry_iput(struct dentr
 	nfs_inode_return_delegation(inode);
 	if (dentry->d_flags & DCACHE_NFSFS_RENAMED) {
 		lock_kernel();
-		inode->i_nlink--;
+		inode_drop_nlink(inode);
 		nfs_complete_unlink(dentry);
 		unlock_kernel();
 	}

@@ -1391,7 +1391,7 @@ static int nfs_safe_remove(struct dentry
 		error = NFS_PROTO(dir)->remove(dir, &dentry->d_name);
 		/* The VFS may want to delete this inode */
 		if (error == 0)
-			inode->i_nlink--;
+			inode_drop_nlink(inode);
 		nfs_mark_for_revalidate(inode);
 		nfs_end_data_update(inode);
 	} else

@@ -1595,7 +1595,7 @@ static int nfs_rename(struct inode *old_
 			goto out;
 		}
 	} else
-		new_inode->i_nlink--;
+		inode_drop_nlink(new_inode);
 
 go_ahead:
 	/*

fs/ocfs2/namei.c renamed from fs/ocfs2/namei.c~B-unlink-monitor-i_nlink-0 +2 -2

diff -puN fs/ocfs2/namei.c~B-unlink-monitor-i_nlink-0 fs/ocfs2/namei.c
@@ -711,7 +711,7 @@ static int ocfs2_link(struct dentry *old
 	err = ocfs2_journal_dirty(handle, fe_bh);
 	if (err < 0) {
 		le16_add_cpu(&fe->i_links_count, -1);
-		inode->i_nlink--;
+		inode_drop_nlink(inode);
 		mlog_errno(err);
 		goto bail;
 	}

@@ -721,7 +721,7 @@ static int ocfs2_link(struct dentry *old
 			      parent_fe_bh, de_bh);
 	if (err) {
 		le16_add_cpu(&fe->i_links_count, -1);
-		inode->i_nlink--;
+		inode_drop_nlink(inode);
 		mlog_errno(err);
 		goto bail;
 	}

fs/qnx4/namei.c renamed from fs/qnx4/namei.c~B-unlink-monitor-i_nlink-0 +2 -4

diff -puN fs/qnx4/namei.c~B-unlink-monitor-i_nlink-0 fs/qnx4/namei.c
@@ -189,8 +189,7 @@ int qnx4_rmdir(struct inode *dir, struct
 	inode->i_nlink = 0;
 	mark_inode_dirty(inode);
 	inode->i_ctime = dir->i_ctime = dir->i_mtime = CURRENT_TIME_SEC;
-	dir->i_nlink--;
-	mark_inode_dirty(dir);
+	inode_dec_link_count(dir);
 	retval = 0;
 
       end_rmdir:

@@ -234,9 +233,8 @@ int qnx4_unlink(struct inode *dir, struc
 	mark_buffer_dirty(bh);
 	dir->i_ctime = dir->i_mtime = CURRENT_TIME_SEC;
 	mark_inode_dirty(dir);
-	inode->i_nlink--;
 	inode->i_ctime = dir->i_ctime;
-	mark_inode_dirty(inode);
+	inode_dec_link_count(inode);
 	retval = 0;
 
 end_unlink:

fs/reiserfs/namei.c renamed from fs/reiserfs/namei.c~B-unlink-monitor-i_nlink-0 +1 -1

diff -puN fs/reiserfs/namei.c~B-unlink-monitor-i_nlink-0 fs/reiserfs/namei.c
@@ -994,7 +994,7 @@ static int reiserfs_unlink(struct inode 
 		inode->i_nlink = 1;
 	}
 
-	inode->i_nlink--;
+	inode_drop_nlink(inode);
 
 	/*
 	 * we schedule before doing the add_save_link call, save the link

fs/sysv/namei.c renamed from fs/sysv/namei.c~B-unlink-monitor-i_nlink-0 +1 -1

diff -puN fs/sysv/namei.c~B-unlink-monitor-i_nlink-0 fs/sysv/namei.c
@@ -250,7 +250,7 @@ static int sysv_rename(struct inode * ol
 		sysv_set_link(new_de, new_page, old_inode);
 		new_inode->i_ctime = CURRENT_TIME_SEC;
 		if (dir_de)
-			new_inode->i_nlink--;
+			inode_drop_nlink(new_inode);
 		inode_dec_link_count(new_inode);
 	} else {
 		if (dir_de) {

fs/udf/namei.c renamed from fs/udf/namei.c~B-unlink-monitor-i_nlink-0 +3 -3

diff -puN fs/udf/namei.c~B-unlink-monitor-i_nlink-0 fs/udf/namei.c
@@ -923,7 +923,7 @@ static int udf_unlink(struct inode * dir
 		goto end_unlink;
 	dir->i_ctime = dir->i_mtime = current_fs_time(dir->i_sb);
 	mark_inode_dirty(dir);
-	inode->i_nlink--;
+	inode_drop_nlink(inode);
 	mark_inode_dirty(inode);
 	inode->i_ctime = dir->i_ctime;
 	retval = 0;

@@ -1101,7 +1101,7 @@ out:
 	return err;
 
 out_no_entry:
-	inode->i_nlink--;
+	inode_drop_nlink(inode);
 	mark_inode_dirty(inode);
 	iput(inode);
 	goto out;

@@ -1261,7 +1261,7 @@ static int udf_rename (struct inode * ol
 
 	if (new_inode)
 	{
-		new_inode->i_nlink--;
+		inode_drop_nlink(new_inode);
 		new_inode->i_ctime = current_fs_time(new_inode->i_sb);
 		mark_inode_dirty(new_inode);
 	}

fs/ufs/namei.c renamed from fs/ufs/namei.c~B-unlink-monitor-i_nlink-0 +1 -1

diff -puN fs/ufs/namei.c~B-unlink-monitor-i_nlink-0 fs/ufs/namei.c
@@ -307,7 +307,7 @@ static int ufs_rename(struct inode *old_
 		ufs_set_link(new_dir, new_de, new_page, old_inode);
 		new_inode->i_ctime = CURRENT_TIME_SEC;
 		if (dir_de)
-			new_inode->i_nlink--;
+			inode_drop_nlink(new_inode);
 		inode_dec_link_count(new_inode);
 	} else {
 		if (dir_de) {

fs/vfat/namei.c renamed from fs/vfat/namei.c~B-unlink-monitor-i_nlink-0 +3 -3

diff -puN fs/vfat/namei.c~B-unlink-monitor-i_nlink-0 fs/vfat/namei.c
@@ -782,7 +782,7 @@ static int vfat_rmdir(struct inode *dir,
 	err = fat_remove_entries(dir, &sinfo);	/* and releases bh */
 	if (err)
 		goto out;
-	dir->i_nlink--;
+	inode_drop_nlink(dir);
 
 	inode->i_nlink = 0;
 	inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;

@@ -930,7 +930,7 @@ static int vfat_rename(struct inode *old
 			if (err)
 				goto error_dotdot;
 		}
-		old_dir->i_nlink--;
+		inode_drop_nlink(old_dir);
 		if (!new_inode)
  			new_dir->i_nlink++;
 	}

@@ -950,7 +950,7 @@ static int vfat_rename(struct inode *old
 		if (is_dir)
 			new_inode->i_nlink -= 2;
 		else
-			new_inode->i_nlink--;
+			inode_drop_nlink(new_inode);
 		new_inode->i_ctime = ts;
 	}
 out:

include/linux/fs.h renamed from include/linux/fs.h~B-unlink-monitor-i_nlink-0 +1 -0

diff -puN include/linux/fs.h~B-unlink-monitor-i_nlink-0 include/linux/fs.h
@@ -1876,6 +1876,7 @@ extern int simple_prepare_write(struct f
 			unsigned offset, unsigned to);
 extern int simple_commit_write(struct file *file, struct page *page,
 				unsigned offset, unsigned to);
+extern void inode_drop_nlink(struct inode *inode);
 
 extern struct dentry *simple_lookup(struct inode *, struct dentry *, struct nameidata *);
 extern ssize_t generic_read_dir(struct file *, char __user *, size_t, loff_t *);

ipc/mqueue.c renamed from ipc/mqueue.c~B-unlink-monitor-i_nlink-0 +1 -1

diff -puN ipc/mqueue.c~B-unlink-monitor-i_nlink-0 ipc/mqueue.c
@@ -307,7 +307,7 @@ static int mqueue_unlink(struct inode *d
 
 	dir->i_ctime = dir->i_mtime = dir->i_atime = CURRENT_TIME;
 	dir->i_size -= DIRENT_SIZE;
-  	inode->i_nlink--;
+  	inode_drop_nlink(inode);
   	dput(dentry);
   	return 0;
 }

mm/shmem.c renamed from mm/shmem.c~B-unlink-monitor-i_nlink-0 +5 -5

diff -puN mm/shmem.c~B-unlink-monitor-i_nlink-0 mm/shmem.c
@@ -1760,7 +1760,7 @@ static int shmem_unlink(struct inode *di
 
 	dir->i_size -= BOGO_DIRENT_SIZE;
 	inode->i_ctime = dir->i_ctime = dir->i_mtime = CURRENT_TIME;
-	inode->i_nlink--;
+	inode_drop_nlink(inode);
 	dput(dentry);	/* Undo the count from "create" - this does all the work */
 	return 0;
 }

@@ -1770,8 +1770,8 @@ static int shmem_rmdir(struct inode *dir
 	if (!simple_empty(dentry))
 		return -ENOTEMPTY;
 
-	dentry->d_inode->i_nlink--;
-	dir->i_nlink--;
+	inode_drop_nlink(dentry->d_inode);
+	inode_drop_nlink(dir);
 	return shmem_unlink(dir, dentry);
 }

@@ -1792,9 +1792,9 @@ static int shmem_rename(struct inode *ol
 	if (new_dentry->d_inode) {
 		(void) shmem_unlink(new_dir, new_dentry);
 		if (they_are_dirs)
-			old_dir->i_nlink--;
+			inode_drop_nlink(old_dir);
 	} else if (they_are_dirs) {
-		old_dir->i_nlink--;
+		inode_drop_nlink(old_dir);
 		new_dir->i_nlink++;
 	}

Dave Hansen

author

2006-07-12 18:26 UTC #4 lore raw

RFC PATCH 4/27 reintroduce list of vfsmounts over superblock

In reply to Dave Hansen

We're moving a big chunk of the burden of keeping people from writing to r/o filesystems from the superblock into the vfsmount. This requires that we consult the superblock's vfsmounts when things like remounts occur.

So, introduce a list of vfsmounts hanging off the superblock. We'll use this in a bit.

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/namespace.c        |   12 ++++++++++++
 lxc-dave/fs/super.c            |    1 +
 lxc-dave/include/linux/fs.h    |    1 +
 lxc-dave/include/linux/mount.h |    1 +
 4 files changed, 15 insertions(+)

fs/namespace.c renamed from fs/namespace.c~C-reintroduce-list-of-vfsmounts-over-superblock +12 -0

diff -puN fs/namespace.c~C-reintroduce-list-of-vfsmounts-over-superblock fs/namespace.c
@@ -85,10 +85,18 @@ struct vfsmount *alloc_vfsmnt(const char
 	return mnt;
 }
 
+void add_mount_to_sb_list(struct vfsmount *mnt, struct super_block *sb)
+{
+	spin_lock(&vfsmount_lock);
+	list_add(&mnt->mnt_sb_list, &sb->s_vfsmounts);
+	spin_unlock(&vfsmount_lock);
+}
+
 int simple_set_mnt(struct vfsmount *mnt, struct super_block *sb)
 {
 	mnt->mnt_sb = sb;
 	mnt->mnt_root = dget(sb->s_root);
+	add_mount_to_sb_list(mnt, sb);
 	return 0;
 }

@@ -96,6 +104,9 @@ EXPORT_SYMBOL(simple_set_mnt);
 
 void free_vfsmnt(struct vfsmount *mnt)
 {
+	spin_lock(&vfsmount_lock);
+	list_del(&mnt->mnt_sb_list);
+	spin_unlock(&vfsmount_lock);
 	kfree(mnt->mnt_devname);
 	kmem_cache_free(mnt_cache, mnt);
 }

@@ -249,6 +260,7 @@ static struct vfsmount *clone_mnt(struct
 		mnt->mnt_root = dget(root);
 		mnt->mnt_mountpoint = mnt->mnt_root;
 		mnt->mnt_parent = mnt;
+		add_mount_to_sb_list(mnt, sb);
 
 		if (flag & CL_SLAVE) {
 			list_add(&mnt->mnt_slave, &old->mnt_slave_list);

fs/super.c renamed from fs/super.c~C-reintroduce-list-of-vfsmounts-over-superblock +1 -0

diff -puN fs/super.c~C-reintroduce-list-of-vfsmounts-over-superblock fs/super.c
@@ -67,6 +67,7 @@ static struct super_block *alloc_super(s
 		INIT_LIST_HEAD(&s->s_dirty);
 		INIT_LIST_HEAD(&s->s_io);
 		INIT_LIST_HEAD(&s->s_files);
+		INIT_LIST_HEAD(&s->s_vfsmounts);
 		INIT_LIST_HEAD(&s->s_instances);
 		INIT_HLIST_HEAD(&s->s_anon);
 		INIT_LIST_HEAD(&s->s_inodes);

include/linux/fs.h renamed from include/linux/fs.h~C-reintroduce-list-of-vfsmounts-over-superblock +1 -0

diff -puN include/linux/fs.h~C-reintroduce-list-of-vfsmounts-over-superblock include/linux/fs.h
@@ -960,6 +960,7 @@ struct super_block {
 	struct list_head	s_dirty;	/* dirty inodes */
 	struct list_head	s_io;		/* parked for writeback */
 	struct hlist_head	s_anon;		/* anonymous dentries for (nfs) exporting */
+	struct list_head	s_vfsmounts;
 	struct list_head	s_files;
 
 	struct block_device	*s_bdev;

include/linux/mount.h renamed from include/linux/mount.h~C-reintroduce-list-of-vfsmounts-over-superblock +1 -0

diff -puN include/linux/mount.h~C-reintroduce-list-of-vfsmounts-over-superblock include/linux/mount.h
@@ -40,6 +40,7 @@ struct vfsmount {
 	struct dentry *mnt_mountpoint;	/* dentry of mountpoint */
 	struct dentry *mnt_root;	/* root of the mounted tree */
 	struct super_block *mnt_sb;	/* pointer to superblock */
+	struct list_head mnt_sb_list;	/* list of all mounts on same sb */
 	struct list_head mnt_mounts;	/* list of children, anchored here */
 	struct list_head mnt_child;	/* and going through their mnt_child */
 	atomic_t mnt_count;

Dave Hansen

author

2006-07-12 18:24 UTC #5 lore raw

RFC PATCH 5/27 Add vfsmount writer count

In reply to Dave Hansen

This allows a vfsmount to keep track of how many instances of files open for write there are at a given time.

A mount can refuse to allow writers. This can be because it is a read-only bind mount, or for other functionality in the future. A mount can also now refuse to make a transition from r/w to r/o whenever it has currently active writers.

When a mount gets its first writer, it tells the superblock about it by incrementing sb->s_mnt_writers. When the last writer goes away, this count is bumped back down. This makes the question of whether or not anything is writing to the sb a much quicker, easier operation than scanning the open files list.

The WARN_ON()s can go away before this is merged into mainline provided it has had some time in -mm or equivalent.

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/namespace.c        |   61 ++++++++++++++++++++++++++++++++++++++
 lxc-dave/fs/super.c            |   65 +++++++++++++++++++++++++++++++++++++----
 lxc-dave/include/linux/fs.h    |    2 +
 lxc-dave/include/linux/mount.h |   18 +++++++++++
 4 files changed, 141 insertions(+), 5 deletions(-)

fs/namespace.c renamed from fs/namespace.c~C-add-vfsmount-writer_count +61 -0

diff -puN fs/namespace.c~C-add-vfsmount-writer_count fs/namespace.c
@@ -65,6 +65,7 @@ struct vfsmount *alloc_vfsmnt(const char
 	if (mnt) {
 		memset(mnt, 0, sizeof(struct vfsmount));
 		atomic_set(&mnt->mnt_count, 1);
+		atomic_set(&mnt->mnt_writers, 0);
 		INIT_LIST_HEAD(&mnt->mnt_hash);
 		INIT_LIST_HEAD(&mnt->mnt_child);
 		INIT_LIST_HEAD(&mnt->mnt_mounts);

@@ -893,6 +894,60 @@ out_unlock:
 	return err;
 }
 
+int mnt_make_readonly(struct vfsmount *mnt)
+{
+	int ret = 0;
+
+	WARN_ON(__mnt_is_readonly(mnt));
+
+	/*
+	 * This flag set is actually redundant with what
+	 * happens in do_remount(), but since we do this
+	 * under the lock, anyone attempting to get a write
+	 * on it after this will fail.
+	 */
+	spin_lock(&mnt->mnt_sb->s_mnt_writers_lock);
+	if (!atomic_read(&mnt->mnt_writers))
+		mnt->mnt_flags |= MNT_READONLY;
+	else
+		ret = -EBUSY;
+	spin_unlock(&mnt->mnt_sb->s_mnt_writers_lock);
+	return ret;
+}
+
+int mnt_want_write(struct vfsmount *mnt)
+{
+	int ret = 0;
+repeat:
+	if (atomic_add_unless(&mnt->mnt_writers, 1, 0))
+		return 0;
+
+	spin_lock(&mnt->mnt_sb->s_mnt_writers_lock);
+	if (__mnt_is_readonly(mnt)) {
+		ret = -EROFS;
+		goto out;
+	}
+	if (atomic_add_return(1, &mnt->mnt_writers) != 1) {
+		atomic_dec(&mnt->mnt_writers);
+		spin_unlock(&mnt->mnt_sb->s_mnt_writers_lock);
+		goto repeat;
+	}
+	atomic_inc(&mnt->mnt_sb->s_mnt_writers);
+out:
+	spin_unlock(&mnt->mnt_sb->s_mnt_writers_lock);
+	return ret;
+}
+
+void mnt_drop_write(struct vfsmount *mnt)
+{
+	if (!atomic_dec_and_lock(&mnt->mnt_writers,
+				 &mnt->mnt_sb->s_mnt_writers_lock))
+		return;
+
+	atomic_dec(&mnt->mnt_sb->s_mnt_writers);
+	spin_unlock(&mnt->mnt_sb->s_mnt_writers_lock);
+}
+
 /*
  * recursively change the type of the mountpoint.
  */

@@ -984,6 +1039,8 @@ static int do_remount(struct nameidata *
 
 	down_write(&sb->s_umount);
 	err = do_remount_sb(sb, flags, data, 0);
+	if (!(sb->s_flags & MS_RDONLY))
+		mnt_flags |= MNT_SB_WRITABLE;
 	if (!err)
 		nd->mnt->mnt_flags = mnt_flags;
 	up_write(&sb->s_umount);

@@ -1124,6 +1181,8 @@ int do_add_mount(struct vfsmount *newmnt
 	if (S_ISLNK(newmnt->mnt_root->d_inode->i_mode))
 		goto unlock;
 
+	if (!(newmnt->mnt_sb->s_flags & MS_RDONLY))
+		mnt_flags |= MNT_SB_WRITABLE;
 	newmnt->mnt_flags = mnt_flags;
 	if ((err = graft_tree(newmnt, nd)))
 		goto unlock;

@@ -1415,6 +1474,8 @@ long do_mount(char *dev_name, char *dir_
 		((char *)data_page)[PAGE_SIZE - 1] = 0;
 
 	/* Separate the per-mountpoint flags */
+	if (flags & MS_RDONLY)
+		mnt_flags |= MNT_READONLY;
 	if (flags & MS_NOSUID)
 		mnt_flags |= MNT_NOSUID;
 	if (flags & MS_NODEV)

fs/super.c renamed from fs/super.c~C-add-vfsmount-writer_count +60 -5

104 lines not on this page · raw message

include/linux/fs.h renamed from include/linux/fs.h~C-add-vfsmount-writer_count +2 -0

diff -puN include/linux/fs.h~C-add-vfsmount-writer_count include/linux/fs.h
@@ -961,6 +961,8 @@ struct super_block {
 	struct list_head	s_io;		/* parked for writeback */
 	struct hlist_head	s_anon;		/* anonymous dentries for (nfs) exporting */
 	struct list_head	s_vfsmounts;
+	atomic_t		s_mnt_writers;	/* vfsmounts with active writers */
+	spinlock_t		s_mnt_writers_lock; /* taken when mounts change rw state */
 	struct list_head	s_files;
 
 	struct block_device	*s_bdev;

include/linux/mount.h renamed from include/linux/mount.h~C-add-vfsmount-writer_count +18 -0

diff -puN include/linux/mount.h~C-add-vfsmount-writer_count include/linux/mount.h
@@ -27,6 +27,8 @@ struct namespace;
 #define MNT_NOEXEC	0x04
 #define MNT_NOATIME	0x08
 #define MNT_NODIRATIME	0x10
+#define MNT_READONLY	0x20 /* does the user want this to be r/o? */
+#define MNT_SB_WRITABLE	0x40 /* does the SB currently allow writes? */
 
 #define MNT_SHRINKABLE	0x100

@@ -44,6 +46,7 @@ struct vfsmount {
 	struct list_head mnt_mounts;	/* list of children, anchored here */
 	struct list_head mnt_child;	/* and going through their mnt_child */
 	atomic_t mnt_count;
+	atomic_t mnt_writers;
 	int mnt_flags;
 	int mnt_expiry_mark;		/* true if marked for expiry */
 	char *mnt_devname;		/* Name of device e.g. /dev/dsk/hda1 */

@@ -64,6 +67,21 @@ static inline struct vfsmount *mntget(st
 	return mnt;
 }
 
+static inline int __mnt_is_readonly(struct vfsmount *mnt)
+{
+	return (mnt->mnt_flags & MNT_READONLY) ||
+	       !(mnt->mnt_flags & MNT_SB_WRITABLE);
+}
+
+static inline void __mnt_unmake_readonly(struct vfsmount *mnt)
+{
+	WARN_ON(!__mnt_is_readonly(mnt));
+	mnt->mnt_flags &= ~MNT_READONLY;
+}
+
+extern int mnt_make_readonly(struct vfsmount *mnt);
+extern int mnt_want_write(struct vfsmount *mnt);
+extern void mnt_drop_write(struct vfsmount *mnt);
 extern void mntput_no_expire(struct vfsmount *mnt);
 extern void mnt_pin(struct vfsmount *mnt);
 extern void mnt_unpin(struct vfsmount *mnt);

Dave Hansen

author

2006-07-12 18:24 UTC #6 lore raw

RFC PATCH 6/27 record when sb_writer_count elevated for inode

In reply to Dave Hansen

There are a number of filesystems that do iput()s without first having messed with i_nlink. In order to keep from accidentally decrementing the superblock writer count for these, we record when the count is bumped up, so that we can properly balance it.

I know the flag name sucks. Anybody have better ideas?

I first tried to do this by catching all of the users an intentions whenever i_nlink is modified, but all of the filesystems do enough creative things with it that even if it was all properly fixed now, new issues with vfsmnt writer count imaglance will probably pop up in the future. This patch trades that possibility for the chance that we will miss a i_nlink--, and not bump the sb writer count.

I like the idea screwing up writing out a single inode better than screwing up a global superblock count imbalance that will affect all inodes.

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/inode.c         |    7 ++++++-
 lxc-dave/fs/libfs.c         |    4 ++++
 lxc-dave/include/linux/fs.h |    1 +
 3 files changed, 11 insertions(+), 1 deletion(-)

fs/inode.c renamed from fs/inode.c~C-record-when-sb_writer_count-elevated-for-inode-in-inode +6 -1

diff -puN fs/inode.c~C-record-when-sb_writer_count-elevated-for-inode-in-inode fs/inode.c
@@ -1114,12 +1114,17 @@ EXPORT_SYMBOL_GPL(generic_drop_inode);
  */
 static inline void iput_final(struct inode *inode)
 {
-	struct super_operations *op = inode->i_sb->s_op;
+	struct super_block *sb = inode->i_sb;
+	struct super_operations *op = sb->s_op;
 	void (*drop)(struct inode *) = generic_drop_inode;
+	int must_drop_sb_write = (inode->i_state & I_WRITING_ON_SB);
 
+	inode->i_state &= ~I_WRITING_ON_SB;
 	if (op && op->drop_inode)
 		drop = op->drop_inode;
 	drop(inode);
+	if (must_drop_sb_write)
+		atomic_dec(&sb->s_mnt_writers);
 }
 
 /**

fs/libfs.c renamed from fs/libfs.c~C-record-when-sb_writer_count-elevated-for-inode-in-inode +4 -0

diff -puN fs/libfs.c~C-record-when-sb_writer_count-elevated-for-inode-in-inode fs/libfs.c
@@ -273,6 +273,9 @@ out:
 void inode_drop_nlink(struct inode *inode)
 {
 	inode->i_nlink--;
+	if (inode->i_nlink)
+		return;
+	inode->i_state |= I_WRITING_ON_SB;
 }
 
 int simple_unlink(struct inode *dir, struct dentry *dentry)

@@ -386,6 +389,7 @@ int simple_fill_super(struct super_block
 	inode = new_inode(s);
 	if (!inode)
 		return -ENOMEM;
+	inode->i_state |= I_WRITING_ON_SB;
 	inode->i_mode = S_IFDIR | 0755;
 	inode->i_uid = inode->i_gid = 0;
 	inode->i_blocks = 0;

include/linux/fs.h renamed from include/linux/fs.h~C-record-when-sb_writer_count-elevated-for-inode-in-inode +1 -0

diff -puN include/linux/fs.h~C-record-when-sb_writer_count-elevated-for-inode-in-inode include/linux/fs.h
@@ -1237,6 +1237,7 @@ struct super_operations {
 #define I_CLEAR			32
 #define I_NEW			64
 #define I_WILL_FREE		128
+#define I_WRITING_ON_SB		256
 
 #define I_DIRTY (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_PAGES)

Dave Hansen

author

2006-07-12 18:22 UTC #7 lore raw

RFC PATCH 7/27 kill open files traverse on remount ro

In reply to Dave Hansen

Now that we have the sb writer count, we don't need to go looking at all of the individual open files.

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/file_table.c    |   25 -------------------------
 lxc-dave/fs/super.c         |    1 -
 lxc-dave/include/linux/fs.h |    2 --
 3 files changed, 28 deletions(-)

fs/file_table.c renamed from fs/file_table.c~C-kill-open-file-traverse-on-remount-ro +0 ~~-25~~

diff -puN fs/file_table.c~C-kill-open-file-traverse-on-remount-ro fs/file_table.c
@@ -269,31 +269,6 @@ void file_kill(struct file *file)
 	}
 }
 
-int fs_may_remount_ro(struct super_block *sb)
-{
-	struct list_head *p;
-
-	/* Check that no files are currently opened for writing. */
-	file_list_lock();
-	list_for_each(p, &sb->s_files) {
-		struct file *file = list_entry(p, struct file, f_u.fu_list);
-		struct inode *inode = file->f_dentry->d_inode;
-
-		/* File with pending delete? */
-		if (inode->i_nlink == 0)
-			goto too_bad;
-
-		/* Writeable file? */
-		if (S_ISREG(inode->i_mode) && (file->f_mode & FMODE_WRITE))
-			goto too_bad;
-	}
-	file_list_unlock();
-	return 1; /* Tis' cool bro. */
-too_bad:
-	file_list_unlock();
-	return 0;
-}
-
 void __init files_init(unsigned long mempages)
 { 
 	int n;

fs/super.c renamed from fs/super.c~C-kill-open-file-traverse-on-remount-ro +0 -1

diff -puN fs/super.c~C-kill-open-file-traverse-on-remount-ro fs/super.c
@@ -555,7 +555,6 @@ static void sb_mounts_clear_flag(struct 
 
 static int sb_remount_ro(struct super_block *sb)
 {
-	return fs_may_remount_ro(sb);
 	spin_lock(&sb->s_mnt_writers_lock);
 	if (atomic_read(&sb->s_mnt_writers) > 0) {
 		spin_unlock(&sb->s_mnt_writers_lock);

include/linux/fs.h renamed from include/linux/fs.h~C-kill-open-file-traverse-on-remount-ro +0 -2

diff -puN include/linux/fs.h~C-kill-open-file-traverse-on-remount-ro include/linux/fs.h
@@ -1584,8 +1584,6 @@ extern const struct file_operations read
 extern const struct file_operations write_fifo_fops;
 extern const struct file_operations rdwr_fifo_fops;
 
-extern int fs_may_remount_ro(struct super_block *);
-
 /*
  * return READ, READA, or WRITE
  */

Dave Hansen

author

2006-07-12 18:24 UTC #8 lore raw

RFC PATCH 8/27 increment sb writer count when nlink hits zero

In reply to Dave Hansen

When a file is unlinked, there will soon be a write to the filesystem. Note this, and disallow remounts to r/o during the time when this write is pending.

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/libfs.c |    1 +
 1 files changed, 1 insertion(+)

fs/libfs.c renamed from fs/libfs.c~C-inc-sb-writer-count-on-dec-nlink-to-zero +1 -0

diff -puN fs/libfs.c~C-inc-sb-writer-count-on-dec-nlink-to-zero fs/libfs.c
@@ -276,6 +276,7 @@ void inode_drop_nlink(struct inode *inod
 	if (inode->i_nlink)
 		return;
 	inode->i_state |= I_WRITING_ON_SB;
+	atomic_inc(&inode->i_sb->s_mnt_writers);
 }
 
 int simple_unlink(struct inode *dir, struct dentry *dentry)

Dave Hansen

author

2006-07-12 18:25 UTC #9 lore raw

RFC PATCH 9/27 elevate writer count for chown and friends

In reply to Dave Hansen

chown/chmod,etc... don't call permission in the same way that the normal "open for write" calls do. They still write to the filesystem, so bump the write count during these operations.

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/open.c |   38 +++++++++++++++++++++++++++++++++-----
 1 files changed, 33 insertions(+), 5 deletions(-)

fs/open.c renamed from fs/open.c~C-elevate-writers-chown-and-friends +33 -5

diff -puN fs/open.c~C-elevate-writers-chown-and-friends fs/open.c
@@ -644,9 +644,12 @@ asmlinkage long sys_fchmod(unsigned int 
 	err = -EROFS;
 	if (IS_RDONLY(inode))
 		goto out_putf;
+	err = mnt_want_write(file->f_vfsmnt);
+	if (err)
+		goto out_putf;
 	err = -EPERM;
 	if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
-		goto out_putf;
+		goto out_drop_write;
 	mutex_lock(&inode->i_mutex);
 	if (mode == (mode_t) -1)
 		mode = inode->i_mode;

@@ -655,6 +658,8 @@ asmlinkage long sys_fchmod(unsigned int 
 	err = notify_change(dentry, &newattrs);
 	mutex_unlock(&inode->i_mutex);
 
+out_drop_write:
+	mnt_drop_write(file->f_vfsmnt);
 out_putf:
 	fput(file);
 out:

@@ -674,13 +679,16 @@ asmlinkage long sys_fchmodat(int dfd, co
 		goto out;
 	inode = nd.dentry->d_inode;
 
+	error = mnt_want_write(nd.mnt);
+	if (error)
+		goto dput_and_out;
 	error = -EROFS;
 	if (IS_RDONLY(inode))
-		goto dput_and_out;
+		goto out_drop_write;
 
 	error = -EPERM;
 	if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
-		goto dput_and_out;
+		goto out_drop_write;
 
 	mutex_lock(&inode->i_mutex);
 	if (mode == (mode_t) -1)

@@ -690,6 +698,8 @@ asmlinkage long sys_fchmodat(int dfd, co
 	error = notify_change(nd.dentry, &newattrs);
 	mutex_unlock(&inode->i_mutex);
 
+out_drop_write:
+	mnt_drop_write(nd.mnt);
 dput_and_out:
 	path_release(&nd);
 out:

@@ -715,7 +725,7 @@ static int chown_common(struct dentry * 
 	error = -EROFS;
 	if (IS_RDONLY(inode))
 		goto out;
-	error = -EPERM;
+ 	error = -EPERM;
 	if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
 		goto out;
 	newattrs.ia_valid =  ATTR_CTIME;

@@ -744,7 +754,12 @@ asmlinkage long sys_chown(const char __u
 	error = user_path_walk(filename, &nd);
 	if (error)
 		goto out;
+	error = mnt_want_write(nd.mnt);
+	if (error)
+		goto out_release;
 	error = chown_common(nd.dentry, user, group);
+	mnt_drop_write(nd.mnt);
+out_release:
 	path_release(&nd);
 out:
 	return error;

@@ -764,7 +779,12 @@ asmlinkage long sys_fchownat(int dfd, co
 	error = __user_walk_fd(dfd, filename, follow, &nd);
 	if (error)
 		goto out;
+	error = mnt_want_write(nd.mnt);
+	if (error)
+		goto out_release;
 	error = chown_common(nd.dentry, user, group);
+	mnt_drop_write(nd.mnt);
+out_release:
 	path_release(&nd);
 out:
 	return error;

@@ -778,7 +798,11 @@ asmlinkage long sys_lchown(const char __
 	error = user_path_walk_link(filename, &nd);
 	if (error)
 		goto out;
+	error = mnt_want_write(nd.mnt);
+	if (error)
+		goto out_release;
 	error = chown_common(nd.dentry, user, group);
+out_release:
 	path_release(&nd);
 out:
 	return error;

@@ -794,10 +818,14 @@ asmlinkage long sys_fchown(unsigned int 
 	file = fget(fd);
 	if (!file)
 		goto out;
-
+	error = mnt_want_write(file->f_vfsmnt);
+	if (error)
+		goto out_fput;
 	dentry = file->f_dentry;
 	audit_inode(NULL, dentry->d_inode);
 	error = chown_common(dentry, user, group);
+	mnt_drop_write(file->f_vfsmnt);
+out_fput:
 	fput(file);
 out:
 	return error;

Dave Hansen

author

2006-07-12 18:21 UTC #10 lore raw

RFC PATCH 10/27 elevate mnt writers for callers of vfs_mkdir()

In reply to Dave Hansen

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/namei.c            |    5 +++++
 lxc-dave/fs/nfsd/nfs4recover.c |    4 ++++
 2 files changed, 9 insertions(+)

fs/namei.c renamed from fs/namei.c~C-elevate-writers-vfs_mkdir +5 -0

diff -puN fs/namei.c~C-elevate-writers-vfs_mkdir fs/namei.c
@@ -1939,7 +1939,12 @@ asmlinkage long sys_mkdirat(int dfd, con
 
 	if (!IS_POSIXACL(nd.dentry->d_inode))
 		mode &= ~current->fs->umask;
+	error = mnt_want_write(nd.mnt);
+	if (error)
+		goto out_dput;
 	error = vfs_mkdir(nd.dentry->d_inode, dentry, mode);
+	mnt_drop_write(nd.mnt);
+out_dput:
 	dput(dentry);
 out_unlock:
 	mutex_unlock(&nd.dentry->d_inode->i_mutex);

fs/nfsd/nfs4recover.c renamed from fs/nfsd/nfs4recover.c~C-elevate-writers-vfs_mkdir +4 -0

diff -puN fs/nfsd/nfs4recover.c~C-elevate-writers-vfs_mkdir fs/nfsd/nfs4recover.c
@@ -155,7 +155,11 @@ nfsd4_create_clid_dir(struct nfs4_client
 		dprintk("NFSD: nfsd4_create_clid_dir: DIRECTORY EXISTS\n");
 		goto out_put;
 	}
+	status = mnt_want_write(rec_dir.mnt);
+	if (status)
+		goto out_put;
 	status = vfs_mkdir(rec_dir.dentry->d_inode, dentry, S_IRWXU);
+	mnt_drop_write(rec_dir.mnt);
 out_put:
 	dput(dentry);
 out_unlock:

Dave Hansen

author

2006-07-12 18:21 UTC #11 lore raw

RFC PATCH 11/27 elevate write count during entire ncp_ioctl()

In reply to Dave Hansen

Some ioctls need write access, but others don't. Make a helper function to decide when write access is needed, and take it.

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/ncpfs/ioctl.c |   55 +++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 54 insertions(+), 1 deletion(-)

fs/ncpfs/ioctl.c renamed from fs/ncpfs/ioctl.c~C-elevate-writers-file_permission-callers +54 -1

diff -puN fs/ncpfs/ioctl.c~C-elevate-writers-file_permission-callers fs/ncpfs/ioctl.c
@@ -15,6 +15,7 @@
 #include <linux/ioctl.h>
 #include <linux/time.h>
 #include <linux/mm.h>
+#include <linux/mount.h>
 #include <linux/highuid.h>
 #include <linux/vmalloc.h>

@@ -182,7 +183,7 @@ ncp_get_charsets(struct ncp_server* serv
 }
 #endif /* CONFIG_NCPFS_NLS */
 
-int ncp_ioctl(struct inode *inode, struct file *filp,
+static int __ncp_ioctl(struct inode *inode, struct file *filp,
 	      unsigned int cmd, unsigned long arg)
 {
 	struct ncp_server *server = NCP_SERVER(inode);

@@ -653,3 +654,55 @@ outrel:			
 /* #endif */
 	return -EINVAL;
 }
+
+static int ncp_ioctl_need_write(unsigned int cmd)
+{
+	switch (cmd) {
+	case NCP_IOC_GET_FS_INFO:
+	case NCP_IOC_GET_FS_INFO_V2:
+	case NCP_IOC_NCPREQUEST:
+	case NCP_IOC_SETDENTRYTTL:
+	case NCP_IOC_SIGN_INIT:
+	case NCP_IOC_LOCKUNLOCK:
+	case NCP_IOC_SET_SIGN_WANTED:
+		return 1;
+	case NCP_IOC_GETOBJECTNAME:
+	case NCP_IOC_SETOBJECTNAME:
+	case NCP_IOC_GETPRIVATEDATA:
+	case NCP_IOC_SETPRIVATEDATA:
+	case NCP_IOC_SETCHARSETS:
+	case NCP_IOC_GETCHARSETS:
+	case NCP_IOC_CONN_LOGGED_IN:
+	case NCP_IOC_GETDENTRYTTL:
+	case NCP_IOC_GETMOUNTUID2:
+	case NCP_IOC_SIGN_WANTED:
+	case NCP_IOC_GETROOT:
+	case NCP_IOC_SETROOT:
+		return 0;
+	default:
+		/* unkown IOCTL command, assume write */
+		WARN_ON(1);
+	}
+	return 1;
+}
+
+int ncp_ioctl(struct inode *inode, struct file *filp,
+	      unsigned int cmd, unsigned long arg)
+{
+	int ret;
+
+	if (ncp_ioctl_need_write(cmd)) {
+		/*
+		 * inside the ioctl(), any failures which
+		 * are because of file_permission() are
+		 * -EACCESS, so it seems consistent to keep
+		 *  that here.
+		 */
+		if (mnt_want_write(filp->f_vfsmnt))
+			return -EACCES;
+	}
+	ret = __ncp_ioctl(inode, filp, cmd, arg);
+	if (ncp_ioctl_need_write(cmd))
+		mnt_drop_write(filp->f_vfsmnt);
+	return ret;
+}

Dave Hansen

author

2006-07-12 18:22 UTC #12 lore raw

RFC PATCH 12/27 sys_symlinkat() elevate write count around vfs_symlink()

In reply to Dave Hansen

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/namei.c |    5 +++++
 1 files changed, 5 insertions(+)

fs/namei.c renamed from fs/namei.c~C-elevate-writers-vfs_symlink-part3 +5 -0

diff -puN fs/namei.c~C-elevate-writers-vfs_symlink-part3 fs/namei.c
@@ -2213,7 +2213,12 @@ asmlinkage long sys_symlinkat(const char
 	if (IS_ERR(dentry))
 		goto out_unlock;
 
+	error = mnt_want_write(nd.mnt);
+	if (error)
+		goto out_dput;
 	error = vfs_symlink(nd.dentry->d_inode, dentry, from, S_IALLUGO);
+	mnt_drop_write(nd.mnt);
+out_dput:
 	dput(dentry);
 out_unlock:
 	mutex_unlock(&nd.dentry->d_inode->i_mutex);

Dave Hansen

author

2006-07-12 18:22 UTC #13 lore raw

RFC PATCH 13/27 elevate mount count for extended attributes

In reply to Dave Hansen

This basically audits the callers of xattr_permission(), which calls permission() and can perform writes to the filesystem.

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/nfsd/nfs4proc.c |    7 ++++++-
 lxc-dave/fs/xattr.c         |   14 ++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

fs/nfsd/nfs4proc.c renamed from fs/nfsd/nfs4proc.c~C-xattr +6 -1

diff -puN fs/nfsd/nfs4proc.c~C-xattr fs/nfsd/nfs4proc.c
@@ -604,13 +604,18 @@ nfsd4_setattr(struct svc_rqst *rqstp, st
 			return status;
 		}
 	}
+	status = mnt_want_write(current_fh->fh_export->ex_mnt);
+	if (status)
+		return status;
 	status = nfs_ok;
 	if (setattr->sa_acl != NULL)
 		status = nfsd4_set_nfs4_acl(rqstp, current_fh, setattr->sa_acl);
 	if (status)
-		return status;
+		goto out;
 	status = nfsd_setattr(rqstp, current_fh, &setattr->sa_iattr,
 				0, (time_t)0);
+out:
+	mnt_drop_write(current_fh->fh_export->ex_mnt);
 	return status;
 }

fs/xattr.c renamed from fs/xattr.c~C-xattr +14 -0

diff -puN fs/xattr.c~C-xattr fs/xattr.c
@@ -12,6 +12,7 @@
 #include <linux/smp_lock.h>
 #include <linux/file.h>
 #include <linux/xattr.h>
+#include <linux/mount.h>
 #include <linux/namei.h>
 #include <linux/security.h>
 #include <linux/syscalls.h>

@@ -210,7 +211,11 @@ sys_setxattr(char __user *path, char __u
 	error = user_path_walk(path, &nd);
 	if (error)
 		return error;
+	error = mnt_want_write(nd.mnt);
+	if (error)
+		return error;
 	error = setxattr(nd.dentry, name, value, size, flags);
+	mnt_drop_write(nd.mnt);
 	path_release(&nd);
 	return error;
 }

@@ -225,7 +230,11 @@ sys_lsetxattr(char __user *path, char __
 	error = user_path_walk_link(path, &nd);
 	if (error)
 		return error;
+	error = mnt_want_write(nd.mnt);
+	if (error)
+		return error;
 	error = setxattr(nd.dentry, name, value, size, flags);
+	mnt_drop_write(nd.mnt);
 	path_release(&nd);
 	return error;
 }

@@ -241,9 +250,14 @@ sys_fsetxattr(int fd, char __user *name,
 	f = fget(fd);
 	if (!f)
 		return error;
+	error = mnt_want_write(f->f_vfsmnt);
+	if (error)
+		goto out_fput;
 	dentry = f->f_dentry;
 	audit_inode(NULL, dentry->d_inode);
 	error = setxattr(dentry, name, value, size, flags);
+	mnt_drop_write(f->f_vfsmnt);
+out_fput:
 	fput(f);
 	return error;
 }

Dave Hansen

author

2006-07-12 18:22 UTC #14 lore raw

RFC PATCH 14/27 sys_linkat(): elevate write count around vfs_link()

In reply to Dave Hansen

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/namei.c |    5 +++++
 1 files changed, 5 insertions(+)

fs/namei.c renamed from fs/namei.c~C-elevate-writers-vfs_link-part1 +5 -0

diff -puN fs/namei.c~C-elevate-writers-vfs_link-part1 fs/namei.c
@@ -2313,7 +2313,12 @@ asmlinkage long sys_linkat(int olddfd, c
 	error = PTR_ERR(new_dentry);
 	if (IS_ERR(new_dentry))
 		goto out_unlock;
+	error = mnt_want_write(nd.mnt);
+	if (error)
+		goto out_dput;
 	error = vfs_link(old_nd.dentry, nd.dentry->d_inode, new_dentry);
+	mnt_drop_write(nd.mnt);
+out_dput:
 	dput(new_dentry);
 out_unlock:
 	mutex_unlock(&nd.dentry->d_inode->i_mutex);

Dave Hansen

author

2006-07-12 18:22 UTC #15 lore raw

RFC PATCH 15/27 mount_is_safe(): add comment

In reply to Dave Hansen

This area of code is currently #ifdef'd out, so add a comment for the time when it is actually used.

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/namespace.c |    4 ++++
 1 files changed, 4 insertions(+)

fs/namespace.c renamed from fs/namespace.c~C-elevate-writers-opens-part5 +4 -0

diff -puN fs/namespace.c~C-elevate-writers-opens-part5 fs/namespace.c
@@ -699,6 +699,10 @@ static int mount_is_safe(struct nameidat
 		if (current->uid != nd->dentry->d_inode->i_uid)
 			return -EPERM;
 	}
+	/*
+	 * We will eventually check for the mnt->writer_count here,
+	 * but since the code is not used now, skip it - Dave Hansen
+	 */
 	if (vfs_permission(nd, MAY_WRITE))
 		return -EPERM;
 	return 0;

Dave Hansen

author

2006-07-12 18:21 UTC #16 lore raw

RFC PATCH 16/27 unix_find_other() elevate write count for touch_atime()

In reply to Dave Hansen

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/net/unix/af_unix.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

net/unix/af_unix.c renamed from net/unix/af_unix.c~C-elevate-writers-opens-part4 +12 -4

diff -puN net/unix/af_unix.c~C-elevate-writers-opens-part4 net/unix/af_unix.c
@@ -709,21 +709,27 @@ static struct sock *unix_find_other(stru
 		err = path_lookup(sunname->sun_path, LOOKUP_FOLLOW, &nd);
 		if (err)
 			goto fail;
+
+		err = mnt_want_write(nd.mnt);
+		if (err)
+			goto put_path_fail;
+
 		err = vfs_permission(&nd, MAY_WRITE);
 		if (err)
-			goto put_fail;
+			goto mnt_drop_write_fail;
 
 		err = -ECONNREFUSED;
 		if (!S_ISSOCK(nd.dentry->d_inode->i_mode))
-			goto put_fail;
+			goto mnt_drop_write_fail;
 		u=unix_find_socket_byinode(nd.dentry->d_inode);
 		if (!u)
-			goto put_fail;
+			goto mnt_drop_write_fail;
 
 		if (u->sk_type == type)
 			touch_atime(nd.mnt, nd.dentry);
 
 		path_release(&nd);
+		mnt_drop_write(nd.mnt);
 
 		err=-EPROTOTYPE;
 		if (u->sk_type != type) {

@@ -743,7 +749,9 @@ static struct sock *unix_find_other(stru
 	}
 	return u;
 
-put_fail:
+mnt_drop_write_fail:
+	mnt_drop_write(nd.mnt);
+put_path_fail:
 	path_release(&nd);
 fail:
 	*error=err;

Dave Hansen

author

2006-07-12 18:18 UTC #17 lore raw

RFC PATCH 17/27 elevate write count over calls to vfs_rename()

In reply to Dave Hansen

This does create a little helper in the NFS code to make an if() a little bit less ugly.

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/namei.c    |    4 ++++
 lxc-dave/fs/nfsd/vfs.c |   25 ++++++++++++++++++++-----
 2 files changed, 24 insertions(+), 5 deletions(-)

fs/namei.c renamed from fs/namei.c~C-elevate-writers-vfs_rename-part1 +4 -0

diff -puN fs/namei.c~C-elevate-writers-vfs_rename-part1 fs/namei.c
@@ -2544,8 +2544,12 @@ static int do_rename(int olddfd, const c
 	if (new_dentry == trap)
 		goto exit5;
 
+	error = mnt_want_write(oldnd.mnt);
+	if (error)
+		goto exit5;
 	error = vfs_rename(old_dir->d_inode, old_dentry,
 				   new_dir->d_inode, new_dentry);
+	mnt_drop_write(oldnd.mnt);
 exit5:
 	dput(new_dentry);
 exit4:

fs/nfsd/vfs.c renamed from fs/nfsd/vfs.c~C-elevate-writers-vfs_rename-part1 +20 -5

diff -puN fs/nfsd/vfs.c~C-elevate-writers-vfs_rename-part1 fs/nfsd/vfs.c
@@ -1533,6 +1533,14 @@ out_nfserr:
 	goto out_unlock;
 }
 
+static inline int svc_msnfs(struct svc_fh *ffhp)
+{
+#ifdef MSNFS
+	return (ffhp->fh_export->ex_flags & NFSEXP_MSNFS);
+#else
+	return 0;
+#endif
+}
 /*
  * Rename a file
  * N.B. After this call _both_ ffhp and tfhp need an fh_put

@@ -1593,20 +1601,27 @@ nfsd_rename(struct svc_rqst *rqstp, stru
 	if (ndentry == trap)
 		goto out_dput_new;
 
-#ifdef MSNFS
-	if ((ffhp->fh_export->ex_flags & NFSEXP_MSNFS) &&
+	if (svc_msnfs(ffhp) &&
 		((atomic_read(&odentry->d_count) > 1)
 		 || (atomic_read(&ndentry->d_count) > 1))) {
 			err = -EPERM;
-	} else
-#endif
+			goto out_dput_new;
+	}
+
+	err = -EXDEV;
+	if (ffhp->fh_export->ex_mnt != tfhp->fh_export->ex_mnt)
+		goto out_dput_new;
+	err = mnt_want_write(ffhp->fh_export->ex_mnt);
+	if (err)
+		goto out_dput_new;
+
 	err = vfs_rename(fdir, odentry, tdir, ndentry);
 	if (!err && EX_ISSYNC(tfhp->fh_export)) {
 		err = nfsd_sync_dir(tdentry);
 		if (!err)
 			err = nfsd_sync_dir(fdentry);
 	}
-
+	mnt_drop_write(ffhp->fh_export->ex_mnt);
  out_dput_new:
 	dput(ndentry);
  out_dput_old:

Dave Hansen

author

2006-07-12 18:18 UTC #18 lore raw

RFC PATCH 18/27 tricky: elevate write count files are open()ed

In reply to Dave Hansen

This is the first really tricky patch in the series. It elevates the writer count on a mount each time a non-special file is opened for write.

This is not completely apparent in the patch because the two if() conditions in may_open() above the mnt_want_write() call are, combined, equivalent to special_file().

There is also an elevated count around the vfs_create() call in open_namei(). The count needs to be kept elevated all the way into the may_open() call. Otherwise, when the write is dropped, a ro->rw transisition could occur. This would lead to having rw access on the newly created file, while the vfsmount is ro. That is bad.

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/file_table.c |    5 ++++-
 lxc-dave/fs/namei.c      |   22 ++++++++++++++++++----
 lxc-dave/ipc/mqueue.c    |    3 +++
 3 files changed, 25 insertions(+), 5 deletions(-)

fs/file_table.c renamed from fs/file_table.c~C-elevate-writers-opens-part1 +4 -1

diff -puN fs/file_table.c~C-elevate-writers-opens-part1 fs/file_table.c
@@ -179,8 +179,11 @@ void fastcall __fput(struct file *file)
 	if (unlikely(S_ISCHR(inode->i_mode) && inode->i_cdev != NULL))
 		cdev_put(inode->i_cdev);
 	fops_put(file->f_op);
-	if (file->f_mode & FMODE_WRITE)
+	if (file->f_mode & FMODE_WRITE) {
 		put_write_access(inode);
+		if(!special_file(inode->i_mode))
+			mnt_drop_write(mnt);
+	}
 	file_kill(file);
 	file->f_dentry = NULL;
 	file->f_vfsmnt = NULL;

fs/namei.c renamed from fs/namei.c~C-elevate-writers-opens-part1 +18 -4

diff -puN fs/namei.c~C-elevate-writers-opens-part1 fs/namei.c
@@ -1532,8 +1532,17 @@ int may_open(struct nameidata *nd, int a
 			return -EACCES;
 
 		flag &= ~O_TRUNC;
-	} else if (IS_RDONLY(inode) && (flag & FMODE_WRITE))
-		return -EROFS;
+	} else if (flag & FMODE_WRITE) {
+		/*
+		 * effectively: !special_file()
+		 * balanced by __fput()
+		 */
+		error = mnt_want_write(nd->mnt);
+		if (error)
+			return error;
+		if (IS_RDONLY(inode))
+			return -EROFS;
+	}
 	/*
 	 * An append-only file must be opened in append mode for writing.
 	 */

@@ -1672,14 +1681,17 @@ do_last:
 	}
 
 	if (IS_ERR(nd->intent.open.file)) {
-		mutex_unlock(&dir->d_inode->i_mutex);
 		error = PTR_ERR(nd->intent.open.file);
-		goto exit_dput;
+		goto exit_mutex_unlock;
 	}
 
 	/* Negative dentry, just create the file */
 	if (!path.dentry->d_inode) {
+		error = mnt_want_write(nd->mnt);
+		if (error)
+			goto exit_mutex_unlock;
 		error = open_namei_create(nd, &path, flag, mode);
+		mnt_drop_write(nd->mnt);
 		if (error)
 			goto exit;
 		return 0;

@@ -1715,6 +1727,8 @@ ok:
 		goto exit;
 	return 0;
 
+exit_mutex_unlock:
+	mutex_unlock(&dir->d_inode->i_mutex);
 exit_dput:
 	dput_path(&path, nd);
 exit:

ipc/mqueue.c renamed from ipc/mqueue.c~C-elevate-writers-opens-part1 +3 -0

diff -puN ipc/mqueue.c~C-elevate-writers-opens-part1 ipc/mqueue.c
@@ -685,6 +685,9 @@ asmlinkage long sys_mq_open(const char _
 				goto out;
 			filp = do_open(dentry, oflag);
 		} else {
+			error = mnt_want_write(mqueue_mnt);
+			if (error)
+				goto out;
 			filp = do_create(mqueue_mnt->mnt_root, dentry,
 						oflag, mode, u_attr);
 		}

Dave Hansen

author

2006-07-12 18:18 UTC #19 lore raw

RFC PATCH 19/27 elevate writer count for do_sys_truncate()

In reply to Dave Hansen

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/open.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

fs/open.c renamed from fs/open.c~C-elevate-writers-opens-part2-do_sys_truncate +11 -5

diff -puN fs/open.c~C-elevate-writers-opens-part2-do_sys_truncate fs/open.c
@@ -244,28 +244,32 @@ static long do_sys_truncate(const char _
 	if (!S_ISREG(inode->i_mode))
 		goto dput_and_out;
 
-	error = vfs_permission(&nd, MAY_WRITE);
+	error = mnt_want_write(nd.mnt);
 	if (error)
 		goto dput_and_out;
 
+	error = vfs_permission(&nd, MAY_WRITE);
+	if (error)
+		goto mnt_drop_write_and_out;
+
 	error = -EROFS;
 	if (IS_RDONLY(inode))
-		goto dput_and_out;
+		goto mnt_drop_write_and_out;
 
 	error = -EPERM;
 	if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
-		goto dput_and_out;
+		goto mnt_drop_write_and_out;
 
 	/*
 	 * Make sure that there are no leases.
 	 */
 	error = break_lease(inode, FMODE_WRITE);
 	if (error)
-		goto dput_and_out;
+		goto mnt_drop_write_and_out;
 
 	error = get_write_access(inode);
 	if (error)
-		goto dput_and_out;
+		goto mnt_drop_write_and_out;
 
 	error = locks_verify_truncate(inode, NULL, length);
 	if (!error) {

@@ -274,6 +278,8 @@ static long do_sys_truncate(const char _
 	}
 	put_write_access(inode);
 
+mnt_drop_write_and_out:
+	mnt_drop_write(nd.mnt);
 dput_and_out:
 	path_release(&nd);
 out:

Dave Hansen

author

2006-07-12 18:18 UTC #20 lore raw

RFC PATCH 20/27 elevate write count for do_utimes()

In reply to Dave Hansen

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/open.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

fs/open.c renamed from fs/open.c~C-elevate-writers-opens-part2-do_utimes +9 -4

diff -puN fs/open.c~C-elevate-writers-opens-part2-do_utimes fs/open.c
@@ -441,16 +441,19 @@ long do_utimes(int dfd, char __user *fil
 		goto out;
 	inode = nd.dentry->d_inode;
 
+	error = mnt_want_write(nd.mnt);
+	if (error)
+		goto dput_and_out;
 	error = -EROFS;
 	if (IS_RDONLY(inode))
-		goto dput_and_out;
+		goto mnt_drop_write_and_out;
 
 	/* Don't worry, the checks are done in inode_change_ok() */
 	newattrs.ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_ATIME;
 	if (times) {
 		error = -EPERM;
                 if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
-                        goto dput_and_out;
+			goto mnt_drop_write_and_out;
 
 		newattrs.ia_atime.tv_sec = times[0].tv_sec;
 		newattrs.ia_atime.tv_nsec = times[0].tv_usec * 1000;

@@ -460,15 +463,17 @@ long do_utimes(int dfd, char __user *fil
 	} else {
 		error = -EACCES;
                 if (IS_IMMUTABLE(inode))
-                        goto dput_and_out;
+			goto mnt_drop_write_and_out;
 
 		if (current->fsuid != inode->i_uid &&
 		    (error = vfs_permission(&nd, MAY_WRITE)) != 0)
-			goto dput_and_out;
+			goto mnt_drop_write_and_out;
 	}
 	mutex_lock(&inode->i_mutex);
 	error = notify_change(nd.dentry, &newattrs);
 	mutex_unlock(&inode->i_mutex);
+mnt_drop_write_and_out:
+	mnt_drop_write(nd.mnt);
 dput_and_out:
 	path_release(&nd);
 out:

Dave Hansen

author

2006-07-12 18:18 UTC #21 lore raw

RFC PATCH 21/27 elevate write count for do_sys_utime() and touch_atime()

In reply to Dave Hansen

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/inode.c |    9 +++++++--
 lxc-dave/fs/open.c  |   16 +++++++++++-----
 2 files changed, 18 insertions(+), 7 deletions(-)

fs/inode.c renamed from fs/inode.c~C-elevate-writers-opens-part2-do_sys_utime +7 -2

diff -puN fs/inode.c~C-elevate-writers-opens-part2-do_sys_utime fs/inode.c
@@ -1191,10 +1191,13 @@ void touch_atime(struct vfsmount *mnt, s
 	if (IS_RDONLY(inode))
 		return;
 
+	if (mnt_want_write(mnt))
+		return;
+
 	if ((inode->i_flags & S_NOATIME) ||
 	    (inode->i_sb->s_flags & MS_NOATIME) ||
 	    ((inode->i_sb->s_flags & MS_NODIRATIME) && S_ISDIR(inode->i_mode)))
-		return;
+		goto out;
 
 	/*
 	 * We may have a NULL vfsmount when coming from NFSD

@@ -1202,13 +1205,15 @@ void touch_atime(struct vfsmount *mnt, s
 	if (mnt &&
 	    ((mnt->mnt_flags & MNT_NOATIME) ||
 	     ((mnt->mnt_flags & MNT_NODIRATIME) && S_ISDIR(inode->i_mode))))
-		return;
+		goto out;
 
 	now = current_fs_time(inode->i_sb);
 	if (!timespec_equal(&inode->i_atime, &now)) {
 		inode->i_atime = now;
 		mark_inode_dirty_sync(inode);
 	}
+out:
+	mnt_drop_write(mnt);
 }
 
 EXPORT_SYMBOL(touch_atime);

fs/open.c renamed from fs/open.c~C-elevate-writers-opens-part2-do_sys_utime +11 -5

diff -puN fs/open.c~C-elevate-writers-opens-part2-do_sys_utime fs/open.c
@@ -384,16 +384,20 @@ asmlinkage long sys_utime(char __user * 
 		goto out;
 	inode = nd.dentry->d_inode;
 
+	error = mnt_want_write(nd.mnt);
+	if (error)
+		goto dput_and_out;
+
 	error = -EROFS;
 	if (IS_RDONLY(inode))
-		goto dput_and_out;
+		goto mnt_drop_write_and_out;
 
 	/* Don't worry, the checks are done in inode_change_ok() */
 	newattrs.ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_ATIME;
 	if (times) {
 		error = -EPERM;
 		if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
-			goto dput_and_out;
+			goto mnt_drop_write_and_out;
 
 		error = get_user(newattrs.ia_atime.tv_sec, &times->actime);
 		newattrs.ia_atime.tv_nsec = 0;

@@ -401,21 +405,23 @@ asmlinkage long sys_utime(char __user * 
 			error = get_user(newattrs.ia_mtime.tv_sec, &times->modtime);
 		newattrs.ia_mtime.tv_nsec = 0;
 		if (error)
-			goto dput_and_out;
+			goto mnt_drop_write_and_out;
 
 		newattrs.ia_valid |= ATTR_ATIME_SET | ATTR_MTIME_SET;
 	} else {
                 error = -EACCES;
                 if (IS_IMMUTABLE(inode))
-                        goto dput_and_out;
+			goto mnt_drop_write_and_out;
 
 		if (current->fsuid != inode->i_uid &&
 		    (error = vfs_permission(&nd, MAY_WRITE)) != 0)
-			goto dput_and_out;
+			goto mnt_drop_write_and_out;
 	}
 	mutex_lock(&inode->i_mutex);
 	error = notify_change(nd.dentry, &newattrs);
 	mutex_unlock(&inode->i_mutex);
+mnt_drop_write_and_out:
+	mnt_drop_write(nd.mnt);
 dput_and_out:
 	path_release(&nd);
 out:

Dave Hansen

author

2006-07-12 18:20 UTC #22 lore raw

RFC PATCH 22/27 sys_mknodat(): elevate write count for vfs_mknod/create()

In reply to Dave Hansen

This takes care of all of the direct callers of vfs_mknod(). Since a few of these cases also handle normal file creation as well, this also covers some calls to vfs_create().

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/namei.c         |   12 ++++++++++++
 lxc-dave/fs/nfsd/vfs.c      |    4 ++++
 lxc-dave/net/unix/af_unix.c |    4 ++++
 3 files changed, 20 insertions(+)

fs/namei.c renamed from fs/namei.c~C-elevate-writers-vfs_mknod-try2 +12 -0

diff -puN fs/namei.c~C-elevate-writers-vfs_mknod-try2 fs/namei.c
@@ -1879,14 +1879,26 @@ asmlinkage long sys_mknodat(int dfd, con
 	if (!IS_ERR(dentry)) {
 		switch (mode & S_IFMT) {
 		case 0: case S_IFREG:
+			error = mnt_want_write(nd.mnt);
+			if (error)
+				break;
 			error = vfs_create(nd.dentry->d_inode,dentry,mode,&nd);
+			mnt_drop_write(nd.mnt);
 			break;
 		case S_IFCHR: case S_IFBLK:
+			error = mnt_want_write(nd.mnt);
+			if (error)
+				break;
 			error = vfs_mknod(nd.dentry->d_inode,dentry,mode,
 					new_decode_dev(dev));
+			mnt_drop_write(nd.mnt);
 			break;
 		case S_IFIFO: case S_IFSOCK:
+			error = mnt_want_write(nd.mnt);
+			if (error)
+				break;
 			error = vfs_mknod(nd.dentry->d_inode,dentry,mode,0);
+			mnt_drop_write(nd.mnt);
 			break;
 		case S_IFDIR:
 			error = -EPERM;

fs/nfsd/vfs.c renamed from fs/nfsd/vfs.c~C-elevate-writers-vfs_mknod-try2 +4 -0

diff -puN fs/nfsd/vfs.c~C-elevate-writers-vfs_mknod-try2 fs/nfsd/vfs.c
@@ -1155,6 +1155,9 @@ nfsd_create(struct svc_rqst *rqstp, stru
 	/*
 	 * Get the dir op function pointer.
 	 */
+	err = mnt_want_write(fhp->fh_export->ex_mnt);
+	if (err)
+		goto out_nfserr;
 	err = nfserr_perm;
 	switch (type) {
 	case S_IFREG:

@@ -1173,6 +1176,7 @@ nfsd_create(struct svc_rqst *rqstp, stru
 	        printk("nfsd: bad file type %o in nfsd_create\n", type);
 		err = -EINVAL;
 	}
+	mnt_drop_write(fhp->fh_export->ex_mnt);
 	if (err < 0)
 		goto out_nfserr;

net/unix/af_unix.c renamed from net/unix/af_unix.c~C-elevate-writers-vfs_mknod-try2 +4 -0

diff -puN net/unix/af_unix.c~C-elevate-writers-vfs_mknod-try2 net/unix/af_unix.c
@@ -822,7 +822,11 @@ static int unix_bind(struct socket *sock
 		 */
 		mode = S_IFSOCK |
 		       (SOCK_INODE(sock)->i_mode & ~current->fs->umask);
+		err = mnt_want_write(nd.mnt);
+		if (err)
+			goto out_mknod_dput;
 		err = vfs_mknod(nd.dentry->d_inode, dentry, mode, 0);
+		mnt_drop_write(nd.mnt);
 		if (err)
 			goto out_mknod_dput;
 		mutex_unlock(&nd.dentry->d_inode->i_mutex);

Dave Hansen

author

2006-07-12 18:17 UTC #23 lore raw

RFC PATCH 23/27 elevate mnt writers for vfs_unlink() callers

In reply to Dave Hansen

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/namei.c   |    4 ++++
 lxc-dave/ipc/mqueue.c |    5 ++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

fs/namei.c renamed from fs/namei.c~C-elevate-writers-vfs_unlink +4 -0

diff -puN fs/namei.c~C-elevate-writers-vfs_unlink fs/namei.c
@@ -2158,7 +2158,11 @@ static long do_unlinkat(int dfd, const c
 		inode = dentry->d_inode;
 		if (inode)
 			atomic_inc(&inode->i_count);
+		error = mnt_want_write(nd.mnt);
+		if (error)
+			goto exit2;
 		error = vfs_unlink(nd.dentry->d_inode, dentry);
+		mnt_drop_write(nd.mnt);
 	exit2:
 		dput(dentry);
 	}

ipc/mqueue.c renamed from ipc/mqueue.c~C-elevate-writers-vfs_unlink +4 -1

diff -puN ipc/mqueue.c~C-elevate-writers-vfs_unlink ipc/mqueue.c
@@ -747,8 +747,11 @@ asmlinkage long sys_mq_unlink(const char
 	inode = dentry->d_inode;
 	if (inode)
 		atomic_inc(&inode->i_count);
-
+	err = mnt_want_write(mqueue_mnt);
+	if (err)
+		goto out_err;
 	err = vfs_unlink(dentry->d_parent->d_inode, dentry);
+	mnt_drop_write(mqueue_mnt);
 out_err:
 	dput(dentry);

Dave Hansen

author

2006-07-12 18:17 UTC #24 lore raw

RFC PATCH 24/27 do_rmdir(): elevate write count

In reply to Dave Hansen

Elevate the write count during the vfs_rmdir() call.

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/namei.c |    5 +++++
 1 files changed, 5 insertions(+)

fs/namei.c renamed from fs/namei.c~C-rmdir1 +5 -0

diff -puN fs/namei.c~C-rmdir1 fs/namei.c
@@ -2078,7 +2078,12 @@ static long do_rmdir(int dfd, const char
 	error = PTR_ERR(dentry);
 	if (IS_ERR(dentry))
 		goto exit2;
+	error = mnt_want_write(nd.mnt);
+	if (error)
+		goto exit3;
 	error = vfs_rmdir(nd.dentry->d_inode, dentry);
+	mnt_drop_write(nd.mnt);
+exit3:
 	dput(dentry);
 exit2:
 	mutex_unlock(&nd.dentry->d_inode->i_mutex);

Dave Hansen

author

2006-07-12 18:20 UTC #25 lore raw

RFC PATCH 25/27 elevate writer count for custom 'struct file'

In reply to Dave Hansen

Some filesystems forego the vfs and may_open() and create their own 'struct file's. Any of these users which set the write flag on the file will cause an extra mnt_drop_write() on __fput(), thus dropping the reference count too low.

These users tend to have artifical in-kernel vfsmounts which aren't really exposed to userspace and can't be remounted, but this patch is included for completeness and so that the warnings don't trip over these cases.

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/hugetlbfs/inode.c |    2 ++
 lxc-dave/mm/shmem.c           |    2 ++
 lxc-dave/mm/tiny-shmem.c      |    2 ++
 lxc-dave/net/socket.c         |    3 +++
 4 files changed, 9 insertions(+)

fs/hugetlbfs/inode.c renamed from fs/hugetlbfs/inode.c~C-elevate-writer-count-for-custom-struct_file +2 -0

diff -puN fs/hugetlbfs/inode.c~C-elevate-writer-count-for-custom-struct_file fs/hugetlbfs/inode.c
@@ -787,6 +787,8 @@ struct file *hugetlb_zero_setup(size_t s
 	file->f_mapping = inode->i_mapping;
 	file->f_op = &hugetlbfs_file_operations;
 	file->f_mode = FMODE_WRITE | FMODE_READ;
+	error = mnt_want_write(hugetlbfs_vfsmount);
+	WARN_ON(error);
 	return file;
 
 out_inode:

mm/shmem.c renamed from mm/shmem.c~C-elevate-writer-count-for-custom-struct_file +2 -0

diff -puN mm/shmem.c~C-elevate-writer-count-for-custom-struct_file mm/shmem.c
@@ -2322,6 +2322,8 @@ struct file *shmem_file_setup(char *name
 	file->f_mapping = inode->i_mapping;
 	file->f_op = &shmem_file_operations;
 	file->f_mode = FMODE_WRITE | FMODE_READ;
+	error = mnt_want_write(shm_mnt);
+	WARN_ON(error);
 	return file;
 
 close_file:

mm/tiny-shmem.c renamed from mm/tiny-shmem.c~C-elevate-writer-count-for-custom-struct_file +2 -0

diff -puN mm/tiny-shmem.c~C-elevate-writer-count-for-custom-struct_file mm/tiny-shmem.c
@@ -84,6 +84,8 @@ struct file *shmem_file_setup(char *name
 	file->f_mapping = inode->i_mapping;
 	file->f_op = &ramfs_file_operations;
 	file->f_mode = FMODE_WRITE | FMODE_READ;
+	error = mnt_want_write(shm_mnt);
+	WARN_ON(error);
 
 	/* notify everyone as to the change of file size */
 	error = do_truncate(dentry, size, 0, file);

net/socket.c renamed from net/socket.c~C-elevate-writer-count-for-custom-struct_file +3 -0

diff -puN net/socket.c~C-elevate-writer-count-for-custom-struct_file net/socket.c
@@ -389,6 +389,7 @@ static int sock_attach_fd(struct socket 
 {
 	struct qstr this;
 	char name[32];
+	int error;
 
 	this.len = sprintf(name, "[%lu]", SOCK_INODE(sock)->i_ino);
 	this.name = name;

@@ -409,6 +410,8 @@ static int sock_attach_fd(struct socket 
 	file->f_flags = O_RDWR;
 	file->f_pos = 0;
 	file->private_data = sock;
+	error = mnt_want_write(sock_mnt);
+	WARN_ON(error);
 
 	return 0;
 }

Dave Hansen

author

2006-07-12 18:20 UTC #26 lore raw

RFC PATCH 26/27 Originally from: Herbert Poetzl <[email protected]>

In reply to Dave Hansen

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/namespace.c |   45 ++++++++++++++++++++++++---------------------
 1 files changed, 24 insertions(+), 21 deletions(-)

fs/namespace.c renamed from fs/namespace.c~D6-proc-show-ro-attr +24 ~~-21~~

diff -puN fs/namespace.c~D6-proc-show-ro-attr fs/namespace.c
@@ -374,24 +374,22 @@ static int show_vfsmnt(struct seq_file *
 {
 	struct vfsmount *mnt = v;
 	int err = 0;
+	int i;
 	static struct proc_fs_info {
-		int flag;
-		char *str;
+		int s_flag;
+		int mnt_flag;
+		char *set_str;
+		char *unset_str;
 	} fs_info[] = {
-		{ MS_SYNCHRONOUS, ",sync" },
-		{ MS_DIRSYNC, ",dirsync" },
-		{ MS_MANDLOCK, ",mand" },
-		{ 0, NULL }
-	};
-	static struct proc_fs_info mnt_info[] = {
-		{ MNT_NOSUID, ",nosuid" },
-		{ MNT_NODEV, ",nodev" },
-		{ MNT_NOEXEC, ",noexec" },
-		{ MNT_NOATIME, ",noatime" },
-		{ MNT_NODIRATIME, ",nodiratime" },
-		{ 0, NULL }
+		{ MS_SYNCHRONOUS, 0, ",sync", NULL },
+		{ MS_DIRSYNC, 0, ",dirsync", NULL },
+		{ MS_MANDLOCK, 0, ",mand", NULL },
+		{ 0, MNT_NOSUID, ",nosuid", NULL },
+		{ 0, MNT_NODEV, ",nodev", NULL },
+		{ 0, MNT_NOEXEC, ",noexec", NULL },
+		{ 0, MNT_NOATIME, ",noatime", NULL },
+		{ 0, MNT_NODIRATIME, ",nodiratime", NULL }
 	};
-	struct proc_fs_info *fs_infop;
 
 	mangle(m, mnt->mnt_devname ? mnt->mnt_devname : "none");
 	seq_putc(m, ' ');

@@ -399,13 +397,18 @@ static int show_vfsmnt(struct seq_file *
 	seq_putc(m, ' ');
 	mangle(m, mnt->mnt_sb->s_type->name);
 	seq_puts(m, mnt->mnt_sb->s_flags & MS_RDONLY ? " ro" : " rw");
-	for (fs_infop = fs_info; fs_infop->flag; fs_infop++) {
-		if (mnt->mnt_sb->s_flags & fs_infop->flag)
-			seq_puts(m, fs_infop->str);
-	}
-	for (fs_infop = mnt_info; fs_infop->flag; fs_infop++) {
+	for (i = 0; i < ARRAY_SIZE(fs_info); i++) {
+		struct proc_fs_info *fs_infop = &fs_info[i];
+		char *str = NULL;
+		if ((mnt->mnt_sb->s_flags & fs_infop->s_flag) ||
+		    mnt_flag_set(mnt, fs_infop->mnt_flag))
+			str = fs_infop->set_str;
+		else
+			str = fs_infop->unset_str;
+
 		if (mnt->mnt_flags & fs_infop->flag)
-			seq_puts(m, fs_infop->str);
+		if (str)
+			seq_puts(m, str);
 	}
 	if (mnt->mnt_sb->s_op->show_options)
 		err = mnt->mnt_sb->s_op->show_options(m, mnt);

Dave Hansen

author

2006-07-12 18:17 UTC #27 lore raw

RFC PATCH 27/27 honor r/w changes at do_remount() time

In reply to Dave Hansen

Originally from: Herbert Poetzl <[email protected]>

This is the core of the read-only bind mount patch set.

Note that this does _not_ add a "ro" option directly to the bind mount operation. If you require such a mount, you must first do the bind, then follow it up with a 'mount -o remount,ro' operation.

Signed-off-by Dave Hansen <[email protected]>

---

 lxc-dave/fs/namespace.c |   28 ++++++++++++++++++++++++----
 lxc-dave/fs/open.c      |    2 +-
 2 files changed, 25 insertions(+), 5 deletions(-)

fs/namespace.c renamed from fs/namespace.c~C-D8-actually-add-flags +24 -4

diff -puN fs/namespace.c~C-D8-actually-add-flags fs/namespace.c
@@ -381,6 +381,7 @@ static int show_vfsmnt(struct seq_file *
 		char *set_str;
 		char *unset_str;
 	} fs_info[] = {
+		{ MS_RDONLY, MNT_READONLY, "ro", "rw" },
 		{ MS_SYNCHRONOUS, 0, ",sync", NULL },
 		{ MS_DIRSYNC, 0, ",dirsync", NULL },
 		{ MS_MANDLOCK, 0, ",mand", NULL },

@@ -396,17 +397,16 @@ static int show_vfsmnt(struct seq_file *
 	seq_path(m, mnt, mnt->mnt_root, " \t\n\\");
 	seq_putc(m, ' ');
 	mangle(m, mnt->mnt_sb->s_type->name);
-	seq_puts(m, mnt->mnt_sb->s_flags & MS_RDONLY ? " ro" : " rw");
+	seq_putc(m, ' ');
 	for (i = 0; i < ARRAY_SIZE(fs_info); i++) {
 		struct proc_fs_info *fs_infop = &fs_info[i];
 		char *str = NULL;
 		if ((mnt->mnt_sb->s_flags & fs_infop->s_flag) ||
-		    mnt_flag_set(mnt, fs_infop->mnt_flag))
+		    (mnt->mnt_flags & fs_infop->mnt_flag))
 			str = fs_infop->set_str;
 		else
 			str = fs_infop->unset_str;
 
-		if (mnt->mnt_flags & fs_infop->flag)
 		if (str)
 			seq_puts(m, str);
 	}

@@ -1024,6 +1024,23 @@ out:
 	return err;
 }
 
+static int change_mount_flags(struct vfsmount *mnt, int ms_flags)
+{
+	int error = 0;
+	int readonly_request = 0;
+
+	if (ms_flags & MS_RDONLY)
+		readonly_request = 1;
+	if (readonly_request == __mnt_is_readonly(mnt))
+		return 0;
+
+	if (readonly_request)
+		error = mnt_make_readonly(mnt);
+	else
+		__mnt_unmake_readonly(mnt);
+	return error;
+}
+
 /*
  * change filesystem flags. dir should be a physical root of filesystem.
  * If you've mounted a non-root directory somewhere and want to do remount

@@ -1045,7 +1062,10 @@ static int do_remount(struct nameidata *
 		return -EINVAL;
 
 	down_write(&sb->s_umount);
-	err = do_remount_sb(sb, flags, data, 0);
+	if (flags & MS_BIND)
+		err = change_mount_flags(nd->mnt, flags);
+	else
+		err = do_remount_sb(sb, flags, data, 0);
 	if (!(sb->s_flags & MS_RDONLY))
 		mnt_flags |= MNT_SB_WRITABLE;
 	if (!err)

fs/open.c renamed from fs/open.c~C-D8-actually-add-flags +1 -1

diff -puN fs/open.c~C-D8-actually-add-flags fs/open.c
@@ -546,7 +546,7 @@ asmlinkage long sys_faccessat(int dfd, c
 	   special_file(nd.dentry->d_inode->i_mode))
 		goto out_path_release;
 
-	if(IS_RDONLY(nd.dentry->d_inode))
+	if(__mnt_is_readonly(nd.mnt) || IS_RDONLY(nd.dentry->d_inode))
 		res = -EROFS;
 
 out_path_release:

Ram Pai

2006-07-13 07:07 UTC #28 lore raw

In reply to Dave Hansen

Quoted 10 lines · Dave Hansen

Tries to incorporate comments from Al:
http://article.gmane.org/gmane.linux.kernel/421029
Quoted 2 lines · Al
b) figuring out what (if anything) should be done with propagation when we have shared subtrees... (not trivial at all)
Talked with Ram: Shared subtrees are about having identical views into the filesystem. Changing the mount permissions doesn't affect the view of the filesystem, so it should not be propagated.

I think shared subtrees propagates mount/unmount events only. This is a case where we are just changing the access permission for a mount instance. So it should not be treated as a propagation event.

Lets say we propagated the event. In that case we would end up with a awkward situation.

1) mount --make-shared /mnt
2) mount --bind /mnt /tmp
3) mount --make-slave /tmp
4) mount -o remount,rw /tmp
5) mount -o remount,ro /mnt

In step (5) all of a sudden the mount at /tmp which was readwrite will be downgraded to read-only. There must have been a reason why /tmp was mounted rw in the first place. Also there would be no way for /mnt to be made 'ro' without effecting /tmp.

Hence I feel the readwrite semantics must be decoupled from the sharedsubtree semantics,

Quoted 38 lines

The things that probably need the heaviest review in here are the i_nlink monitoring patch (including the inode state flag patches 03 and 06) and the new MNT_SB_WRITABLE flag (patch 05).
---
The following series implements read-only bind mounts. This feature allows a read-only view into a read-write filesystem. In the process of doing that, it also provides infrastructure for keeping track of the number of writers to any given mount. In this version, if there are writers on a superblock, the filesystem may not be remounted r/o. The same goes for MS_BIND mounts, and writers on a vfsmount.
This has a number of uses. It allows chroots to have parts of filesystems writable. It will be useful for containers in the future and is intended to replace patches that vserver has had out of the tree for several years. It allows security enhancement by making sure that parts of your filesystem read-only, when you don't want to have entire new filesystems mounted, or when you want atime selectively updated.
This set makes no attempt to keep the return codes for these r/o bind mounts the same as for a real r/o filesystem or device. It would require significantly more code and be quite a bit more invasive.
Using this feature requires two steps:
	mount --bind /source /dest
	mount -o remount,ro  /dest
Signed-off-by Dave Hansen <[email protected]>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/