[Cluster-devel] [GFS Patch] GFS: Don't free superblock until last kobject user is done

[Bob Peterson]

Hi,

This patch rearranges the sysfs interface to more closely match
GFS2, plus it adds an object release mechanism, as described in
kernel/Documentation/kobject.txt

rhbz#788694

Regards,

Bob Peterson
Red Hat File Systems

Signed-off-by: Bob Peterson <rpeterso at redhat.com> 
---
 gfs-kernel/src/gfs/ops_fstype.c |   27 +++++++++++++++++++--------
 gfs-kernel/src/gfs/ops_fstype.h |    1 -
 gfs-kernel/src/gfs/ops_super.c  |    6 +-----
 gfs-kernel/src/gfs/sys.c        |   14 ++++++++++++++
 4 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/gfs-kernel/src/gfs/ops_fstype.c b/gfs-kernel/src/gfs/ops_fstype.c
index 34d9fa4..7104a2d 100644
--- a/gfs-kernel/src/gfs/ops_fstype.c
+++ b/gfs-kernel/src/gfs/ops_fstype.c
@@ -682,12 +682,24 @@ static int fill_super(struct super_block *sb, void *data, int silent)
 		goto fail;
 	}
 	error = init_names(sdp, silent);
-	if (error)
-		goto fail;
+	if (error) {
+		/* In this case, we haven't initialized sysfs, so we have to
+		   manually free the sdp. */
+		vfree(sdp);
+		sb->s_fs_info = NULL;
+		return error;
+	}
 
 	error = gfs_sys_fs_add(sdp);
+	/*
+	 * If we hit an error here, gfs2_sys_fs_add will have called function
+	 * kobject_put which causes the sysfs usage count to go to zero, which
+	 * causes sysfs to call function gfs2_sbd_release, which frees sdp.
+	 * Subsequent error paths here will call gfs_sys_fs_del, which also
+	 * kobject_put to free sdp.
+	 */
 	if (error)
-		goto fail;
+		return error;
 
 	/*  Mount an inter-node lock module, check for local optimizations */
 
@@ -746,17 +758,16 @@ fail_locking:
 fail_lm:
 	gfs_gl_hash_clear(sdp, TRUE);
 	gfs_lm_unmount(sdp);
+fail_sys:
 	gfs_clear_dirty_j(sdp);
 	while (invalidate_inodes(sb))
 		yield();
 
-fail_sys:
-	gfs_sys_fs_del(sdp);
-
 fail:
-	vfree(sdp);
+	/* gfs_sys_fs_del must be the last thing we do, since it causes
+	 * sysfs to call function gfs2_sbd_release, which frees sdp. */
+	gfs_sys_fs_del(sdp);
 	sb->s_fs_info = NULL;
-
 	return error;
 }
 
diff --git a/gfs-kernel/src/gfs/ops_fstype.h b/gfs-kernel/src/gfs/ops_fstype.h
index 8f6f0f7..d31225e 100644
--- a/gfs-kernel/src/gfs/ops_fstype.h
+++ b/gfs-kernel/src/gfs/ops_fstype.h
@@ -16,7 +16,6 @@
 
 int gfs_sys_init(void);
 void gfs_sys_uninit(void);
-void gfs_sys_fs_del(struct gfs_sbd *sdp);
 int gfs_test_bdev_super(struct super_block *sb, void *data);
 int gfs_set_bdev_super(struct super_block *sb, void *data);
 int init_names(struct gfs_sbd *sdp, int silent);
diff --git a/gfs-kernel/src/gfs/ops_super.c b/gfs-kernel/src/gfs/ops_super.c
index 1fa7faf..c111a2e 100644
--- a/gfs-kernel/src/gfs/ops_super.c
+++ b/gfs-kernel/src/gfs/ops_super.c
@@ -167,17 +167,13 @@ gfs_put_super(struct super_block *sb)
 
 	/*  At this point, we're through participating in the lockspace  */
 
-	gfs_sys_fs_del(sdp);
-
 	gfs_clear_dirty_j(sdp);
 
 	/*  Get rid of any extra inodes  */
 	while (invalidate_inodes(sb))
 		yield();
 
-	vfree(sdp);
-
-	set_v2sdp(sb, NULL);
+	gfs_sys_fs_del(sdp);
 }
 
 /**
diff --git a/gfs-kernel/src/gfs/sys.c b/gfs-kernel/src/gfs/sys.c
index 81fe82b..3079cae 100644
--- a/gfs-kernel/src/gfs/sys.c
+++ b/gfs-kernel/src/gfs/sys.c
@@ -20,6 +20,7 @@
 #include <linux/buffer_head.h>
 #include <linux/proc_fs.h>
 #include <linux/module.h>
+#include <linux/vmalloc.h>
 #include <asm/uaccess.h>
 
 #include "gfs.h"
@@ -75,12 +76,20 @@ static ssize_t gfs_attr_store(struct kobject *kobj, struct attribute *attr,
 	return a->store ? a->store(sdp, buf, len) : len;
 }
 
+static void gfs_sbd_release(struct kobject *kobj)
+{
+       struct gfs_sbd *sdp = container_of(kobj, struct gfs_sbd, sd_kobj);
+
+       vfree(sdp);
+}
+
 static struct sysfs_ops gfs_attr_ops = {
 	.show  = gfs_attr_show,
 	.store = gfs_attr_store,
 };
 
 static struct kobj_type gfs_ktype = {
+	.release = gfs_sbd_release,
 	.default_attrs = gfs_attrs,
 	.sysfs_ops     = &gfs_attr_ops,
 };
@@ -93,6 +102,7 @@ static struct kset gfs_kset = {
 
 int gfs_sys_fs_add(struct gfs_sbd *sdp)
 {
+	struct super_block *sb;
 	int error;
 
 	sdp->sd_kobj.kset = &gfs_kset;
@@ -109,6 +119,10 @@ int gfs_sys_fs_add(struct gfs_sbd *sdp)
 	return 0;
 
  fail:
+	printk("GFS: error %d adding sysfs files", error);
+	sb = sdp->sd_vfs;
+	vfree(sdp);
+	sb->s_fs_info = NULL;
 	return error;
 }