[lustre-devel] [PATCH 02/37] lustre: llite: don't use class_setup_tunables()

[James Simmons]

> >> llite_kobj does not benefit from being in the
> >> lustre_kset kset (though it does need lustre_kset
> >> as a parent)
> >
> > Nak.
> >
> > Yes llite does benefit from being in the kset. On large clusters you
> > can end up with 1000s of clients so keeping the sysfs files setting 
> > in sync needs to be done in mass. The way Lustre does this is by 
> > running on the MGS server 'lctl conf_param $FSNAME.llite.lazystats=1'
> > or with the newer 'lctl set_param -P llite.$FSNAME-*.lazystats=1' as
> > an example. When this is done on the MGS that new value is first
> > persistently stored and then transmitted to the target nodes.
> >   
> > The magic behind this is with class_process_config(). In the code
> > path LCFG_PARAM, which handles the conf_param case' the main function
> > is class_modify_config() which is used by both llite and the obd
> > devices. This function first attempts to find the sysfs attribute
> > from the kobject and call lustre_attr_store() directly to set it.
> > If it can't find the attribute, which means its a debugfs file
> > most likely, then a uevent is created and sent off. The udev rule
> > then exexcutes some application to handle the debugfs settings.
> > For uevents to work the kobject (llite) must belong to a kset.
> >
> > With LCFG_SET_PARAM, which handles the 'set_param -P' case the
> > master function is process_param2_config(). For this case we
> > always send attribute changes with uevents. Besides the requirement
> > of the kobject belonging to the kset this function uses 
> > kset_find_object() directly which if llite is not in the kset
> > will never be found.
> 
> Thanks a lot for the explanation - there are definitely things I missed
> there.
> I'll drop this patch, and keep class_setup_tunables() in the later patch
> that removed it.

I have a patch to replace this one. In fact I love the using the release
for cleanups for ktype that I moved debugfs handling in llite into a new
llite_kobj_release(). I'm looking at the next patch but that one is 
tricker since it impacts server code as well as I pointed out in another
email. I think we can do a reasonable cleanup here.

> >> It also does not need the class_ktype type, as dynamic_kobj_ktype
> >> is sufficient.
> >> 
> >> So use the simple kobject_create_and_add() to initialize it.
> >> 
> >> This provides flexibility for making changes to
> >> class_setup_tunables().
> >> 
> >> Signed-off-by: NeilBrown <neilb at suse.com>
> >> ---
> >>  drivers/staging/lustre/lustre/llite/lproc_llite.c |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/staging/lustre/lustre/llite/lproc_llite.c b/drivers/staging/lustre/lustre/llite/lproc_llite.c
> >> index 8215296dc15d..78ec0fa65c58 100644
> >> --- a/drivers/staging/lustre/lustre/llite/lproc_llite.c
> >> +++ b/drivers/staging/lustre/lustre/llite/lproc_llite.c
> >> @@ -46,7 +46,7 @@ int llite_tunables_register(void)
> >>  {
> >>  	int rc = 0;
> >>  
> >> -	llite_kobj = class_setup_tunables("llite");
> >> +	llite_kobj = kobject_create_and_add("llite", &lustre_kset->kobj);
> >>  	if (IS_ERR(llite_kobj))
> >>  		return PTR_ERR(llite_kobj);
> >>  
> >
> >         llite_kobj->kset = kset_get(&lustre_kset);
> >> 
> >
> > In llite_tunables_unregister() the call for kobject_put()
> > should really be replaced with kobject_del(). This is a bug
> > in my original work. 
>