Re:Re: Re: [PATCH] gpio: rockchip: Fix missing of_node_put() in rockchip_gpio_probe() and rockchip_gpiolib_register()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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





[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux