linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kobject: kobject_add_internal cleanup
@ 2021-07-27 14:32 Qiao Yanbo
  2021-07-28 10:10 ` Greg KH
  0 siblings, 1 reply; 6+ messages in thread
From: Qiao Yanbo @ 2021-07-27 14:32 UTC (permalink / raw)
  To: gregkh; +Cc: rafael, linux-kernel, qiaoyanbo

From: qiaoyanbo <qiaoyanbo_310@163.com>

parent assignment in "if" block only need to consider when parent is NULL.

Signed-off-by: qiaoyanbo <qiaoyanbo_310@163.com>
---
 lib/kobject.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/kobject.c b/lib/kobject.c
index ea53b30cf..d1f4b3411 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -241,10 +241,11 @@ static int kobject_add_internal(struct kobject *kobj)
 
 	/* join kset if set, use it as parent if we do not already have one */
 	if (kobj->kset) {
-		if (!parent)
+		if (!parent) {
 			parent = kobject_get(&kobj->kset->kobj);
+			kobj->parent = parent;
+		}
 		kobj_kset_join(kobj);
-		kobj->parent = parent;
 	}
 
 	pr_debug("kobject: '%s' (%p): %s: parent: '%s', set: '%s'\n",
-- 
2.30.2



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kobject: kobject_add_internal cleanup
  2021-07-27 14:32 [PATCH] kobject: kobject_add_internal cleanup Qiao Yanbo
@ 2021-07-28 10:10 ` Greg KH
       [not found]   ` <3be7ce57.62f6.17af280a47f.Coremail.qiaoyanbo_310@163.com>
  0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2021-07-28 10:10 UTC (permalink / raw)
  To: Qiao Yanbo; +Cc: rafael, linux-kernel

On Tue, Jul 27, 2021 at 10:32:12PM +0800, Qiao Yanbo wrote:
> From: qiaoyanbo <qiaoyanbo_310@163.com>

This does not match your "From:" line in your email address, please use
your real name here and in your signed-off-by line like your email
shows.

> 
> parent assignment in "if" block only need to consider when parent is NULL.

I do not understand, why is this needed?  What does this fix?

> 
> Signed-off-by: qiaoyanbo <qiaoyanbo_310@163.com>
> ---
>  lib/kobject.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/kobject.c b/lib/kobject.c
> index ea53b30cf..d1f4b3411 100644
> --- a/lib/kobject.c
> +++ b/lib/kobject.c
> @@ -241,10 +241,11 @@ static int kobject_add_internal(struct kobject *kobj)
>  
>  	/* join kset if set, use it as parent if we do not already have one */
>  	if (kobj->kset) {
> -		if (!parent)
> +		if (!parent) {
>  			parent = kobject_get(&kobj->kset->kobj);
> +			kobj->parent = parent;
> +		}
>  		kobj_kset_join(kobj);
> -		kobj->parent = parent;

I think this might break code as well, did you test this?  What about
the root kobjects with no parent, you still need to set that, right?

What problem does this solve that you have run into?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Re: [PATCH] kobject: kobject_add_internal cleanup
       [not found]   ` <3be7ce57.62f6.17af280a47f.Coremail.qiaoyanbo_310@163.com>
@ 2021-07-29 13:59     ` Greg KH
       [not found]       ` <54cc7e43.808a.17af32725d9.Coremail.qiaoyanbo_310@163.com>
  0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2021-07-29 13:59 UTC (permalink / raw)
  To: qiaoyanbo_310; +Cc: rafael, linux-kernel

A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Thu, Jul 29, 2021 at 09:42:34PM +0800, qiaoyanbo_310 wrote:
> Hi Greg k-h,
> 
> 
> First at all, there is my mistake about From line different with Sign off line. They all should be "Qiao Yanbo".

Great, please fix that up.

> Secondly, the problem this patch wants to solve is: In this function, the parent variable is first called "kobject_get (kobj->parent) " assignment. When the parent is not null, the just obtained parent is assigned back to "kobj - > parent", which is meaningless. Actually, "kobj->kset->kobj" will be assigned to "kobj->parent" only when the parent is NULL. So this patch solves the problem of meaningless assignment.

Why is this a problem?  What bug is this solving?  Is the code somehow
now faster or smaller that can be measured?

