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

2013-07-11 Thread Chen Gang 
On 07/10/2013 11:01 AM, Chen Gang F T wrote:
> 
>> > Hmm..., do we need call kobject_get() before kobject_put() in failure
>> > processing block ?
>> > 
> Oh, sorry for what I said for kobject_get/put() items above, it is
> incorrect.
> 
> What about the diff below for kobject_get() ?
> 
> ---diff begin---
> 
> diff --git a/kernel/params.c b/kernel/params.c
> index 440e65d..ef8d720 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -754,11 +754,11 @@ static struct module_kobject * __init 
> locate_module_kobject(const char *name)
>   name, err);
>   return NULL;
>   }
> -
> - /* So that we hold reference in both cases. */
> - kobject_get(>kobj);
>   }
>  
> + /* So that we hold reference in both cases. */
> + kobject_get(>kobj);
> +
>   return mk;
>  }
> 
> ---diff end-
> 

Sorry again, this diff is incorrect, the original implementation has no
issues.


> And it also need add additional kobject_put(), if we really need
> process the failure in version_sysfs_builtin().

If need process failure, we really need it, but now, we only use
BUG_ON() is enough.


Thanks.
-- 
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-07-11 Thread Chen Gang 
On 07/10/2013 11:01 AM, Chen Gang F T wrote:
 
  Hmm..., do we need call kobject_get() before kobject_put() in failure
  processing block ?
  
 Oh, sorry for what I said for kobject_get/put() items above, it is
 incorrect.
 
 What about the diff below for kobject_get() ?
 
 ---diff begin---
 
 diff --git a/kernel/params.c b/kernel/params.c
 index 440e65d..ef8d720 100644
 --- a/kernel/params.c
 +++ b/kernel/params.c
 @@ -754,11 +754,11 @@ static struct module_kobject * __init 
 locate_module_kobject(const char *name)
   name, err);
   return NULL;
   }
 -
 - /* So that we hold reference in both cases. */
 - kobject_get(mk-kobj);
   }
  
 + /* So that we hold reference in both cases. */
 + kobject_get(mk-kobj);
 +
   return mk;
  }
 
 ---diff end-
 

Sorry again, this diff is incorrect, the original implementation has no
issues.


 And it also need add additional kobject_put(), if we really need
 process the failure in version_sysfs_builtin().

If need process failure, we really need it, but now, we only use
BUG_ON() is enough.


Thanks.
-- 
Chen Gang
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-07-10 Thread Chen Gang 
On 07/11/2013 09:53 AM, Rusty Russell wrote:
> Chen Gang F T  writes:
>> > On 07/09/2013 04:07 PM, Rusty Russell wrote:
>>> >> Chen Gang  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 
>>> >> 
>>> >> 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().
> It still can't fail.  sysfs_create_file() can fail due to OOM (not at
> boot) or name duplication (not here).
> 
> You can BUG_ON() if you want.
> 

OK, thanks, I will send patch v2 for it.


> And feel free to fix locate_module_kobject() in a separate patch.

OK, thanks, I will send related patch for it.

:-)


Thanks.
-- 
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-07-10 Thread Rusty Russell 
Chen Gang F T  writes:
> On 07/09/2013 04:07 PM, Rusty Russell wrote:
>> Chen Gang  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 
>> 
>> 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().

It still can't fail.  sysfs_create_file() can fail due to OOM (not at
boot) or name duplication (not here).

You can BUG_ON() if you want.

And feel free to fix locate_module_kobject() in a separate patch.

Cheers,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-07-10 Thread Rusty Russell 
Chen Gang F T chen.gang.flying.transfor...@gmail.com writes:
 On 07/09/2013 04:07 PM, Rusty Russell wrote:
 Chen Gang gang.c...@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.c...@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().

It still can't fail.  sysfs_create_file() can fail due to OOM (not at
boot) or name duplication (not here).

You can BUG_ON() if you want.

And feel free to fix locate_module_kobject() in a separate patch.

Cheers,
Rusty.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-07-10 Thread Chen Gang 
On 07/11/2013 09:53 AM, Rusty Russell wrote:
 Chen Gang F T chen.gang.flying.transfor...@gmail.com writes:
  On 07/09/2013 04:07 PM, Rusty Russell wrote:
  Chen Gang gang.c...@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.c...@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().
 It still can't fail.  sysfs_create_file() can fail due to OOM (not at
 boot) or name duplication (not here).
 
 You can BUG_ON() if you want.
 

OK, thanks, I will send patch v2 for it.


 And feel free to fix locate_module_kobject() in a separate patch.

OK, thanks, I will send related patch for it.

:-)


