* [PATCH] arm/mach-at91/pm: Do not double put the device node
@ 2018-08-14 1:54 zhong jiang
2018-08-14 12:59 ` Alexandre Belloni
0 siblings, 1 reply; 4+ messages in thread
From: zhong jiang @ 2018-08-14 1:54 UTC (permalink / raw)
To: nicolas.ferre, alexandre.belloni, linux; +Cc: linux-arm-kernel, linux-kernel
Device node iterators put the previous value of the index variable,
so an explicit put causes a double put.
I detect the issue with the help of Coccinelle.
Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
arch/arm/mach-at91/pm.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 32fae4d..a5ec35f 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -143,15 +143,12 @@ static int at91_pm_config_ws(unsigned int pm_mode, bool set)
/* Check if enabled on SHDWC. */
if (wsi->shdwc_mr_bit && !(val & wsi->shdwc_mr_bit))
- goto put_node;
+ continue;
mode |= wsi->pmc_fsmr_bit;
if (wsi->set_polarity)
polarity |= wsi->pmc_fsmr_bit;
}
-
-put_node:
- of_node_put(np);
}
if (mode) {
--
1.7.12.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] arm/mach-at91/pm: Do not double put the device node
2018-08-14 1:54 [PATCH] arm/mach-at91/pm: Do not double put the device node zhong jiang
@ 2018-08-14 12:59 ` Alexandre Belloni
2018-08-16 9:32 ` Claudiu Beznea
0 siblings, 1 reply; 4+ messages in thread
From: Alexandre Belloni @ 2018-08-14 12:59 UTC (permalink / raw)
To: zhong jiang; +Cc: nicolas.ferre, linux, linux-arm-kernel, linux-kernel
On 14/08/2018 09:54:56+0800, zhong jiang wrote:
> Device node iterators put the previous value of the index variable,
> so an explicit put causes a double put.
>
While for_each_matching_node_and_match will get and put the node
properly, there is also a call to of_find_device_by_node that will get a
reference to the node.
> I detect the issue with the help of Coccinelle.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
> arch/arm/mach-at91/pm.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index 32fae4d..a5ec35f 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -143,15 +143,12 @@ static int at91_pm_config_ws(unsigned int pm_mode, bool set)
>
> /* Check if enabled on SHDWC. */
> if (wsi->shdwc_mr_bit && !(val & wsi->shdwc_mr_bit))
> - goto put_node;
> + continue;
>
> mode |= wsi->pmc_fsmr_bit;
> if (wsi->set_polarity)
> polarity |= wsi->pmc_fsmr_bit;
> }
> -
> -put_node:
> - of_node_put(np);
> }
>
> if (mode) {
> --
> 1.7.12.4
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] arm/mach-at91/pm: Do not double put the device node
2018-08-14 12:59 ` Alexandre Belloni
@ 2018-08-16 9:32 ` Claudiu Beznea
2018-08-16 10:18 ` zhong jiang
0 siblings, 1 reply; 4+ messages in thread
From: Claudiu Beznea @ 2018-08-16 9:32 UTC (permalink / raw)
To: Alexandre Belloni, zhong jiang; +Cc: linux-kernel, linux-arm-kernel, linux
Hi Alexandre,
On 14.08.2018 15:59, Alexandre Belloni wrote:
> On 14/08/2018 09:54:56+0800, zhong jiang wrote:
>> Device node iterators put the previous value of the index variable,
>> so an explicit put causes a double put.
>>
>
> While for_each_matching_node_and_match will get and put the node
> properly, there is also a call to of_find_device_by_node that will get a
> reference to the node.
>
Looking through of_find_device_by_node() it seems that a put_device() on the
struct device member of the returned struct platform_device has to be called
instead of of_node_put().
of_find_device_by_node() calls bus_find_device():
dev = bus_find_device(&platform_bus_type, NULL, np, of_dev_node_match);
the match function, of_dev_node_match(), is just as follows:
static int of_dev_node_match(struct device *dev, void *data)
{
return dev->of_node == data;
}
but bus_find_device() takes a reference to the struct device returned in case it
founds a match, via get_device():
struct device *bus_find_device(struct bus_type *bus,
struct device *start, void *data,
int (*match)(struct device *dev,
void*data))
{
struct klist_iter i;
struct device *dev;
if (!bus || !bus->p)
return NULL;
klist_iter_init_node(&bus->p->klist_devices, &i,
(start ? &start->p->knode_bus : NULL));
while ((dev = next_device(&i)))
if (match(dev, data) && get_device(dev))
break;
klist_iter_exit(&i);
return dev;
}
So, I think a put_device(&pdev->dev) has to be called in at91_pm_config_ws()
instead of of_node_put(np). My bad!
Thank you,
Claudiu Beznea
>> I detect the issue with the help of Coccinelle.
>>
>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>> ---
>> arch/arm/mach-at91/pm.c | 5 +----
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
>> index 32fae4d..a5ec35f 100644
>> --- a/arch/arm/mach-at91/pm.c
>> +++ b/arch/arm/mach-at91/pm.c
>> @@ -143,15 +143,12 @@ static int at91_pm_config_ws(unsigned int pm_mode, bool set)
>>
>> /* Check if enabled on SHDWC. */
>> if (wsi->shdwc_mr_bit && !(val & wsi->shdwc_mr_bit))
>> - goto put_node;
>> + continue;
>>
>> mode |= wsi->pmc_fsmr_bit;
>> if (wsi->set_polarity)
>> polarity |= wsi->pmc_fsmr_bit;
>> }
>> -
>> -put_node:
>> - of_node_put(np);
>> }
>>
>> if (mode) {
>> --
>> 1.7.12.4
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] arm/mach-at91/pm: Do not double put the device node
2018-08-16 9:32 ` Claudiu Beznea
@ 2018-08-16 10:18 ` zhong jiang
0 siblings, 0 replies; 4+ messages in thread
From: zhong jiang @ 2018-08-16 10:18 UTC (permalink / raw)
To: Claudiu Beznea; +Cc: Alexandre Belloni, linux-kernel, linux-arm-kernel, linux
On 2018/8/16 17:32, Claudiu Beznea wrote:
> Hi Alexandre,
>
> On 14.08.2018 15:59, Alexandre Belloni wrote:
>> On 14/08/2018 09:54:56+0800, zhong jiang wrote:
>>> Device node iterators put the previous value of the index variable,
>>> so an explicit put causes a double put.
>>>
>> While for_each_matching_node_and_match will get and put the node
>> properly, there is also a call to of_find_device_by_node that will get a
>> reference to the node.
>>
> Looking through of_find_device_by_node() it seems that a put_device() on the
> struct device member of the returned struct platform_device has to be called
> instead of of_node_put().
>
> of_find_device_by_node() calls bus_find_device():
>
> dev = bus_find_device(&platform_bus_type, NULL, np, of_dev_node_match);
>
> the match function, of_dev_node_match(), is just as follows:
>
> static int of_dev_node_match(struct device *dev, void *data)
> {
> return dev->of_node == data;
> }
>
> but bus_find_device() takes a reference to the struct device returned in case it
> founds a match, via get_device():
>
> struct device *bus_find_device(struct bus_type *bus,
> struct device *start, void *data,
> int (*match)(struct device *dev,
> void*data))
> {
> struct klist_iter i;
> struct device *dev;
>
> if (!bus || !bus->p)
> return NULL;
>
> klist_iter_init_node(&bus->p->klist_devices, &i,
> (start ? &start->p->knode_bus : NULL));
> while ((dev = next_device(&i)))
> if (match(dev, data) && get_device(dev))
> break;
> klist_iter_exit(&i);
> return dev;
> }
>
> So, I think a put_device(&pdev->dev) has to be called in at91_pm_config_ws()
> instead of of_node_put(np). My bad!
Yes, you're right. Thanks, Claudiu. I will repost in v2.
Sincerely,
zhong jiang
> Thank you,
> Claudiu Beznea
>
>>> I detect the issue with the help of Coccinelle.
>>>
>>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>>> ---
>>> arch/arm/mach-at91/pm.c | 5 +----
>>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
>>> index 32fae4d..a5ec35f 100644
>>> --- a/arch/arm/mach-at91/pm.c
>>> +++ b/arch/arm/mach-at91/pm.c
>>> @@ -143,15 +143,12 @@ static int at91_pm_config_ws(unsigned int pm_mode, bool set)
>>>
>>> /* Check if enabled on SHDWC. */
>>> if (wsi->shdwc_mr_bit && !(val & wsi->shdwc_mr_bit))
>>> - goto put_node;
>>> + continue;
>>>
>>> mode |= wsi->pmc_fsmr_bit;
>>> if (wsi->set_polarity)
>>> polarity |= wsi->pmc_fsmr_bit;
>>> }
>>> -
>>> -put_node:
>>> - of_node_put(np);
>>> }
>>>
>>> if (mode) {
>>> --
>>> 1.7.12.4
>>>
> .
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-08-16 10:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-14 1:54 [PATCH] arm/mach-at91/pm: Do not double put the device node zhong jiang
2018-08-14 12:59 ` Alexandre Belloni
2018-08-16 9:32 ` Claudiu Beznea
2018-08-16 10:18 ` zhong jiang
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).