Re: [PATCH v5 0/2] media: ipu-bridge: survive module unload and reuse the software nodes on rebind

[Bartosz Golaszewski]

On Wed, 28 Oct 2026 15:41:45 +0100, "D. Manresa" <dmanresa@xxxxxxxxx> said:
> Hello,
>
> Eduardo, Andy, Bart: an apology, and a caveat on this series that comes
> out of your thread.
>
> The cover letter above says that the asymmetry underneath 2/2 is in
> drivers/base rather than in the bridge, and ends by offering to send a
> fix for software_node_notify() "if it should be balanced there instead".
> I wrote that without noticing that Eduardo had already reported exactly
> this on 11 October, with a reproducer and a splat:
>
>   https://lore.kernel.org/linux-media/20261011131518.48838-1-toorandom@xxxxxxxxx/
>
> that Andy had pointed it at the core two days later, and that Bart had
> answered on the 15th that it looks like a bug in swnode.c and that he
> would send a fix proposal. That was on the list I was posting to, eleven
> days before I posted. I should have cited it instead of presenting the
> observation as mine, and the offer at the end of my cover was redundant
> with Bart's. Sorry for the noise.
>
> The analysis in that thread still holds on current mainline, for what it
> is worth: the code it quotes predates c393ef0741d4 ("software node:
> provide wrappers around kobject_get/put()"), so the calls now read
> swnode_get()/swnode_put(), but both are plain passthroughs to
> kobject_get()/kobject_put() and nothing else moved. The put in
> software_node_notify_remove() is still outside the "if (swnode->managed)"
> block, and dev_to_swnode() still resolves dev_fwnode(dev) and then its
> ->secondary, which is what makes the two sides unbalanced: at device_add()
> time a device that has no secondary fwnode yet gives NULL and
> software_node_notify() returns before its swnode_get(), while at
> device_del() the secondary is set, dev_to_swnode() resolves, and the
> swnode_put() runs.
>
> Now the caveat, which is the part that actually matters for this series.
>
> 2/2 depends on that imbalance on purpose. The reuse path keeps the
> reference that software_node_find_by_name() returns, instead of dropping
> it, precisely so that there is something for the unmatched swnode_put()
> to consume on the next device_del(). That is what makes the count flat
> across a PCI remove/rescan cycle (4 -> 3 on remove, back to 4 on bind,
> over 16 cycles) rather than losing one per cycle.
>
> So if the core put becomes balanced, 2/2 as posted leaks one reference
> per remove/rescan cycle and the IPU node is never freed. Eduardo's patch
> is in the same position for the same reason: its fwnode_handle_get()
> before set_secondary_fwnode() has no counterpart once the core stops
> putting what it never got. Both are compensations for one core bug, at
> two different sites, and whichever core fix lands should take both of
> them with it.
>
> They do not collide textually: Eduardo's hunk is in
> ipu_bridge_instantiate_ivsc(), on the mei-csi device, and mine is in
> ipu_bridge_init(), on the IPU PCI device.
>
> Sakari, Hans: given the above, I would rather you did not apply 2/2 as
> it stands. 1/2 is independent of all this and can go on its own. I am
> happy to rework 2/2 on top of whatever lands in swnode.c, or to drop it
> if the core fix makes it unnecessary.
>
> Bart, if you have a proposal in a tree somewhere I can run it on the
> Surface Pro 7+ that produced the numbers above: three sensors on IPU6
> across PCI remove/rescan cycles, with a module that reads the node's
> kobject reference count directly. The machine is being given away in a
> few days, so that offer has a short fuse; after that I have no IPU6
> hardware and someone else would have to.

Yes, I've sent a series[1]. So far no reviews but please leave your Tested-by
under this thread if you could.

And yes: with the driver core change merged, the code in media should not be
changed as it will lead to a reference leak instead.

>
> Assisted-by: LLM
>

Does this mean you had an agent write this email? This would explain why it's
way too long for what it needs to say. :( I have nothing against using LLMs
for writing code and reviews but would appreciate it if we could keep human
interaction, well... human.

Thanks,
Bartosz

[1] https://lore.kernel.org/all/20261023-swnode-dev-notify-balance-v2-0-948d920a79f6@xxxxxxxxxxxxxxxx/