linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] (1/11) fs/super.c fixes
@ 2001-08-14  1:31 Alexander Viro
  2001-08-14  1:32 ` [PATCH] (2/11) " Alexander Viro
  2001-08-14  3:56 ` [PATCH] (1/11) " Linus Torvalds
  0 siblings, 2 replies; 16+ messages in thread
From: Alexander Viro @ 2001-08-14  1:31 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel

	Linus, I'm resending the second series of superblock handling
fixes. There are 11 chunks (the largest being 5Kb) - I'm sending each
of them in a separate mail (with descriptions). This stuff finally
closes the get_super() races and seriously cleans superblock handling.
It had been in -ac for a while (and got more than a month of testing on
local boxen). Please, apply.

Part 1/11

We grab exclusive lock on ->s_umount() in get_sb_...() and release it
once vfsmount is formed (by do_add_mount()). No deadlocks, since all
activity in protected area is already not allowed to lead to anything
that would grab ->s_umount. We hold ->s_lock over the whole non-trivial part
and anything of that kind would deadlock on it.

We are almost done with the get_super() races by now.

diff -urN S9-pre3/fs/super.c S9-pre3-s_umount/fs/super.c
--- S9-pre3/fs/super.c	Sat Aug 11 14:59:24 2001
+++ S9-pre3-s_umount/fs/super.c	Mon Aug 13 21:21:26 2001
@@ -820,6 +820,7 @@
 	spin_lock(&sb_lock);
 	list_add (&s->s_list, super_blocks.prev);
 	spin_unlock(&sb_lock);
+	down_write(&s->s_umount);
 	lock_super(s);
 	if (!type->read_super(s, data, silent))
 		goto out_fail;
@@ -839,7 +840,7 @@
 	spin_lock(&sb_lock);
 	list_del(&s->s_list);
 	spin_unlock(&sb_lock);
-	__put_super(s);
+	put_super(s);
 	return NULL;
 }
 
@@ -919,7 +920,9 @@
 				spin_unlock(&sb_lock);
 			}
 			atomic_inc(&sb->s_active);
+			/* Next chunk will drop it */
 			up_read(&sb->s_umount);
+			down_write(&sb->s_umount);
 			path_release(&nd);
 			return sb;
 		}
@@ -986,6 +989,7 @@
 		BUG();
 	atomic_inc(&sb->s_active);
 	do_remount_sb(sb, flags, data);
+	down_write(&sb->s_umount);
 	return sb;
 }
 
@@ -1104,6 +1108,7 @@
 	mnt->mnt_mountpoint = mnt->mnt_root;
 	mnt->mnt_parent = mnt;
 	type->kern_mnt = mnt;
+	up_write(&sb->s_umount);
 	return mnt;
 }
 
@@ -1379,6 +1384,7 @@
 	mnt->mnt_root = dget(sb->s_root);
 	mnt->mnt_mountpoint = mnt->mnt_root;
 	mnt->mnt_parent = mnt;
+	up_write(&sb->s_umount);
 
 	/* Something was mounted here while we slept */
 	while(d_mountpoint(nd->dentry) && follow_down(&nd->mnt, &nd->dentry))
@@ -1639,6 +1645,7 @@
 		fs_type = sb->s_type;
 		atomic_inc(&sb->s_active);
 		up_read(&sb->s_umount);
+		down_write(&sb->s_umount);
 		goto mount_it;
 	}
 
@@ -1659,6 +1666,8 @@
 	panic("VFS: Unable to mount root fs on %s", kdevname(ROOT_DEV));
 
 mount_it:
+	/* FIXME */
+	up_write(&sb->s_umount);
 	printk ("VFS: Mounted root (%s filesystem)%s.\n",
 		fs_type->name,
 		(sb->s_flags & MS_RDONLY) ? " readonly" : "");


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

* [PATCH] (2/11) fs/super.c fixes
  2001-08-14  1:31 [PATCH] (1/11) fs/super.c fixes Alexander Viro