Thanks.
-- 
Chen Gang
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-07-09 Thread Chen Gang F T 
On 07/10/2013 10:35 AM, Chen Gang wrote:
> On 07/10/2013 10:17 AM, Chen Gang F T wrote:
>> On 07/09/2013 04:07 PM, Rusty Russell wrote:
>>> Chen Gang  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 
>>>
>>> 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 ?
> 

Oh, sorry for what I said for kobject_get/put() items above, it is
incorrect.

What about the diff below for kobject_get() ?

---diff begin---

diff --git a/kernel/params.c b/kernel/params.c
index 440e65d..ef8d720 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -754,11 +754,11 @@ static struct module_kobject * __init 
locate_module_kobject(const char *name)
name, err);
return NULL;
}
-
-   /* So that we hold reference in both cases. */
-   kobject_get(>kobj);
}
 
+   /* So that we hold reference in both cases. */
+   kobject_get(>kobj);
+
return mk;
 }

---diff end-

And it also need add additional kobject_put(), if we really need
process the failure in version_sysfs_builtin().

Thanks.

> 
> 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(>kobj, _ktype, NULL,
> 746"%s", name);
> 747 #ifdef CONFIG_MODULES
> 748 if (!err)
> 749 err = sysfs_create_file(>kobj, 
> _uevent.attr);
> 750 #endif
> 751 if (err) {
> 752 kobject_put(>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(>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(>kobj, >mattr.attr);
 -  kobject_uevent(>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(>kobj, KOBJ_ADD);
kobject_put(>kobj);
}
}
 -- 
 1.7.7.6
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to majord...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-07-09 Thread Chen Gang 
On 07/10/2013 10:17 AM, Chen Gang F T wrote:
> On 07/09/2013 04:07 PM, Rusty Russell wrote:
>> Chen Gang  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 
>>
>> 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(>kobj, _ktype, NULL,
746"%s", name);
747 #ifdef CONFIG_MODULES
748 if (!err)
749 err = sysfs_create_file(>kobj, 
_uevent.attr);
750 #endif
751 if (err) {
752 kobject_put(>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(>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(>kobj, >mattr.attr);
>>> -   kobject_uevent(>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(>kobj, KOBJ_ADD);
>>> kobject_put(>kobj);
>>> }
>>> }
>>> -- 
>>> 1.7.7.6
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majord...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-07-09 Thread Chen Gang F T 
On 07/09/2013 04:07 PM, Rusty Russell wrote:
> Chen Gang  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 
> 
> 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().

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(>kobj, >mattr.attr);
>> -kobject_uevent(>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(>kobj, KOBJ_ADD);
>>  kobject_put(>kobj);
>>  }
>>  }
>> -- 
>> 1.7.7.6
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-07-09 Thread Rusty Russell 
Chen Gang  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 

sysfs_create_file() should not fail during boot, should it?

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(>kobj, >mattr.attr);
> - kobject_uevent(>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(>kobj, KOBJ_ADD);
>   kobject_put(>kobj);
>   }
>   }
> -- 
> 1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-07-09 Thread Rusty Russell 
Chen Gang gang.c...@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.c...@asianux.com

sysfs_create_file() should not fail during boot, should it?

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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-07-09 Thread Chen Gang F T 
On 07/09/2013 04:07 PM, Rusty Russell wrote:
 Chen Gang gang.c...@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.c...@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().

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 majord...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-07-09 Thread Chen Gang 
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.c...@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.c...@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 majord...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-07-09 Thread Chen Gang F T 
On 07/10/2013 10:35 AM, Chen Gang wrote:
 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.c...@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.c...@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 ?
 

Oh, sorry for what I said for kobject_get/put() items above, it is
incorrect.

What about the diff below for kobject_get() ?

---diff begin---

diff --git a/kernel/params.c b/kernel/params.c
index 440e65d..ef8d720 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -754,11 +754,11 @@ static struct module_kobject * __init 
locate_module_kobject(const char *name)
name, err);
return NULL;
}
-
-   /* So that we hold reference in both cases. */
-   kobject_get(mk-kobj);
}
 
+   /* So that we hold reference in both cases. */
+   kobject_get(mk-kobj);
+
return mk;
 }

---diff end-

And it also need add additional kobject_put(), if we really need
process the failure in version_sysfs_builtin().

Thanks.

 
 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 majord...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-07-08 Thread Chen Gang 
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 
---
 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(>kobj, >mattr.attr);
-   kobject_uevent(>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(>kobj, KOBJ_ADD);
kobject_put(>kobj);
}
}
-- 
1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-07-08 Thread Chen Gang 
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.c...@asianux.com
---
 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/