Re: [PATCH] kernel/params.c: print failure information instead of 'KOBJ_ADD' to user space, when sysfs_create_file() fails.

[Chen Gang <gang.chen@asianux.com>]

On 07/10/2013 10:17 AM, Chen Gang F T wrote:
> On 07/09/2013 04:07 PM, Rusty Russell wrote:
>> Chen Gang <gang.chen@asianux.com> writes:
>>> When sysfs_create_file() fails, recommend to print the related failure
>>> information. And it is useless to still 'KOBJ_ADD' to user space.
>>>
>>> Signed-off-by: Chen Gang <gang.chen@asianux.com>
>>
>> sysfs_create_file() should not fail during boot, should it?
>>
> 
> Hmm..., please reference locate_module_kobject() in "kernel/params.c",
> which is an '__init' function, and also call sysfs_create_file(), it
> processes the related error.
> 
> So I recommend to get the check too in version_sysfs_builtin().
> 

Oh, also for locate_module_kobject(), if !CONFIG_MODULES, when error
occurs, it still print the information about "Adding module".

Hmm..., do we need call kobject_get() before kobject_put() in failure
processing block ?


740                 mk = kzalloc(sizeof(struct module_kobject), GFP_KERNEL);
741                 BUG_ON(!mk);
742 
743                 mk->mod = THIS_MODULE;
744                 mk->kobj.kset = module_kset;
745                 err = kobject_init_and_add(&mk->kobj, &module_ktype, NULL,
746                                            "%s", name);
747 #ifdef CONFIG_MODULES
748                 if (!err)
749                         err = sysfs_create_file(&mk->kobj, &module_uevent.attr);
750 #endif
751                 if (err) {
752                         kobject_put(&mk->kobj);
753                         pr_crit("Adding module '%s' to sysfs failed (%d), the system may be unstable.\n",
754                                 name, err);
755                         return NULL;
756                 }
757 
758                 /* So that we hold reference in both cases. */
759                 kobject_get(&mk->kobj);
760         }
761 
762         return mk;
763 }


> Thanks.
> 
>> Cheers,
>> Rusty.
>>
>>> ---
>>>  kernel/params.c |    8 +++++++-
>>>  1 files changed, 7 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/kernel/params.c b/kernel/params.c
>>> index 440e65d..f5299c1 100644
>>> --- a/kernel/params.c
>>> +++ b/kernel/params.c
>>> @@ -845,7 +845,13 @@ static void __init version_sysfs_builtin(void)
>>>  		mk = locate_module_kobject(vattr->module_name);
>>>  		if (mk) {
>>>  			err = sysfs_create_file(&mk->kobj, &vattr->mattr.attr);
>>> -			kobject_uevent(&mk->kobj, KOBJ_ADD);
>>> +			if (err)
>>> +				printk(KERN_WARNING
>>> +				       "%s (%d): sysfs_create_file fail for %s, err: %d\n",
>>> +				       __FILE__, __LINE__,
>>> +				       vattr->module_name, err);
>>> +			else
>>> +				kobject_uevent(&mk->kobj, KOBJ_ADD);
>>>  			kobject_put(&mk->kobj);
>>>  		}
>>>  	}
>>> -- 
>>> 1.7.7.6
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>>
> 
> 


-- 
Chen Gang