@ 2001-08-14  1:32 ` Alexander Viro
  2001-08-14  1:32   ` [PATCH] (3/11) " Alexander Viro
  2001-08-14  3:56 ` [PATCH] (1/11) " Linus Torvalds
  1 sibling, 1 reply; 16+ messages in thread
From: Alexander Viro @ 2001-08-14  1:32 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel


Part 2/11

First part of get_sb_bdev() rewrite. We move opening the device to the
beginning of the function. If we already have a superblock from that device
- well, no problem. That, BTW, fixes a buglet with permissions: suppose
we mount /dev/foo, chmod it to 0 and say mount /dev/foo again. Old code
merrily didn't notice that permissions had been revoked and allowed mount.
Main reason for the change is different, though - we are getting the
blocking operations from the area we want to protect with sb_lock (see
the next chunk).


Cleanup: we move decrementing ->s_active into put_super(). Callers updated.

diff -urN S9-pre3-s_umount/fs/super.c S9-pre3-get_sb_bdev/fs/super.c
--- S9-pre3-s_umount/fs/super.c	Mon Aug 13 21:21:26 2001
+++ S9-pre3-get_sb_bdev/fs/super.c	Mon Aug 13 21:21:26 2001
@@ -872,6 +872,26 @@
 			kdevname(dev));
 }
 
+static int grab_super(struct super_block *sb)
+{
+	sb->s_count++;
+	atomic_inc(&sb->s_active);
+	spin_unlock(&sb_lock);
+	down_write(&sb->s_umount);
+	if (sb->s_root) {
+		/* Still relying on mount_sem */
+		if (atomic_read(&sb->s_active) > 1) {
+			spin_lock(&sb_lock);
+			sb->s_count--;
+			spin_unlock(&sb_lock);
+			return 1;
+		}
+	}
+	atomic_dec(&sb->s_active);
+	put_super(sb);
+	return 0;
+}
+
 static struct super_block *get_sb_bdev(struct file_system_type *fs_type,
 	char *dev_name, int flags, void * data)
 {
@@ -880,8 +900,11 @@
 	struct block_device_operations *bdops;
 	struct super_block * sb;
 	struct nameidata nd;
+	struct list_head *p;
 	kdev_t dev;
 	int error = 0;
+	mode_t mode = FMODE_READ; /* we always need it ;-) */
+
 	/* What device it is? */
 	if (!dev_name || !*dev_name)
 		return ERR_PTR(-EINVAL);
@@ -902,52 +925,45 @@
 	/* Done with lookups, semaphore down */
 	down(&mount_sem);
 	dev = to_kdev_t(bdev->bd_dev);
-	sb = get_super(dev);
-	if (sb) {
-		if (fs_type == sb->s_type &&
-		    ((flags ^ sb->s_flags) & MS_RDONLY) == 0) {
-/*
- * We are heavily relying on mount_sem here. We _will_ get rid of that
- * ugliness RSN (and then atomicity of ->s_active will play), but first
- * we need to get rid of "reuse" branch of get_empty_super() and that
- * requires reference counters. Chicken and egg problem, but fortunately
- * we can use the fact that right now all accesses to ->s_active are
- * under mount_sem.
- */
-			if (atomic_read(&sb->s_active)) {
-				spin_lock(&sb_lock);
-				sb->s_count--;
-				spin_unlock(&sb_lock);
-			}
-			atomic_inc(&sb->s_active);
-			/* Next chunk will drop it */
-			up_read(&sb->s_umount);
-			down_write(&sb->s_umount);
-			path_release(&nd);
-			return sb;
-		}
-		drop_super(sb);
-	} else {
-		mode_t mode = FMODE_READ; /* we always need it ;-) */
-		if (!(flags & MS_RDONLY))
-			mode |= FMODE_WRITE;
-		error = blkdev_get(bdev, mode, 0, BDEV_FS);
-		if (error)
-			goto out;
-		check_disk_change(dev);
-		error = -EACCES;
-		if (!(flags & MS_RDONLY) && is_read_only(dev))
+	if (!(flags & MS_RDONLY))
+		mode |= FMODE_WRITE;
+	error = blkdev_get(bdev, mode, 0, BDEV_FS);
+	if (error)
+		goto out;
+	check_disk_change(dev);
+	error = -EACCES;
+	if (!(flags & MS_RDONLY) && is_read_only(dev))
+		goto out1;
+
+	error = -EBUSY;
+restart:
+	spin_lock(&sb_lock);
+
+	list_for_each(p, &super_blocks) {
+		struct super_block *old = sb_entry(p);
+		if (old->s_dev != dev)
+			continue;
+		if (old->s_type != fs_type ||
+		    ((flags ^ old->s_flags) & MS_RDONLY)) {
+			spin_unlock(&sb_lock);
 			goto out1;
-		error = -EINVAL;
-		sb = read_super(dev, bdev, fs_type, flags, data, 0);
-		if (sb) {
-			get_filesystem(fs_type);
-			path_release(&nd);
-			return sb;
 		}
-out1:
+		if (!grab_super(old))
+			goto restart;
 		blkdev_put(bdev, BDEV_FS);
+		path_release(&nd);
+		return old;
 	}
+	spin_unlock(&sb_lock);
+	error = -EINVAL;
+	sb = read_super(dev, bdev, fs_type, flags, data, 0);
+	if (sb) {
+		get_filesystem(fs_type);
+		path_release(&nd);
+		return sb;
+	}
+out1:
+	blkdev_put(bdev, BDEV_FS);
 out:
 	path_release(&nd);
 	up(&mount_sem);
diff -urN S9-pre3-s_count/fs/super.c S9-pre3-put_super/fs/super.c
--- S9-pre3-s_count/fs/super.c	Mon Aug 13 21:21:29 2001
+++ S9-pre3-put_super/fs/super.c	Mon Aug 13 21:21:29 2001
@@ -666,6 +666,7 @@
 
 static void put_super(struct super_block *sb)
 {
+	atomic_dec(&sb->s_active);
 	up_write(&sb->s_umount);
 	__put_super(sb);
 }
@@ -832,7 +833,6 @@
 	s->s_bdev = 0;
 	s->s_type = NULL;
 	unlock_super(s);
-	atomic_dec(&s->s_active);
 	spin_lock(&sb_lock);
 	list_del(&s->s_list);
 	list_del(&s->s_instances);
@@ -885,7 +885,6 @@
 		}
 		spin_unlock(&sb_lock);
 	}
-	atomic_dec(&sb->s_active);
 	put_super(sb);
 	return 0;
 }
@@ -950,13 +949,11 @@
 		if (old->s_type != fs_type ||
 		    ((flags ^ old->s_flags) & MS_RDONLY)) {
 			spin_unlock(&sb_lock);
-			atomic_dec(&s->s_active);
 			put_super(s);
 			goto out1;
 		}
 		if (!grab_super(old))
 			goto restart;
-		atomic_dec(&s->s_active);
 		put_super(s);
 		blkdev_put(bdev, BDEV_FS);
 		path_release(&nd);
@@ -988,7 +985,6 @@
 	s->s_bdev = 0;
 	s->s_type = NULL;
 	unlock_super(s);
-	atomic_dec(&s->s_active);
 	spin_lock(&sb_lock);
 	list_del(&s->s_list);
 	list_del(&s->s_instances);
@@ -1044,14 +1040,12 @@
 				s_instances);
 		if (!grab_super(old))
 			goto retry;
-		atomic_dec(&s->s_active);
 		put_super(s);
 		do_remount_sb(old, flags, data);
 		return old;
 	} else {
 		kdev_t dev = get_unnamed_dev();
 		if (!dev) {
-			atomic_dec(&s->s_active);
 			put_super(s);
 			up(&mount_sem);
 			return ERR_PTR(-EMFILE);
@@ -1075,7 +1069,6 @@
 		s->s_bdev = 0;
 		s->s_type = NULL;
 		unlock_super(s);
-		atomic_dec(&s->s_active);
 		spin_lock(&sb_lock);
 		list_del(&s->s_list);
 		list_del(&s->s_instances);
@@ -1142,6 +1135,7 @@
 	list_del(&sb->s_list);
 	list_del(&sb->s_instances);
 	spin_unlock(&sb_lock);
+	atomic_inc(&sb->s_active);
 	put_super(sb);
 }
 



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

* [PATCH] (3/11) fs/super.c fixes
  2001-08-14  1:32 ` [PATCH] (2/11) " Alexander Viro
@ 2001-08-14  1:32   ` Alexander Viro
  2001-08-14  1:33     ` [PATCH] (4/11) " Alexander Viro
  0 siblings, 1 reply; 16+ messages in thread
From: Alexander Viro @ 2001-08-14  1:32 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel


Part 3/11

And now the second part - we expand the call of read_super() in get_sb_bdev()
and move superblock allocation in the beginning of the function. That
gets the search in super_blocks for existing superblocks from our device
together with insertion of new superblock into the list (if search fails, that
is). We hold sb_lock over that area, which makes the whole "add if we hadn't
one" thing atomic.

diff -urN S9-pre3-get_sb_bdev/fs/super.c S9-pre3-get_sb_bdev2/fs/super.c
--- S9-pre3-get_sb_bdev/fs/super.c	Mon Aug 13 21:21:26 2001
+++ S9-pre3-get_sb_bdev2/fs/super.c	Mon Aug 13 21:21:26 2001
@@ -898,7 +898,7 @@
 	struct inode *inode;
 	struct block_device *bdev;
 	struct block_device_operations *bdops;
-	struct super_block * sb;
+	struct super_block * s;
 	struct nameidata nd;
 	struct list_head *p;
 	kdev_t dev;
@@ -935,6 +935,12 @@
 	if (!(flags & MS_RDONLY) && is_read_only(dev))
 		goto out1;
 
+	error = -ENOMEM;
+	s = alloc_super();
+	if (!s)
+		goto out1;
+	down_write(&s->s_umount);
+
 	error = -EBUSY;
 restart:
 	spin_lock(&sb_lock);
@@ -946,22 +952,47 @@
 		if (old->s_type != fs_type ||
 		    ((flags ^ old->s_flags) & MS_RDONLY)) {
 			spin_unlock(&sb_lock);
+			atomic_dec(&s->s_active);
+			put_super(s);
 			goto out1;
 		}
 		if (!grab_super(old))
 			goto restart;
+		atomic_dec(&s->s_active);
+		put_super(s);
 		blkdev_put(bdev, BDEV_FS);
 		path_release(&nd);
 		return old;
 	}
+	s->s_dev = dev;
+	s->s_bdev = bdev;
+	s->s_flags = flags;
+	s->s_type = fs_type;
+	list_add (&s->s_list, super_blocks.prev);
+
 	spin_unlock(&sb_lock);
+
 	error = -EINVAL;
-	sb = read_super(dev, bdev, fs_type, flags, data, 0);
-	if (sb) {
-		get_filesystem(fs_type);
-		path_release(&nd);
-		return sb;
-	}
+	lock_super(s);
+	if (!fs_type->read_super(s, data, 0))
+		goto out_fail;
+	unlock_super(s);
+	/* tell bdcache that we are going to keep this one */
+	atomic_inc(&bdev->bd_count);
+	get_filesystem(fs_type);
+	path_release(&nd);
+	return s;
+
+out_fail:
+	s->s_dev = 0;
+	s->s_bdev = 0;
+	s->s_type = NULL;
+	unlock_super(s);
+	atomic_dec(&s->s_active);
+	spin_lock(&sb_lock);
+	list_del(&s->s_list);
+	spin_unlock(&sb_lock);
+	put_super(s);
 out1:
 	blkdev_put(bdev, BDEV_FS);
 out:



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

