[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL_JsqL4cb8NVOV9QF5__PtjyDUQd-MnuhJhbTHmUF3qr3x47w@mail.gmail.com>
Date: Thu, 29 Feb 2024 15:50:04 -0600
From: Rob Herring <robh@...nel.org>
To: Peng Fan <peng.fan@....com>
Cc: "Peng Fan (OSS)" <peng.fan@....nxp.com>, "saravanak@...gle.com" <saravanak@...gle.com>,
"bhelgaas@...gle.com" <bhelgaas@...gle.com>, "pali@...nel.org" <pali@...nel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] of: dynamic: notify before revert
On Thu, Feb 29, 2024 at 2:01 AM Peng Fan <peng.fan@....com> wrote:
>
> > Subject: Re: [PATCH] of: dynamic: notify before revert
> >
> > On Wed, Feb 28, 2024 at 12:13 AM Peng Fan (OSS) <peng.fan@....nxp.com>
> > wrote:
> > >
> > > From: Peng Fan <peng.fan@....com>
> > >
> > > When PCI node was created using an overlay and the overlay is
> > > reverted/destroyed, the "linux,pci-domain" property no longer exists,
> > > so of_get_pci_domain_nr will return failure. Then
> > > of_pci_bus_release_domain_nr will actually use the dynamic IDA, even
> > > if the IDA was allocated in static IDA.
> >
> > That usage is broken to begin with unless there is a guarantee that static and
> > dynamic domain numbers don't overlap. For example, a dynamic number is
> > assigned and then you load an overlay with the same number defined in it.
>
> I may not describe it clear, the code is here, because overlay property
> removed, of_get_pci_domain_nr will return failure, so the code path
> will goest into free a dynamic ida. But actually there is no such dynamic
> ida, so dump.
I understood the problem.
Your usage of this is broken on applying your overlay. You just got lucky.
> So the problem is overlay was removed, but the notify callback may
> still use the property to do some work.
>
> static void of_pci_bus_release_domain_nr(struct pci_bus *bus, struct device *parent)
> {
> if (bus->domain_nr < 0)
> return;
>
> /* Release domain from IDA where it was allocated. */
> if (of_get_pci_domain_nr(parent->of_node) == bus->domain_nr)
> ida_free(&pci_domain_nr_static_ida, bus->domain_nr);
> else
> ida_free(&pci_domain_nr_dynamic_ida, bus->domain_nr);
> }
> >
> > > So move the notify before revert to fix the issue.
> >
> > You can't just change the timing. Something might require notify to be after
> > the revert.
Again ^^^
> >
> > > Fixes: c14f7ccc9f5d ("PCI: Assign PCI domain IDs by ida_alloc()")
> >
> > I don't see where the notifier is even used.
>
> The stack is as below:
>
> [ 595.150529] CPU: 1 PID: 582 Comm: jailhouse Tainted: G O 6.5.0-rc4-next-20230804-05021-g3d4cc14b42ef-dirty #355
> [ 595.161998] Hardware name: NXP i.MX93 11X11 EVK board (DT)
> [ 595.167475] Call trace:
> [ 595.169908] dump_backtrace+0x94/0xec
> [ 595.173573] show_stack+0x18/0x24
> [ 595.176884] dump_stack_lvl+0x48/0x60
> [ 595.180541] dump_stack+0x18/0x24
> [ 595.183843] pci_bus_release_domain_nr+0x34/0x84
> [ 595.188453] pci_remove_root_bus+0xa0/0xa4
> [ 595.192544] pci_host_common_remove+0x28/0x40
> [ 595.196895] platform_remove+0x54/0x6c
> [ 595.200641] device_remove+0x4c/0x80
> [ 595.204209] device_release_driver_internal+0x1d4/0x230
> [ 595.209430] device_release_driver+0x18/0x24
> [ 595.213691] bus_remove_device+0xcc/0x10c
> [ 595.217686] device_del+0x15c/0x41c
> [ 595.221170] platform_device_del.part.0+0x1c/0x88
> [ 595.225861] platform_device_unregister+0x24/0x40
> [ 595.230557] of_platform_device_destroy+0xfc/0x10c
> [ 595.235344] of_platform_notify+0x13c/0x178
> [ 595.239518] blocking_notifier_call_chain+0x6c/0xa0
> [ 595.244389] __of_changeset_entry_notify+0x148/0x16c
> [ 595.249346] of_changeset_revert+0xa8/0xcc
> [ 595.253437] jailhouse_pci_virtual_root_devices_remove+0x50/0x74 [jailhouse]
$ git grep jailhouse_pci_virtual_root_devices_remove
(END)
Another out of tree overlay user. I have little interest in worrying
about them. No one wants to step up and solve the problems with
overlays, so I'm not going to worry about them either.
Rob
Powered by blists - more mailing lists