> Finally , I tested this patch and didn't see any additional problems in my environment.
> 
> 
> Here is the original code snippet. I hope you can understand my idea.
> ======================================================= 
>        parent = kobject_get(kobj->parent);
> 
> 
>         /* join kset if set, use it as parent if we do not already have one */
>         if (kobj->kset) {
>                 if (!parent)
>                         parent = kobject_get(&kobj->kset->kobj);
>                 kobj_kset_join(kobj);
>                 kobj->parent = parent;

I do not see a bug with this code, what is wrong with it?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Re: [PATCH] kobject: kobject_add_internal cleanup
       [not found]       ` <54cc7e43.808a.17af32725d9.Coremail.qiaoyanbo_310@163.com>
@ 2021-07-30  4:48         ` Greg KH
  2021-07-30 12:47           ` qiaoyanbo_310
  0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2021-07-30  4:48 UTC (permalink / raw)
  To: qiaoyanbo_310; +Cc: rafael, linux-kernel

On Fri, Jul 30, 2021 at 12:44:26AM +0800, qiaoyanbo_310 wrote:
> Hi Greg KH,

Sorry, but html email is rejected by the mailing lists.  Please try
again after fixing up your email client, and I will be glad to respond.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Re: [PATCH] kobject: kobject_add_internal cleanup
  2021-07-30  4:48         ` Greg KH
@ 2021-07-30 12:47           ` qiaoyanbo_310
  2021-07-30 13:37             ` Greg KH
  0 siblings, 1 reply; 6+ messages in thread
From: qiaoyanbo_310 @ 2021-07-30 12:47 UTC (permalink / raw)
  To: Greg KH; +Cc: rafael, linux-kernel

On Fri, Jul 30, 2021 at 06:48:23AM +0200, Greg KH wrote:
> On Fri, Jul 30, 2021 at 12:44:26AM +0800, qiaoyanbo_310 wrote:
> > Hi Greg KH,
> 
> Sorry, but html email is rejected by the mailing lists.  Please try
> again after fixing up your email client, and I will be glad to respond.
> 
> thanks,
> 
> greg k-h

(Sorry, this is the first time I use the mutt environment to send mail,
	shamed on me I know~)

>Why is this a problem?  What bug is this solving?  Is the code somehow
>now faster or smaller that can be measured? 

I don't think the logic of the original code is self consistent.
This is not a bug,  but it causes  some unnecessary assignment operations.
This patch is to clean unnecessary assignment operations.

Thanks,
Qiao Yanbo


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Re: [PATCH] kobject: kobject_add_internal cleanup
  2021-07-30 12:47           ` qiaoyanbo_310
@ 2021-07-30 13:37             ` Greg KH
  0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2021-07-30 13:37 UTC (permalink / raw)
  To: qiaoyanbo_310; +Cc: rafael, linux-kernel

On Fri, Jul 30, 2021 at 08:47:28PM +0800, qiaoyanbo_310@163.com wrote:
> On Fri, Jul 30, 2021 at 06:48:23AM +0200, Greg KH wrote:
> > On Fri, Jul 30, 2021 at 12:44:26AM +0800, qiaoyanbo_310 wrote:
> > > Hi Greg KH,
> > 
> > Sorry, but html email is rejected by the mailing lists.  Please try
> > again after fixing up your email client, and I will be glad to respond.
> > 
> > thanks,
> > 
> > greg k-h
> 
> (Sorry, this is the first time I use the mutt environment to send mail,
> 	shamed on me I know~)
> 
> >Why is this a problem?  What bug is this solving?  Is the code somehow
> >now faster or smaller that can be measured? 
> 
> I don't think the logic of the original code is self consistent.

I no longer even remember what the patch was anymore here, sorry.
That's the problem when emails are not quoted properly :(

> This is not a bug,  but it causes  some unnecessary assignment operations.

Are you sure?  What would set the kobject parent to NULL then?

> This patch is to clean unnecessary assignment operations.

Are you sure there is anything unneeded?  Have you measured the
performance issues here?  What benchmark is affected by this?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-07-30 13:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-27 14:32 [PATCH] kobject: kobject_add_internal cleanup Qiao Yanbo
2021-07-28 10:10 ` Greg KH
     [not found]   ` <3be7ce57.62f6.17af280a47f.Coremail.qiaoyanbo_310@163.com>
2021-07-29 13:59     ` Greg KH
     [not found]       ` <54cc7e43.808a.17af32725d9.Coremail.qiaoyanbo_310@163.com>
2021-07-30  4:48         ` Greg KH
2021-07-30 12:47           ` qiaoyanbo_310
2021-07-30 13:37             ` Greg KH

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