Re: [PATCH] pmdomain: core: Wait for device link removals before dropping genpd->dev

On Tue, Aug 4, 2026 at 7:31 AM Ulf Hansson <[email protected]> wrote:
>
> >
> + Saravana, Rafael, Danilo
>
> On Fri, Jul 31, 2026 at 12:21 PM jaseg <[email protected]> wrote:
> >
> > From: Jan Sebastian Götte <[email protected]>
> >
> > genpd->dev is embedded in struct generic_pm_domain and its release
> > function is empty, so providers free the containing genpd with a plain
> > kfree() once of_genpd_remove_last() returns, ignoring its refcount.
>
> Yes, there is certainly room for improvements in regards to reference
> counting and freeing data in genpd. It's moving slowly in the right
> direction though.
>
> Anyway, the release function is actually set to
> genpd_provider_release(), which ideally should free the data in the
> long run, but we haven't fully reached that point yet.
>
> That said, I guess you actually are referring to the ->remove()
> callback for the genpd_provider_drv, right?
>
> >
> > Since genpd->dev is registered on the genpd provider bus, fw_devlink
> > creates device links to it, and those are torn down asynchronously.
> > Nothing made genpd_remove() wait for those teardowns, so the provider
> > could free the memory backing genpd->dev while the queued workers still
> > used it.
> >
> > This is reachable at boot on qrb2210, where the firmware rejects PC mode
> > and psci_cpuidle_domain_probe() removes all the CPU PM domains before
> > returning -EPROBE_DEFER. The bug is asymptomatic on defconfig, but shows
> > up when enabling KASAN or INIT_ON_FREE_DEFAULT_ON. In some builds, it
> > causes the kernel to crash a few hundred ms into the boot.
> >
> > Call device_link_wait_removal() before dropping the final reference. All
> > link removal work is queued from device_del(), via
> > device_links_driver_cleanup() and device_links_purge(), which precedes
> > genpd_free_data(), and flush_workqueue() waits for it to complete.
> >
> > Note: This patch was LLM-assisted. I reproduced the issue and tested
> > this patch on hardware, and I did my best to verify it by hand. However,
> > I'm far from an expert in pmdomain, so YMMV.
> >
> > Assisted-by: Claude:claude-5-opus
> > Fixes: 18a3a510ecfd ("pmdomain: core: Add the genpd->dev to the genpd provider bus")
> > Cc: [email protected]
> > Signed-off-by: Jan Sebastian Götte <[email protected]>
> > ---
> >  drivers/pmdomain/core.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> > index 842c4169e290..4eeb980e5a40 100644
> > --- a/drivers/pmdomain/core.c
> > +++ b/drivers/pmdomain/core.c
> > @@ -2348,6 +2348,9 @@ static int genpd_alloc_data(struct generic_pm_domain *genpd)
> >
> >  static void genpd_free_data(struct generic_pm_domain *genpd)
> >  {
> > +       /* Pending device link removals still reference genpd->dev. */
> > +       device_link_wait_removal();
> > +
>
> Even if this fixes the issue, it looks wrong to me that drivers should
> have to deal with this themselves at device removal.
>
> In other words, I was expecting the driver core to deal with this for
> everyone, but that seems not to be the case. Hmm.

I agree.

 I'm not even sure if device_link_wait_removal() should have been
added. The point of refcounting is that we don't have to worry about
who releases in what order and the last one to release will have a
resource freed. Needing both ref counting and a say to sync feels
wrong. Let me take a closer look.

-Saravana

>
> Note that, the device removal should typically happen when the genpd
> provider calls of_genpd_del_provider() and it calls device_del().
>
> >         put_device(&genpd->dev);
> >         if (genpd->device_id != -ENXIO)
> >                 ida_free(&genpd_ida, genpd->device_id);
> > --
> > 2.53.0
> >
>
> I will have a closer look at the driver core for this. In the
> meantime, maybe Saravana, Rafael or Danilo have some ideas already?
>
> Kind regards
> Uffe