* [PATCH] (4/11) fs/super.c fixes
  2001-08-14  1:32   ` [PATCH] (3/11) " Alexander Viro
@ 2001-08-14  1:33     ` Alexander Viro
  2001-08-14  1:33       ` [PATCH] (5/11) " Alexander Viro
  0 siblings, 1 reply; 16+ messages in thread
From: Alexander Viro @ 2001-08-14  1:33 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel


Part 4/11

do_remount_sb() placed under the exclusive lock on ->s_umount. All it takes
is replacement of fsync_dev() with fsync_super(). Callers updated.

diff -urN S9-pre3-get_sb_bdev2/fs/super.c S9-pre3-do_remount_sb/fs/super.c
--- S9-pre3-get_sb_bdev2/fs/super.c	Mon Aug 13 21:21:26 2001
+++ S9-pre3-do_remount_sb/fs/super.c	Mon Aug 13 21:21:27 2001
@@ -1035,8 +1035,8 @@
 	if (!sb)
 		BUG();
 	atomic_inc(&sb->s_active);
-	do_remount_sb(sb, flags, data);
 	down_write(&sb->s_umount);
+	do_remount_sb(sb, flags, data);
 	return sb;
 }
 
@@ -1107,7 +1107,7 @@
 	if (flags & MS_RDONLY)
 		acct_auto_close(sb->s_dev);
 	shrink_dcache_sb(sb);
-	fsync_dev(sb->s_dev);
+	fsync_super(sb);
 	/* If we are remounting RDONLY, make sure there are no rw files open */
 	if ((flags & MS_RDONLY) && !(sb->s_flags & MS_RDONLY))
 		if (!fs_may_remount_ro(sb))
@@ -1192,8 +1192,11 @@
 		 * Special case for "unmounting" root ...
 		 * we just try to remount it readonly.
 		 */
-		if (!(sb->s_flags & MS_RDONLY))
+		if (!(sb->s_flags & MS_RDONLY)) {
+			down_write(&sb->s_umount);
 			retval = do_remount_sb(sb, MS_RDONLY, 0);
+			up_write(&sb->s_umount);
+		}
 		return retval;
 	}
 
@@ -1369,13 +1372,19 @@
 
 static int do_remount(struct nameidata *nd, int flags, char *data)
 {
+	int err;
+	struct super_block * sb = nd->mnt->mnt_sb;
+
 	if (!capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
 	if (nd->dentry != nd->mnt->mnt_root)
 		return -EINVAL;
 
-	return do_remount_sb(nd->mnt->mnt_sb, flags, data);
+	down_write(&sb->s_umount);
+	err = do_remount_sb(sb, flags, data);
+	up_write(&sb->s_umount);
+	return err;
 }
 
 static int do_add_mount(struct nameidata *nd, char *type, int flags,



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

* [PATCH] (5/11) fs/super.c fixes
  2001-08-14  1:33     ` [PATCH] (4/11) " Alexander Viro
@ 2001-08-14  1:33       ` Alexander Viro
  2001-08-14  1:33         ` [PATCH] (6/11) " Alexander Viro
  0 siblings, 1 reply; 16+ messages in thread
From: Alexander Viro @ 2001-08-14  1:33 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel


Part 5/11

First step of 4-parter that corrects the idiotic misdesign I've done a
year ago when I was doing FS_SINGLE/kern_mount()/kern_umount(). Here we
go:
	* all superblocks of given type are placed on a list anchored in
struct file_system_type. It starts at ->fs_supers and goes through the
->s_instances of superblocks in question. Protected by sb_lock.

diff -urN S9-pre3-do_remount_sb/fs/super.c S9-pre3-fs_supers/fs/super.c
--- S9-pre3-do_remount_sb/fs/super.c	Mon Aug 13 21:21:27 2001
+++ S9-pre3-fs_supers/fs/super.c	Mon Aug 13 21:21:27 2001
@@ -122,6 +122,7 @@
 		return -EINVAL;
 	if (fs->next)
 		return -EBUSY;
+	INIT_LIST_HEAD(&fs->fs_supers);
 	write_lock(&file_systems_lock);
 	p = find_filesystem(fs->name);
 	if (*p)
@@ -792,6 +793,7 @@
 		INIT_LIST_HEAD(&s->s_dirty);
 		INIT_LIST_HEAD(&s->s_locked_inodes);
 		INIT_LIST_HEAD(&s->s_files);
+		INIT_LIST_HEAD(&s->s_instances);
 		init_rwsem(&s->s_umount);
 		sema_init(&s->s_lock, 1);
 		s->s_count = 1;
@@ -819,6 +821,7 @@
 	s->s_type = type;
 	spin_lock(&sb_lock);
 	list_add (&s->s_list, super_blocks.prev);
+	list_add (&s->s_instances, &type->fs_supers);
 	spin_unlock(&sb_lock);
 	down_write(&s->s_umount);
 	lock_super(s);
@@ -839,6 +842,7 @@
 	atomic_dec(&s->s_active);
 	spin_lock(&sb_lock);
 	list_del(&s->s_list);
+	list_del(&s->s_instances);
 	spin_unlock(&sb_lock);
 	put_super(s);
 	return NULL;
@@ -969,6 +973,7 @@
 	s->s_flags = flags;
 	s->s_type = fs_type;
 	list_add (&s->s_list, super_blocks.prev);
+	list_add (&s->s_instances, &fs_type->fs_supers);
 
 	spin_unlock(&sb_lock);
 
@@ -991,6 +996,7 @@
 	atomic_dec(&s->s_active);
 	spin_lock(&sb_lock);
 	list_del(&s->s_list);
+	list_del(&s->s_instances);
 	spin_unlock(&sb_lock);
 	put_super(s);
 out1:
@@ -1088,6 +1094,7 @@
 		put_unnamed_dev(dev);
 	spin_lock(&sb_lock);
 	list_del(&sb->s_list);
+	list_del(&sb->s_instances);
 	spin_unlock(&sb_lock);
 	put_super(sb);
 }
diff -urN S9-pre3-do_remount_sb/include/linux/fs.h S9-pre3-fs_supers/include/linux/fs.h
--- S9-pre3-do_remount_sb/include/linux/fs.h	Mon Aug 13 21:16:35 2001
+++ S9-pre3-fs_supers/include/linux/fs.h	Mon Aug 13 21:21:27 2001
@@ -688,6 +688,7 @@
 	struct list_head	s_files;
 
 	struct block_device	*s_bdev;
+	struct list_head	s_instances;
 	struct quota_mount_options s_dquot;	/* Diskquota specific options */
 
 	union {
@@ -915,6 +916,7 @@
 	struct module *owner;
 	struct vfsmount *kern_mnt; /* For kernel mount, if it's FS_SINGLE fs */
 	struct file_system_type * next;
+	struct list_head fs_supers;
 };
 
 #define DECLARE_FSTYPE(var,type,read,flags) \



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

