Hi Niklas,
On Mon, Oct 31, 2022 at 12:56 PM Niklas Schnelle <schnelle@xxxxxxxxxxxxx> wrote:
> On Wed, 2022-07-13 at 11:35 +0200, Geert Uytterhoeven wrote:
> > On r8a7791/koelsch:
> >
> > kmemleak: 1 new suspected memory leaks (see /sys/kernel/debug/kmemleak)
> > # cat /sys/kernel/debug/kmemleak
> > unreferenced object 0xc3a34e00 (size 64):
> > comm "swapper/0", pid 1, jiffies 4294937460 (age 199.080s)
> > hex dump (first 32 bytes):
> > b4 5d 81 f0 b4 5d 81 f0 c0 b0 a2 c3 00 00 00 00 .]...]..........
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> > backtrace:
> > [<fe3aa979>] __kmalloc+0xf0/0x140
> > [<34bd6bc0>] resource_list_create_entry+0x18/0x38
> > [<767046bc>] pci_add_resource_offset+0x20/0x68
> > [<b3f3edf2>] devm_of_pci_get_host_bridge_resources.constprop.0+0xb0/0x390
> >
> > When coalescing two resources for a contiguous aperture, the first
> > resource is enlarged to cover the full contiguous range, while the
> > second resource is marked invalid. This invalidation is done by
> > clearing the flags, start, and end members.
> >
> > When adding the initial resources to the bus later, invalid resources
> > are skipped. Unfortunately, the check for an invalid resource considers
> > only the end member, causing false positives.
> >
> > E.g. on r8a7791/koelsch, root bus resource 0 ("bus 00") is skipped, and
> > no longer registered with pci_bus_insert_busn_res() (causing the memory
> > leak), nor printed:
> >
> > pci-rcar-gen2 ee090000.pci: host bridge /soc/pci@ee090000 ranges:
> > pci-rcar-gen2 ee090000.pci: MEM 0x00ee080000..0x00ee08ffff -> 0x00ee080000
> > pci-rcar-gen2 ee090000.pci: PCI: revision 11
> > pci-rcar-gen2 ee090000.pci: PCI host bridge to bus 0000:00
> > -pci_bus 0000:00: root bus resource [bus 00]
> > pci_bus 0000:00: root bus resource [mem 0xee080000-0xee08ffff]
> >
> > Fix this by only skipping resources where all of the flags, start, and
> > end members are zero.
> >
> > Fixes: 7c3855c423b17f6c ("PCI: Coalesce host bridge contiguous apertures")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > ---
> > Is there any side effect of not registering the root bus resource with
> > pci_bus_insert_busn_res()? This is the resource created by
> > of_pci_parse_bus_range(), and thus affects any DT platforms using
> > "bus-range = <0 0>".
> >
> > Perhaps checking for "!res->flags" would be sufficient?
> >
> > I assume this still causes memory leaks on systems where resources are
> > coalesced, as the second resource of a contiguous aperture is no longer
> > referenced? Perhaps instead of clearing the resource, it should be
> > removed from the list (and freed? is it actually safe to do that?)?
> >
> > Apparently Johannes had identified the bug before, but didn't realize
> > the full impact...
> > https://lore.kernel.org/r/5331e942ff28bb191d62bb403b03ceb7d750856c.camel@xxxxxxxxxxxxxxxx/
> > ---
> > drivers/pci/probe.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 17a969942d37033a..be628798d279ada0 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -994,7 +994,7 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
> > resource_list_for_each_entry_safe(window, n, &resources) {
> > offset = window->offset;
> > res = window->res;
> > - if (!res->end)
> > + if (!res->flags && !res->start && !res->end)
> > continue;
> >
> > list_move_tail(&window->node, &bridge->windows);
>
> Hi Geert, Hi Bjorn, Hi Kai-Heng,
>
> I just stumbled over this issue on s390 with the below kmemleak
> splat[0]. On s390 we currently always have a single PCI bus with bus
> number 00 per PCI domain so this is triggered whenever there are PCI
> devices attached to the system.
>
> Applying the patch from this mail makes the splat go away and the
> 'pci_bus 0002:00: root bus resource [bus 00]' message reappear. As this
> mail is from July I guess it got lost and this was never picked up ;-(
Sorry, I still have to go over all patches submitted last summer that
didn't make it...
> For now feel free to add my:
>
> Tested-by: Niklas Schnelle <schnelle@xxxxxxxxxxxxx>
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
