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