* [PATCH] (6/11) fs/super.c fixes
  2001-08-14  1:33       ` [PATCH] (5/11) " Alexander Viro
@ 2001-08-14  1:33         ` Alexander Viro
  2001-08-14  1:34           ` [PATCH] (7/11) " Alexander Viro
  0 siblings, 1 reply; 16+ messages in thread
From: Alexander Viro @ 2001-08-14  1:33 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel


Part 6/11

	* get_sb_single() doesn't rely on kern_mount() being already done.
It simply checks list_empty(&type->fs_supers) and if we already have a
superblocks - grabs and returns it. If we don't have one it does the
right thing - inserts preallocated superblock into the ->fs_supers (and
super_blocks, indeed) and does ->read_super() on it, etc.

	It means that get_sb_single() works regardless of the kern_mount() -
if the latter had been done the thing will pick the superblock as it used
to do, if not - no problems.

diff -urN S9-pre3-fs_supers/fs/super.c S9-pre3-get_sb_single/fs/super.c
--- S9-pre3-fs_supers/fs/super.c	Mon Aug 13 21:21:27 2001
+++ S9-pre3-get_sb_single/fs/super.c	Mon Aug 13 21:21:27 2001
@@ -1031,19 +1031,62 @@
 static struct super_block *get_sb_single(struct file_system_type *fs_type,
 	int flags, void *data)
 {
-	struct super_block * sb;
+	struct super_block * s = alloc_super();
+	if (!s)
+		return ERR_PTR(-ENOMEM);
+	down_write(&s->s_umount);
 	/*
 	 * Get the superblock of kernel-wide instance, but
 	 * keep the reference to fs_type.
 	 */
 	down(&mount_sem);
-	sb = fs_type->kern_mnt->mnt_sb;
-	if (!sb)
-		BUG();
-	atomic_inc(&sb->s_active);
-	down_write(&sb->s_umount);
-	do_remount_sb(sb, flags, data);
-	return sb;
+retry:
+	spin_lock(&sb_lock);
+	if (!list_empty(&fs_type->fs_supers)) {
+		struct super_block *old;
+		old = list_entry(fs_type->fs_supers.next, struct super_block,
+				s_instances);
+		if (!grab_super(old))
+			goto retry;
+		atomic_dec(&s->s_active);
+		put_super(s);
+		do_remount_sb(old, flags, data);
+		return old;
+	} else {
+		kdev_t dev = get_unnamed_dev();
+		if (!dev) {
+			atomic_dec(&s->s_active);
+			put_super(s);
+			up(&mount_sem);
+			return ERR_PTR(-EMFILE);
+		}
+		s->s_dev = dev;
+		s->s_flags = flags;
+		s->s_type = fs_type;
+		list_add (&s->s_list, super_blocks.prev);
+		list_add (&s->s_instances, &fs_type->fs_supers);
+		spin_unlock(&sb_lock);
+		lock_super(s);
+		if (!fs_type->read_super(s, data, 0))
+			goto out_fail;
+		unlock_super(s);
+		return s;
+
+	out_fail:
+		s->s_dev = 0;
+		s->s_bdev = 0;
+		s->s_type = NULL;
+		unlock_super(s);
+		atomic_dec(&s->s_active);
+		spin_lock(&sb_lock);
+		list_del(&s->s_list);
+		list_del(&s->s_instances);
+		spin_unlock(&sb_lock);
+		put_super(s);
+		put_unnamed_dev(dev);
+		up(&mount_sem);
+		return ERR_PTR(-EINVAL);
+	}
 }
 
 static void kill_super(struct super_block *sb)



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

* [PATCH] (7/11) fs/super.c fixes
  2001-08-14  1:33         ` [PATCH] (6/11) " Alexander Viro
@ 2001-08-14  1:34           ` Alexander Viro
  2001-08-14  1:34             ` [PATCH] (8/11) " Alexander Viro
  0 siblings, 1 reply; 16+ messages in thread
From: Alexander Viro @ 2001-08-14  1:34 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel


Part 7/11

	* Now we can drop kern_mount() from the places that don't need it.
I.e. unless the kernel itself needs private vfsmount - no need to bother.
	* kern_mount() itself is simplified - it calls get_sb_nodev() or
get_sb_single() instead of doing stuff by hands. As the matter of fact,
it's quite similar to part of do_add_mount(). It's really "get the
private vfsmount, but don't bother attaching it" - what kern_mount()
was always supposed to be.
	* Refcounting for FS_SINGLE filesystems is not different from the
rest now. As the matter of fact, the only difference between "single" and
"nodev" is that the former refuses to have multiple instances and returns
an existing one if found. All special-casing is gone.
	* shmfs explicitly stores result of its kern_mount() in variable
instead of using ->kern_mnt (preparation to the next step).

BTW, now we can call kern_mount() several times on the same fs if we find
it convenient for some reason.

In short, lots of crap is gone now. I should've done it that way from the
very beginning and life would be easier for everyone.

diff -urN S9-pre3-get_sb_single/drivers/usb/inode.c S9-pre3-kern_mount/drivers/usb/inode.c
--- S9-pre3-get_sb_single/drivers/usb/inode.c	Sun Jul 29 01:54:47 2001
+++ S9-pre3-kern_mount/drivers/usb/inode.c	Mon Aug 13 21:21:28 2001
@@ -751,7 +751,6 @@
 		usb_deregister(&usbdevfs_driver);
 		return ret;
 	}
-	kern_mount(&usbdevice_fs_type);
 #ifdef CONFIG_PROC_FS		
 	/* create mount point for usbdevfs */
 	usbdir = proc_mkdir("usb", proc_bus);
@@ -763,7 +762,6 @@
 {
 	usb_deregister(&usbdevfs_driver);
 	unregister_filesystem(&usbdevice_fs_type);
-	kern_umount(usbdevice_fs_type.kern_mnt);
 #ifdef CONFIG_PROC_FS	
         if (usbdir)
                 remove_proc_entry("usb", proc_bus);
diff -urN S9-pre3-get_sb_single/fs/pipe.c S9-pre3-kern_mount/fs/pipe.c
--- S9-pre3-get_sb_single/fs/pipe.c	Fri Feb 16 22:52:04 2001
+++ S9-pre3-kern_mount/fs/pipe.c	Mon Aug 13 21:21:28 2001
@@ -630,8 +630,7 @@
 	return sb;
 }
 
-static DECLARE_FSTYPE(pipe_fs_type, "pipefs", pipefs_read_super,
-	FS_NOMOUNT|FS_SINGLE);
+static DECLARE_FSTYPE(pipe_fs_type, "pipefs", pipefs_read_super, FS_NOMOUNT);
 
 static int __init init_pipe_fs(void)
 {
@@ -650,7 +649,7 @@
 static void __exit exit_pipe_fs(void)
 {
 	unregister_filesystem(&pipe_fs_type);
-	kern_umount(pipe_mnt);
+	mntput(pipe_mnt);
 }
 
 module_init(init_pipe_fs)
diff -urN S9-pre3-get_sb_single/fs/super.c S9-pre3-kern_mount/fs/super.c
--- S9-pre3-get_sb_single/fs/super.c	Mon Aug 13 21:21:27 2001
+++ S9-pre3-kern_mount/fs/super.c	Mon Aug 13 21:21:28 2001
@@ -388,8 +388,6 @@
 	spin_lock(&dcache_lock);
 	list_add(&mnt->mnt_list, vfsmntlist.prev);
 	spin_unlock(&dcache_lock);
-	if (sb->s_type->fs_flags & FS_SINGLE)
-		get_filesystem(sb->s_type);
 out:
 	return mnt;
 }
@@ -436,8 +434,6 @@
 	list_add(&mnt->mnt_list, vfsmntlist.prev);
 	spin_unlock(&dcache_lock);
 	up(&nd->dentry->d_inode->i_zombie);
-	if (mnt->mnt_sb->s_type->fs_flags & FS_SINGLE)
-		get_filesystem(mnt->mnt_sb->s_type);
 	mntget(mnt);
 	return 0;
 fail:
