At 2022-07-06 21:03:27, "Andy Shevchenko" <andy.shevchenko@xxxxxxxxx> wrote: >On Wed, Jul 6, 2022 at 2:51 PM Liang He <windhl@xxxxxxx> wrote: >> At 2022-07-06 20:27:41, "Andy Shevchenko" <andy.shevchenko@xxxxxxxxx> wrote: >> >On Wed, Jul 6, 2022 at 8:29 AM Liang He <windhl@xxxxxxx> wrote: >> >> >> >> We should call of_node_put() for the reference returned by >> >> of_get_parent() which will increase the refcount. >> > >> >Is it suggested by the so-called Hulk Robot? If so, it's not the first >> >time I see that people don't think about, and just repeat as robots >> >do. Have you read the code? Have you tried to understand what may >> >happen when you put an OF node? What would be possible consequences to >> >the rest of the code? >> > >> >The above sentence is correct, implementation is not thought through. >> >It might be a correct fix, but the commit message doesn't show that >> >you really spent time on the change. >> > >> >P.S> I would personally put all those Hulk Robot bla-bla-bla with >> >lowest priority to consider, too many broken submissions... > >> Thanks very much for you to review my another patch code. > >You're welcome! > >> First of all, all of my commits are not from Hulk Robot and I am sorry that you have spent lots of time >> for those broken submissions. >> >> Second, for this bug, while the of_get_parent() is only 3-4 lines above the adding of_node_put(), >> it is my fault to write so short commit log. > >The problem is not in the length of the commit message, but in the >semantics of it. > >The *put() methods that drop reference counting are part of the >garbage collector. If reference goes to 0 (or other defined threshold >for that purpose) the resource may be very well gone. Now, you should >check who and how is using that resource in the code. Depending on >that the patch may be good in its current form, or should be >completely rewritten. > >-- >With Best Regards, >Andy Shevchenko Thanks, Andy, For refcounting, please trust me, I have spent years on it. But considering other parts of these kernel files and their relationship, i.e., "who and how is using that resource", your suggestion is right and professional. I will treat these possible bugs more carefully and begin to consider if it is better to completely rewritten. Thanks again, Liang