LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Laurentiu Tudor <firstname.lastname@example.org>
To: Andy Shevchenko <email@example.com>
Cc: Andy Shevchenko <firstname.lastname@example.org>,
Jon Nettleton <email@example.com>,
Heikki Krogerus <firstname.lastname@example.org>,
Greg Kroah-Hartman <email@example.com>,
"Rafael J . Wysocki" <firstname.lastname@example.org>,
ACPI Devel Maling List <email@example.com>,
Linux Kernel Mailing List <firstname.lastname@example.org>
Subject: Re: [PATCH] software node: balance refcount for managed sw nodes
Date: Mon, 26 Jul 2021 10:59:20 +0300 [thread overview]
Message-ID: <email@example.com> (raw)
On 7/20/2021 1:27 PM, Andy Shevchenko wrote:
> On Tue, Jul 20, 2021 at 12:22 PM Laurentiu Tudor
> <firstname.lastname@example.org> wrote:
>> On 7/19/2021 3:22 PM, Andy Shevchenko wrote:
>>> On Mon, Jul 19, 2021 at 03:00:17PM +0300, Laurentiu Tudor wrote:
>>>> On 7/16/2021 8:21 PM, Jon Nettleton wrote:
>>>>> On Fri, Jul 16, 2021 at 2:17 PM Andy Shevchenko
>>>>> <email@example.com> wrote:
>>>>>> On Fri, Jul 16, 2021 at 01:16:02PM +0300, firstname.lastname@example.org wrote:
>>>>>>> From: Laurentiu Tudor <email@example.com>
>>>>>>> software_node_notify(), on KOBJ_REMOVE drops the refcount twice on managed
>>>>>>> software nodes, thus leading to underflow errors. Balance the refcount by
>>>>>>> bumping it in the device_create_managed_software_node() function.
>>>>>>> The error  was encountered after adding a .shutdown() op to our
>>>>>>> fsl-mc-bus driver.
>>>>>> Looking into the history of adding ->shutdown() to dwc3 driver (it got reverted
>>>>>> later on), I can tell that probably something is wrong in the ->shutdown()
>>>>>> method itself.
>>>>> Isn't the other alternative to just remove the second kobject_put from
>>>>> KOBJ_REMOVE ?
>>>> Or maybe on top of Heikki's suggestion, replace the calls to
>>>> sysfs_create_link() from KOBJ_ADD with sysfs_create_link_nowarn()?
>>> _noearn will hide the problem. It was there, it was removed from there.
>>> Perhaps we have to understand the root cause better (some specific flow?).
>>> Any insight from you on the flow when the issue appears? I.o.w. what happened
>>> on the big picture that we got into the warning you see?
>> I encountered the initial issue when trying to shut down a system booted
>> with ACPI but only after adding a .shutdown() callback to our bus driver
>> so that the devices are properly taken down. The problem was that
>> software_node_notify(), on KOBJ_REMOVE was dropping the reference count
>> twice leading to an underflow error. My initial proposal was to just
>> bump the refcount in device_create_managed_software_node(). The device
>> properties that triggered the problem are created here .
>> Heikko suggested that instead of manually incrementing the refcount to
>> use software_node_notify(KOBJ_ADD). This triggered the second issue, a
>> duplicated sysfs entry warning originating in the usb subsystem:
>> device_create_managed_software_node() ends up being called twice, once
>> here  and secondly, the place I previous mentioned .
> This  is what I have reported against DWC3 when ->shutdown() has
> been added there. And here  is another thread about the issue with
> that callback. The ->release() callback is called at put_device() 
> and ->shutdown() is called before that . That said, can you inspect
> your ->shutdown() implementation once more time and perhaps see if
> there is anything that can be amended?
Will do, thanks for the pointers. It could be that we mess something out
in how we use the driver model.
Best Regards, Laurentiu
next prev parent reply other threads:[~2021-07-26 7:59 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-16 10:16 laurentiu.tudor
2021-07-16 10:34 ` Laurentiu Tudor
2021-07-16 12:17 ` Andy Shevchenko
2021-07-16 17:21 ` Jon Nettleton
2021-07-19 12:00 ` Laurentiu Tudor
2021-07-19 12:22 ` Andy Shevchenko
2021-07-20 9:20 ` Laurentiu Tudor
2021-07-20 10:27 ` Andy Shevchenko
2021-07-26 7:59 ` Laurentiu Tudor [this message]
2021-09-07 15:59 ` Laurentiu Tudor
2021-09-09 12:13 ` Heikki Krogerus
2021-09-09 12:16 ` Heikki Krogerus
2021-09-09 14:01 ` Laurentiu Tudor
2021-09-10 12:05 ` Laurentiu Tudor
2021-09-10 12:38 ` Heikki Krogerus
2021-09-10 13:00 ` Laurentiu Tudor
2021-09-14 14:13 ` Heikki Krogerus
2021-09-14 14:00 ` Heikki Krogerus
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--subject='Re: [PATCH] software node: balance refcount for managed sw nodes' \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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).