@@ -1070,6 +1066,7 @@
 		if (!fs_type->read_super(s, data, 0))
 			goto out_fail;
 		unlock_super(s);
+		get_filesystem(fs_type);
 		return s;
 
 	out_fail:
@@ -1184,21 +1181,17 @@
 {
 	struct super_block *sb;
 	struct vfsmount *mnt = alloc_vfsmnt();
-	kdev_t dev;
 
 	if (!mnt)
 		return ERR_PTR(-ENOMEM);
 
-	dev = get_unnamed_dev();
-	if (!dev) {
+	if (type->fs_flags & FS_SINGLE)
+		sb = get_sb_single(type, 0, NULL);
+	else
+		sb = get_sb_nodev(type, 0, NULL);
+	if (IS_ERR(sb)) {
 		kmem_cache_free(mnt_cache, mnt);
-		return ERR_PTR(-EMFILE);
-	}
-	sb = read_super(dev, NULL, type, 0, NULL, 0);
-	if (!sb) {
-		put_unnamed_dev(dev);
-		kmem_cache_free(mnt_cache, mnt);
-		return ERR_PTR(-EINVAL);
+		return (struct vfsmount *)sb;
 	}
 	mnt->mnt_sb = sb;
 	mnt->mnt_root = dget(sb->s_root);
@@ -1206,6 +1199,7 @@
 	mnt->mnt_parent = mnt;
 	type->kern_mnt = mnt;
 	up_write(&sb->s_umount);
+	up(&mount_sem);
 	return mnt;
 }
 
@@ -1257,8 +1251,6 @@
 			spin_unlock(&dcache_lock);
 			return -EBUSY;
 		}
-		if (sb->s_type->fs_flags & FS_SINGLE)
-			put_filesystem(sb->s_type);
 		detach_mnt(mnt, &parent_nd);
 		list_del(&mnt->mnt_list);
 		spin_unlock(&dcache_lock);
diff -urN S9-pre3-get_sb_single/mm/shmem.c S9-pre3-kern_mount/mm/shmem.c
--- S9-pre3-get_sb_single/mm/shmem.c	Sat Aug 11 14:59:25 2001
+++ S9-pre3-kern_mount/mm/shmem.c	Mon Aug 13 21:21:28 2001
@@ -1158,6 +1158,7 @@
 #else
 static DECLARE_FSTYPE(tmpfs_fs_type, "tmpfs", shmem_read_super, FS_LITTER|FS_NOMOUNT);
 #endif
+static struct vfsmount *shm_mnt;
 
 static int __init init_shmem_fs(void)
 {
@@ -1181,6 +1182,7 @@
 		unregister_filesystem(&tmpfs_fs_type);
 		return PTR_ERR(res);
 	}
+	shm_mnt = res;
 
 	/* The internal instance should not do size checking */
 	if ((error = shmem_set_size(&res->mnt_sb->u.shmem_sb, ULONG_MAX, ULONG_MAX)))
@@ -1195,6 +1197,7 @@
 	unregister_filesystem(&shmem_fs_type);
 #endif
 	unregister_filesystem(&tmpfs_fs_type);
+	mntput(shm_mnt);
 }
 
 module_init(init_shmem_fs)
@@ -1292,7 +1295,7 @@
 	this.name = name;
 	this.len = strlen(name);
 	this.hash = 0; /* will go */
-	root = tmpfs_fs_type.kern_mnt->mnt_root;
+	root = shm_mnt->mnt_root;
 	dentry = d_alloc(root, &this);
 	if (!dentry)
 		return ERR_PTR(-ENOMEM);
@@ -1310,7 +1313,7 @@
 	d_instantiate(dentry, inode);
 	dentry->d_inode->i_size = size;
 	shmem_truncate(inode);
-	file->f_vfsmnt = mntget(tmpfs_fs_type.kern_mnt);
+	file->f_vfsmnt = mntget(shm_mnt);
 	file->f_dentry = dentry;
 	file->f_op = &shmem_file_operations;
 	file->f_mode = FMODE_WRITE | FMODE_READ;
diff -urN S9-pre3-get_sb_single/net/socket.c S9-pre3-kern_mount/net/socket.c
--- S9-pre3-get_sb_single/net/socket.c	Sun Jul 29 01:54:49 2001
+++ S9-pre3-kern_mount/net/socket.c	Mon Aug 13 21:21:28 2001
@@ -303,8 +303,7 @@
 }
 
 static struct vfsmount *sock_mnt;
-static DECLARE_FSTYPE(sock_fs_type, "sockfs", sockfs_read_super,
-	FS_NOMOUNT|FS_SINGLE);
+static DECLARE_FSTYPE(sock_fs_type, "sockfs", sockfs_read_super, FS_NOMOUNT);
 static int sockfs_delete_dentry(struct dentry *dentry)
 {
 	return 1;



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

* [PATCH] (8/11) fs/super.c fixes
  2001-08-14  1:34           ` [PATCH] (7/11) " Alexander Viro
@ 2001-08-14  1:34             ` Alexander Viro
  2001-08-14  1:34               ` [PATCH] (9/11) " Alexander Viro
  0 siblings, 1 reply; 16+ messages in thread
From: Alexander Viro @ 2001-08-14  1:34 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel


Part 8/11

Cleanup: ->kern_mnt is gone. Nothing uses it anymore.

diff -urN S9-pre3-kern_mount/fs/super.c S9-pre3-kern_mnt/fs/super.c
--- S9-pre3-kern_mount/fs/super.c	Mon Aug 13 21:21:28 2001
+++ S9-pre3-kern_mnt/fs/super.c	Mon Aug 13 21:21:28 2001
@@ -1197,7 +1197,6 @@
 	mnt->mnt_root = dget(sb->s_root);
 	mnt->mnt_mountpoint = mnt->mnt_root;
 	mnt->mnt_parent = mnt;
-	type->kern_mnt = mnt;
 	up_write(&sb->s_umount);
 	up(&mount_sem);
 	return mnt;
diff -urN S9-pre3-kern_mount/include/linux/fs.h S9-pre3-kern_mnt/include/linux/fs.h
--- S9-pre3-kern_mount/include/linux/fs.h	Mon Aug 13 21:21:27 2001
+++ S9-pre3-kern_mnt/include/linux/fs.h	Mon Aug 13 21:21:28 2001
@@ -89,12 +89,7 @@
 #define FS_NO_PRELIM	4 /* prevent preloading of dentries, even if
 			   * FS_NO_DCACHE is not set.
 			   */
-#define FS_SINGLE	8 /*
-			   * Filesystem that can have only one superblock;
-			   * kernel-wide vfsmnt is placed in ->kern_mnt by
-			   * kern_mount() which must be called _after_
-			   * register_filesystem().
-			   */
+#define FS_SINGLE	8 /* Filesystem that can have only one superblock */
 #define FS_NOMOUNT	16 /* Never mount from userland */
 #define FS_LITTER	32 /* Keeps the tree in dcache */
 #define FS_ODD_RENAME	32768	/* Temporary stuff; will go away as soon
@@ -914,7 +909,6 @@
 	int fs_flags;
 	struct super_block *(*read_super) (struct super_block *, void *, int);
 	struct module *owner;
-	struct vfsmount *kern_mnt; /* For kernel mount, if it's FS_SINGLE fs */
 	struct file_system_type * next;
 	struct list_head fs_supers;
 };



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

* [PATCH] (9/11) fs/super.c fixes
  2001-08-14  1:34             ` [PATCH] (8/11) " Alexander Viro
@ 2001-08-14  1:34               ` Alexander Viro
  2001-08-14  1:35                 ` [PATCH] (10/11) " Alexander Viro
  0 siblings, 1 reply; 16+ messages in thread
From: Alexander Viro @ 2001-08-14  1:34 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel


Part 9/11

Cleanup: invalidate_dquot() can rely on the fact that struct superblock is
not reused.

diff -urN S9-pre3-kern_mnt/fs/dquot.c S9-pre3-dquot/fs/dquot.c
--- S9-pre3-kern_mnt/fs/dquot.c	Sat Aug 11 14:59:24 2001
+++ S9-pre3-dquot/fs/dquot.c	Mon Aug 13 21:21:28 2001
@@ -325,7 +325,7 @@
         memset(&dquot->dq_dqb, 0, sizeof(struct dqblk));
 }
 
