Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20210713075726.1232938-1-kai.heng.feng@canonical.com>
 <20210713125007.1260304-1-kai.heng.feng@canonical.com> <5331e942ff28bb191d62bb403b03ceb7d750856c.camel@sipsolutions.net>
In-Reply-To: <5331e942ff28bb191d62bb403b03ceb7d750856c.camel@sipsolutions.net>
From:   Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Wed, 13 Jul 2022 11:36:41 +0200
Message-ID: <CAMuHMdWrLxHi6x0VKeB3emHYkDcVmAm=u4K2xzPqKOQ9znvuNg@mail.gmail.com>
Subject: Re: [PATCH v2] PCI: Reinstate "PCI: Coalesce host bridge contiguous apertures"
To:     Johannes Berg <johannes@sipsolutions.net>
Cc:     Kai-Heng Feng <kai.heng.feng@canonical.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Guenter Roeck <linux@roeck-us.net>,
        "open list:PCI SUBSYSTEM" <linux-pci@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Johannes,

On Wed, Nov 17, 2021 at 6:41 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> So this patch landed now ... :)
>
> > +     /* Coalesce contiguous windows */
> > +     resource_list_for_each_entry_safe(window, n, &resources) {
> > +             if (list_is_last(&window->node, &resources))
> > +                     break;
> > +
> > +             next = list_next_entry(window, node);
> > +             offset = window->offset;
> > +             res = window->res;
> > +             next_offset = next->offset;
> > +             next_res = next->res;
> > +
> > +             if (res->flags != next_res->flags || offset != next_offset)
> > +                     continue;
> > +
> > +             if (res->end + 1 == next_res->start) {
> > +                     next_res->start = res->start;
> > +                     res->flags = res->start = res->end = 0;
> > +             }
> > +     }
> >
>
> Maybe this was already a problem before - but I had a stupid thing in
> arch/um/drivers/virt-pci.c (busn_resource has start == end == 0), and
> your changes here caused that resource to be dropped off the list.
>
> Now this wouldn't be a problem, but we add it using pci_add_resource()
> and then that does a memory allocation, but you don't free it here? I'm
> not sure it'd even be safe to free it here and I'll just set
> busn_resource to have end==1 instead (it's all kind of virtual).
>
> But I still wanted to ask if this might be a problem here for others.

Yes it is.  I've sent a fix
https://lore.kernel.org/r/9c41a4372b27420c732ff5599d823e363de00c6d.1657704829.git.geert+renesas@glider.be

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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