* [Cluster-devel] [GFS Patch] GFS: Don't free superblock until last kobject user is done
[not found] <0538d6a2-2392-425e-b25d-626c5d33dbb6@zmail12.collab.prod.int.phx2.redhat.com>
@ 2012-06-26 13:27 ` Bob Peterson
2012-06-28 8:42 ` Steven Whitehouse
0 siblings, 1 reply; 2+ messages in thread
From: Bob Peterson @ 2012-06-26 13:27 UTC (permalink / raw)
To: cluster-devel.redhat.com
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@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;
}
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [Cluster-devel] [GFS Patch] GFS: Don't free superblock until last kobject user is done
2012-06-26 13:27 ` [Cluster-devel] [GFS Patch] GFS: Don't free superblock until last kobject user is done Bob Peterson
@ 2012-06-28 8:42 ` Steven Whitehouse
0 siblings, 0 replies; 2+ messages in thread
From: Steven Whitehouse @ 2012-06-28 8:42 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
Looks good to me,
Steve.
On Tue, 2012-06-26 at 09:27 -0400, Bob Peterson wrote:
> 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@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;
> }
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-06-28 8:42 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <0538d6a2-2392-425e-b25d-626c5d33dbb6@zmail12.collab.prod.int.phx2.redhat.com>
2012-06-26 13:27 ` [Cluster-devel] [GFS Patch] GFS: Don't free superblock until last kobject user is done Bob Peterson
2012-06-28 8:42 ` Steven Whitehouse
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).