-static void invalidate_dquots(kdev_t dev, short type)
+static void invalidate_dquots(struct super_block *sb, short type)
 {
 	struct dquot *dquot, *next;
 	int need_restart;
@@ -335,12 +335,10 @@
 	need_restart = 0;
 	while ((dquot = next) != NULL) {
 		next = dquot->dq_next;
-		if (dquot->dq_dev != dev)
+		if (dquot->dq_sb != sb)
 			continue;
 		if (dquot->dq_type != type)
 			continue;
-		if (!dquot->dq_sb)	/* Already invalidated entry? */
-			continue;
 		if (dquot->dq_flags & DQ_LOCKED) {
 			__wait_on_dquot(dquot);
 
@@ -349,12 +347,10 @@
 			/*
 			 * Make sure it's still the same dquot.
 			 */
-			if (dquot->dq_dev != dev)
+			if (dquot->dq_sb != sb)
 				continue;
 			if (dquot->dq_type != type)
 				continue;
-			if (!dquot->dq_sb)
-				continue;
 		}
 		/*
 		 *  Because inodes needn't to be the only holders of dquot
@@ -1409,7 +1405,7 @@
 
 		/* Note: these are blocking operations */
 		remove_dquot_ref(sb, cnt);
-		invalidate_dquots(sb->s_dev, cnt);
+		invalidate_dquots(sb, cnt);
 
 		/* Wait for any pending IO - remove me as soon as invalidate is more polite */
 		down(&dqopt->dqio_sem);



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

* [PATCH] (10/11) fs/super.c fixes
  2001-08-14  1:34               ` [PATCH] (9/11) " Alexander Viro
@ 2001-08-14  1:35                 ` Alexander Viro
  2001-08-14  1:35                   ` [PATCH] (11/11) " Alexander Viro
  0 siblings, 1 reply; 16+ messages in thread
From: Alexander Viro @ 2001-08-14  1:35 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel


Part 10/11

Long-promised cleanup: lock_super(sb);unlock_super(sb); is gone from
get_super(). ->s_umount gives all needed exclusion.

And here comes the rest of ->s_count story: we lump all permanent references
together, but now we count them as 1+BIGNUM. Now conversion of temporary
reference to permanent one can recognize dying superblocks by s_count alone.
We don't have to rely on mount_sem for that. Check grab_super() and the very
beginning of kill_super() for details - it's quite straightforward. That's
where we need atomicity of s_active, BTW.

diff -urN S9-pre3-dquot/fs/super.c S9-pre3-s_count/fs/super.c
--- S9-pre3-dquot/fs/super.c	Mon Aug 13 21:21:28 2001
+++ S9-pre3-s_count/fs/super.c	Mon Aug 13 21:21:29 2001
@@ -732,10 +732,6 @@
 	s = find_super(dev);
 	if (s) {
 		spin_unlock(&sb_lock);
-		/* Yes, it sucks. As soon as we get refcounting... */
-		/* Almost there - next two lines will go away RSN */
-		lock_super(s);
-		unlock_super(s);
 		down_read(&s->s_umount);
 		if (s->s_root)
 			return s;
@@ -818,6 +814,7 @@
 	spin_lock(&sb_lock);
 	list_add (&s->s_list, super_blocks.prev);
 	list_add (&s->s_instances, &type->fs_supers);
+	s->s_count += S_BIAS;
 	spin_unlock(&sb_lock);
 	down_write(&s->s_umount);
 	lock_super(s);
@@ -839,6 +836,7 @@
 	spin_lock(&sb_lock);
 	list_del(&s->s_list);
 	list_del(&s->s_instances);
+	s->s_count -= S_BIAS;
 	spin_unlock(&sb_lock);
 	put_super(s);
 	return NULL;
@@ -879,13 +877,13 @@
 	spin_unlock(&sb_lock);
 	down_write(&sb->s_umount);
 	if (sb->s_root) {
-		/* Still relying on mount_sem */
-		if (atomic_read(&sb->s_active) > 1) {
-			spin_lock(&sb_lock);
+		spin_lock(&sb_lock);
+		if (sb->s_count > S_BIAS) {
 			sb->s_count--;
 			spin_unlock(&sb_lock);
 			return 1;
 		}
+		spin_unlock(&sb_lock);
 	}
 	atomic_dec(&sb->s_active);
 	put_super(sb);
@@ -970,6 +968,7 @@
 	s->s_type = fs_type;
 	list_add (&s->s_list, super_blocks.prev);
 	list_add (&s->s_instances, &fs_type->fs_supers);
+	s->s_count += S_BIAS;
 
 	spin_unlock(&sb_lock);
 
@@ -993,6 +992,7 @@
 	spin_lock(&sb_lock);
 	list_del(&s->s_list);
 	list_del(&s->s_instances);
+	s->s_count -= S_BIAS;
 	spin_unlock(&sb_lock);
 	put_super(s);
 out1:
@@ -1061,6 +1061,7 @@
 		s->s_type = fs_type;
 		list_add (&s->s_list, super_blocks.prev);
 		list_add (&s->s_instances, &fs_type->fs_supers);
+		s->s_count += S_BIAS;
 		spin_unlock(&sb_lock);
 		lock_super(s);
 		if (!fs_type->read_super(s, data, 0))
@@ -1078,6 +1079,7 @@
 		spin_lock(&sb_lock);
 		list_del(&s->s_list);
 		list_del(&s->s_instances);
+		s->s_count -= S_BIAS;
 		spin_unlock(&sb_lock);
 		put_super(s);
 		put_unnamed_dev(dev);
@@ -1094,8 +1096,12 @@
 	struct file_system_type *fs = sb->s_type;
 	struct super_operations *sop = sb->s_op;
 
-	if (!atomic_dec_and_test(&sb->s_active))
+	if (!atomic_dec_and_lock(&sb->s_active, &sb_lock))
 		return;
+
+	sb->s_count -= S_BIAS;
+	spin_unlock(&sb_lock);
+
 	down_write(&sb->s_umount);
 	lock_kernel();
 	sb->s_root = NULL;
diff -urN S9-pre3-dquot/include/linux/fs.h S9-pre3-s_count/include/linux/fs.h
--- S9-pre3-dquot/include/linux/fs.h	Mon Aug 13 21:21:28 2001
+++ S9-pre3-s_count/include/linux/fs.h	Mon Aug 13 21:21:29 2001
@@ -660,6 +660,7 @@
 extern spinlock_t sb_lock;
 
 #define sb_entry(list)	list_entry((list), struct super_block, s_list)
+#define S_BIAS (1<<30)
 struct super_block {
 	struct list_head	s_list;		/* Keep this first */
 	kdev_t			s_dev;



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

* [PATCH] (11/11) fs/super.c fixes
  2001-08-14  1:35                 ` [PATCH] (10/11) " Alexander Viro
@ 2001-08-14  1:35                   ` Alexander Viro
  0 siblings, 0 replies; 16+ messages in thread
From: Alexander Viro @ 2001-08-14  1:35 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel


Part 11/11

Cleanup: we move decrementing ->s_active into put_super(). Callers updated.

diff -urN S9-pre3-s_count/fs/super.c S9-pre3-put_super/fs/super.c
--- S9-pre3-s_count/fs/super.c	Mon Aug 13 21:21:29 2001
+++ S9-pre3-put_super/fs/super.c	Mon Aug 13 21:21:29 2001
@@ -666,6 +666,7 @@
 
 static void put_super(struct super_block *sb)
 {
+	atomic_dec(&sb->s_active);
 	up_write(&sb->s_umount);
 	__put_super(sb);
 }
@@ -832,7 +833,6 @@
 	s->s_bdev = 0;
 	s->s_type = NULL;
 	unlock_super(s);
-	atomic_dec(&s->s_active);
 	spin_lock(&sb_lock);
 	list_del(&s->s_list);
 	list_del(&s->s_instances);
@@ -885,7 +885,6 @@
 		}
 		spin_unlock(&sb_lock);
 	}
-	atomic_dec(&sb->s_active);
 	put_super(sb);
 	return 0;
 }
@@ -950,13 +949,11 @@
 		if (old->s_type != fs_type ||
 		    ((flags ^ old->s_flags) & MS_RDONLY)) {
 			spin_unlock(&sb_lock);
-			atomic_dec(&s->s_active);
 			put_super(s);
 			goto out1;
 		}
 		if (!grab_super(old))
 			goto restart;
-		atomic_dec(&s->s_active);
 		put_super(s);
 		blkdev_put(bdev, BDEV_FS);
 		path_release(&nd);
@@ -988,7 +985,6 @@
 	s->s_bdev = 0;
 	s->s_type = NULL;
 	unlock_super(s);
-	atomic_dec(&s->s_active);
 	spin_lock(&sb_lock);
 	list_del(&s->s_list);
 	list_del(&s->s_instances);
@@ -1044,14 +1040,12 @@
 				s_instances);
 		if (!grab_super(old))
 			goto retry;
-		atomic_dec(&s->s_active);
 		put_super(s);
 		do_remount_sb(old, flags, data);
 		return old;
 	} else {
 		kdev_t dev = get_unnamed_dev();
 		if (!dev) {
-			atomic_dec(&s->s_active);
 			put_super(s);
 			up(&mount_sem);
 			return ERR_PTR(-EMFILE);
@@ -1075,7 +1069,6 @@
 		s->s_bdev = 0;
 		s->s_type = NULL;
 		unlock_super(s);
-		atomic_dec(&s->s_active);
 		spin_lock(&sb_lock);
 		list_del(&s->s_list);
 		list_del(&s->s_instances);
@@ -1142,6 +1135,7 @@
 	list_del(&sb->s_list);
 	list_del(&sb->s_instances);
 	spin_unlock(&sb_lock);
+	atomic_inc(&sb->s_active);
 	put_super(sb);
 }
 



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

* Re: [PATCH] (1/11) fs/super.c fixes
  2001-08-14  1:31 [PATCH] (1/11) fs/super.c fixes Alexander Viro
  2001-08-14  1:32 ` [PATCH] (2/11) " Alexander Viro
@ 2001-08-14  3:56 ` Linus Torvalds
  2001-08-14  4:11   ` Alexander Viro
  1 sibling, 1 reply; 16+ messages in thread
From: Linus Torvalds @ 2001-08-14  3:56 UTC (permalink / raw)
  To: Alexander Viro; +Cc: linux-kernel


On Mon, 13 Aug 2001, Alexander Viro wrote:
>
> 	Linus, I'm resending the second series of superblock handling
> fixes.

Please verify that the patches apply. They don't. Re-sending will not
help, as long as the patches do not actually apply in series.

With these patches, as with the previous batch, the result is:

	patching file fs/super.c
	patching file fs/super.c
	patching file fs/super.c
	Hunk #1 succeeded at 669 (offset 3 lines).
	Hunk #2 succeeded at 834 with fuzz 1 (offset 1 line).
	Hunk #3 succeeded at 886 with fuzz 2 (offset 1 line).
	Hunk #4 FAILED at 950.
	Hunk #5 FAILED at 986.
	Hunk #6 FAILED at 1041.
	Hunk #7 FAILED at 1070.
	Hunk #8 succeeded at 1050 with fuzz 2 (offset -85 lines).
	4 out of 8 hunks FAILED -- saving rejects to file fs/super.c.rej
	... more failures ..

ie serious failures starting with 3/11.

		Linus



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

* Re: [PATCH] (1/11) fs/super.c fixes
  2001-08-14  3:56 ` [PATCH] (1/11) " Linus Torvalds
@ 2001-08-14  4:11   ` Alexander Viro
  2001-08-14  4:13     ` Linus Torvalds
  0 siblings, 1 reply; 16+ messages in thread
From: Alexander Viro @ 2001-08-14  4:11 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel



On Mon, 13 Aug 2001, Linus Torvalds wrote:

> 
> On Mon, 13 Aug 2001, Alexander Viro wrote:
> >
> > 	Linus, I'm resending the second series of superblock handling
> > fixes.
> 
> Please verify that the patches apply. They don't. Re-sending will not
> help, as long as the patches do not actually apply in series.

> With these patches, as with the previous batch, the result is:
> 
> 	patching file fs/super.c
> 	patching file fs/super.c
> 	patching file fs/super.c
> 	Hunk #1 succeeded at 669 (offset 3 lines).
> 	Hunk #2 succeeded at 834 with fuzz 1 (offset 1 line).
> 	Hunk #3 succeeded at 886 with fuzz 2 (offset 1 line).
> 	Hunk #4 FAILED at 950.
> 	Hunk #5 FAILED at 986.
> 	Hunk #6 FAILED at 1041.
> 	Hunk #7 FAILED at 1070.
> 	Hunk #8 succeeded at 1050 with fuzz 2 (offset -85 lines).
> 	4 out of 8 hunks FAILED -- saving rejects to file fs/super.c.rej
> 	... more failures ..
> 
> ie serious failures starting with 3/11.

Oh, hell... Looks like I'm in for downloading the tarball over 56K link ;-/
Just in case - md5 of fs/super.c (2.4.9-pre3) here is
3e98e0cc929aebcb186698eae026a0b1.  If it differs from your tree...

_Ouch_.


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

* Re: [PATCH] (1/11) fs/super.c fixes
  2001-08-14  4:11   ` Alexander Viro
@ 2001-08-14  4:13     ` Linus Torvalds
  2001-08-14  4:29       ` Alexander Viro
  0 siblings, 1 reply; 16+ messages in thread
From: Linus Torvalds @ 2001-08-14  4:13 UTC (permalink / raw)
  To: Alexander Viro; +Cc: linux-kernel


On Tue, 14 Aug 2001, Alexander Viro wrote:
> >
> > ie serious failures starting with 3/11.
>
> Oh, hell... Looks like I'm in for downloading the tarball over 56K link ;-/
> Just in case - md5 of fs/super.c (2.4.9-pre3) here is
> 3e98e0cc929aebcb186698eae026a0b1.  If it differs from your tree...

Nope, thats' the right one. You seem to have a pristine tree.

	3e98e0cc929aebcb186698eae026a0b1  fs/super.c

I suspect that you may have generated the diffs in a different order than
the subject lines imply (ie maybe 3/11 should be 4/11 and vice versa).

		Linus


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

* Re: [PATCH] (1/11) fs/super.c fixes
  2001-08-14  4:13     ` Linus Torvalds
@ 2001-08-14  4:29       ` Alexander Viro
  2001-08-14  4:35         ` Alexander Viro
  0 siblings, 1 reply; 16+ messages in thread
From: Alexander Viro @ 2001-08-14  4:29 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel



On Mon, 13 Aug 2001, Linus Torvalds wrote:

> I suspect that you may have generated the diffs in a different order than
> the subject lines imply (ie maybe 3/11 should be 4/11 and vice versa).

Damn. No, it's even dumber - cat desc-$i B$i*-S9-pre3 > $(($i+1) doesn't do
anything good when $i is 1 and we have more than 10 chunks.

IOW, 2/11 got patch from 11/11 added to its tail. Sorry. Correct (== truncated)
variant follows - the rest is OK (checked, applies as posted). If you want
me to resend them too - tell and I'll do that.

Part 2/11

First part of get_sb_bdev() rewrite. We move opening the device to the
beginning of the function. If we already have a superblock from that device
- well, no problem. That, BTW, fixes a buglet with permissions: suppose
we mount /dev/foo, chmod it to 0 and say mount /dev/foo again. Old code
merrily didn't notice that permissions had been revoked and allowed mount.
Main reason for the change is different, though - we are getting the
blocking operations from the area we want to protect with sb_lock (see
the next chunk).


Cleanup: we move decrementing ->s_active into put_super(). Callers updated.

diff -urN S9-pre3-s_umount/fs/super.c S9-pre3-get_sb_bdev/fs/super.c
--- S9-pre3-s_umount/fs/super.c	Mon Aug 13 21:21:26 2001
+++ S9-pre3-get_sb_bdev/fs/super.c	Mon Aug 13 21:21:26 2001
@@ -872,6 +872,26 @@
 			kdevname(dev));
 }
 
+static int grab_super(struct super_block *sb)
+{
+	sb->s_count++;
+	atomic_inc(&sb->s_active);
+	spin_unlock(&sb_lock);
+	down_write(&sb->s_umount);
+	if (sb->s_root) {
+		/* Still relying on mount_sem */
+		if (atomic_read(&sb->s_active) > 1) {
+			spin_lock(&sb_lock);
+			sb->s_count--;
+			spin_unlock(&sb_lock);
+			return 1;
+		}
+	}
+	atomic_dec(&sb->s_active);
+	put_super(sb);
+	return 0;
+}
+
 static struct super_block *get_sb_bdev(struct file_system_type *fs_type,
 	char *dev_name, int flags, void * data)
 {
@@ -880,8 +900,11 @@
 	struct block_device_operations *bdops;
 	struct super_block * sb;
 	struct nameidata nd;
+	struct list_head *p;
 	kdev_t dev;
 	int error = 0;
+	mode_t mode = FMODE_READ; /* we always need it ;-) */
+
 	/* What device it is? */
 	if (!dev_name || !*dev_name)
 		return ERR_PTR(-EINVAL);
@@ -902,52 +925,45 @@
 	/* Done with lookups, semaphore down */
 	down(&mount_sem);
 	dev = to_kdev_t(bdev->bd_dev);
-	sb = get_super(dev);
-	if (sb) {
-		if (fs_type == sb->s_type &&
-		    ((flags ^ sb->s_flags) & MS_RDONLY) == 0) {
-/*
- * We are heavily relying on mount_sem here. We _will_ get rid of that
- * ugliness RSN (and then atomicity of ->s_active will play), but first
- * we need to get rid of "reuse" branch of get_empty_super() and that
- * requires reference counters. Chicken and egg problem, but fortunately
- * we can use the fact that right now all accesses to ->s_active are
- * under mount_sem.
- */
-			if (atomic_read(&sb->s_active)) {
-				spin_lock(&sb_lock);
-				sb->s_count--;
-				spin_unlock(&sb_lock);
-			}
-			atomic_inc(&sb->s_active);
-			/* Next chunk will drop it */
-			up_read(&sb->s_umount);
-			down_write(&sb->s_umount);
-			path_release(&nd);
-			return sb;
-		}
-		drop_super(sb);
-	} else {
-		mode_t mode = FMODE_READ; /* we always need it ;-) */
-		if (!(flags & MS_RDONLY))
-			mode |= FMODE_WRITE;
-		error = blkdev_get(bdev, mode, 0, BDEV_FS);
-		if (error)
-			goto out;
-		check_disk_change(dev);
-		error = -EACCES;
-		if (!(flags & MS_RDONLY) && is_read_only(dev))
+	if (!(flags & MS_RDONLY))
+		mode |= FMODE_WRITE;
+	error = blkdev_get(bdev, mode, 0, BDEV_FS);
+	if (error)
+		goto out;
+	check_disk_change(dev);
+	error = -EACCES;
+	if (!(flags & MS_RDONLY) && is_read_only(dev))
+		goto out1;
+
+	error = -EBUSY;
+restart:
+	spin_lock(&sb_lock);
+
+	list_for_each(p, &super_blocks) {
+		struct super_block *old = sb_entry(p);
+		if (old->s_dev != dev)
+			continue;
+		if (old->s_type != fs_type ||
+		    ((flags ^ old->s_flags) & MS_RDONLY)) {
+			spin_unlock(&sb_lock);
 			goto out1;
-		error = -EINVAL;
-		sb = read_super(dev, bdev, fs_type, flags, data, 0);
-		if (sb) {
-			get_filesystem(fs_type);
-			path_release(&nd);
-			return sb;
 		}
-out1:
+		if (!grab_super(old))
+			goto restart;
 		blkdev_put(bdev, BDEV_FS);
+		path_release(&nd);
+		return old;
 	}
+	spin_unlock(&sb_lock);
+	error = -EINVAL;
+	sb = read_super(dev, bdev, fs_type, flags, data, 0);
+	if (sb) {
+		get_filesystem(fs_type);
+		path_release(&nd);
+		return sb;
+	}
+out1:
+	blkdev_put(bdev, BDEV_FS);
 out:
 	path_release(&nd);
 	up(&mount_sem);


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

* Re: [PATCH] (1/11) fs/super.c fixes
  2001-08-14  4:29       ` Alexander Viro
@ 2001-08-14  4:35         ` Alexander Viro
  0 siblings, 0 replies; 16+ messages in thread
From: Alexander Viro @ 2001-08-14  4:35 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel



On Tue, 14 Aug 2001, Alexander Viro wrote:
 
> Cleanup: we move decrementing ->s_active into put_super(). Callers updated.

... and that line in description is also bogus, indeed (same reasons -
description from 11/11). Self-LART applied...


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

end of thread, other threads:[~2001-08-14  4:35 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2001-08-14  1:31 [PATCH] (1/11) fs/super.c fixes Alexander Viro
2001-08-14  1:32 ` [PATCH] (2/11) " Alexander Viro
2001-08-14  1:32   ` [PATCH] (3/11) " Alexander Viro
2001-08-14  1:33     ` [PATCH] (4/11) " Alexander Viro
2001-08-14  1:33       ` [PATCH] (5/11) " Alexander Viro
2001-08-14  1:33         ` [PATCH] (6/11) " Alexander Viro
2001-08-14  1:34           ` [PATCH] (7/11) " Alexander Viro
2001-08-14  1:34             ` [PATCH] (8/11) " Alexander Viro
2001-08-14  1:34               ` [PATCH] (9/11) " Alexander Viro
2001-08-14  1:35                 ` [PATCH] (10/11) " Alexander Viro
2001-08-14  1:35                   ` [PATCH] (11/11) " Alexander Viro
2001-08-14  3:56 ` [PATCH] (1/11) " Linus Torvalds
2001-08-14  4:11   ` Alexander Viro
2001-08-14  4:13     ` Linus Torvalds
2001-08-14  4:29       ` Alexander Viro
2001-08-14  4:35         ` Alexander Viro

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).