2011-02-04 16:47:37

by dann frazier

[permalink] [raw]
Subject: [PATCH] [ocfs2] Use a kobject instead of a kset

From: dann frazier <[email protected]>

We currently allocate a kset and add attributes to its internal kobject.
However, Documentation/kobject.txt says only the kset code should manipulate
this internal object, and we can get the same results by just creating our
own kobject and avoiding a kset altogether.

Signed-off-by: dann frazier <[email protected]>
---
fs/ocfs2/stackglue.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/ocfs2/stackglue.c b/fs/ocfs2/stackglue.c
index 39abf89..7098101 100644
--- a/fs/ocfs2/stackglue.c
+++ b/fs/ocfs2/stackglue.c
@@ -607,29 +607,29 @@ static struct attribute_group ocfs2_attr_group = {
.attrs = ocfs2_attrs,
};

-static struct kset *ocfs2_kset;
+static struct kobject *ocfs2_kobj;

static void ocfs2_sysfs_exit(void)
{
- kset_unregister(ocfs2_kset);
+ kobject_put(ocfs2_kobj);
}

static int ocfs2_sysfs_init(void)
{
int ret;

- ocfs2_kset = kset_create_and_add("ocfs2", NULL, fs_kobj);
- if (!ocfs2_kset)
+ ocfs2_kobj = kobject_create_and_add("ocfs2", fs_kobj);
+ if (!ocfs2_kobj)
return -ENOMEM;

- ret = sysfs_create_group(&ocfs2_kset->kobj, &ocfs2_attr_group);
+ ret = sysfs_create_group(ocfs2_kobj, &ocfs2_attr_group);
if (ret)
goto error;

return 0;

error:
- kset_unregister(ocfs2_kset);
+ kobject_put(ocfs2_kobj);
return ret;
}

--
1.7.2.3


2011-02-04 17:58:34

by Joel Becker

[permalink] [raw]
Subject: Re: [Ocfs2-devel] [PATCH] [ocfs2] Use a kobject instead of a kset

On Fri, Feb 04, 2011 at 09:40:27AM -0700, [email protected] wrote:
> From: dann frazier <[email protected]>
>
> We currently allocate a kset and add attributes to its internal kobject.
> However, Documentation/kobject.txt says only the kset code should manipulate
> this internal object, and we can get the same results by just creating our
> own kobject and avoiding a kset altogether.

Hmm, is this actually valid? I thought you had to be a kset to
have children. Did this change at some point?

Joel

--

"Soap and education are not as sudden as a massacre, but they are more
deadly in the long run."
- Mark Twain


http://www.jlbec.org/
[email protected]

2011-02-05 18:52:25

by dann frazier

[permalink] [raw]
Subject: Re: [Ocfs2-devel] [PATCH] [ocfs2] Use a kobject instead of a kset

On Fri, Feb 04, 2011 at 09:58:01AM -0800, Joel Becker wrote:
> On Fri, Feb 04, 2011 at 09:40:27AM -0700, [email protected] wrote:
> > From: dann frazier <[email protected]>
> >
> > We currently allocate a kset and add attributes to its internal kobject.
> > However, Documentation/kobject.txt says only the kset code should manipulate
> > this internal object, and we can get the same results by just creating our
> > own kobject and avoiding a kset altogether.
>
> Hmm, is this actually valid? I thought you had to be a kset to
> have children. Did this change at some point?

hey Joel,
Well, I'm by no means an expert on kobjects - hopefully someone who
is will chime in. I came across this while debugging a sysfs teardown
issue and looking through this code to see if it might be related. I
read through kobject.txt & saw that note about a kset's kobj being for
internal-use only, and then saw that samples/kobject/kobject-example.c
seems to achieve the same goal (a sysfs dir w/ attributes) w/o using a
kset.

The sysfs entries look the same to me before & after this patch:

$ ls -l /sys/fs/ocfs2
total 0
-r--r--r-- 1 root root 4096 Feb 5 11:40 active_cluster_plugin
-rw-r--r-- 1 root root 4096 Feb 5 11:40 cluster_stack
-r--r--r-- 1 root root 4096 Feb 5 11:40 loaded_cluster_plugins
-r--r--r-- 1 root root 4096 Feb 5 11:40 max_locking_protocol

That said, I haven't been able to demonstate that the current code is
causing any actual problems - this was just a cleanup attempt.