[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aPZ09UZMfKhYSUZE@smile.fi.intel.com>
Date: Mon, 20 Oct 2025 20:44:21 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: linux-pci@...r.kernel.org, Bjorn Helgaas <bhelgaas@...gle.com>,
Geert Uytterhoeven <geert@...ux-m68k.org>,
Kai-Heng Feng <kaihengf@...dia.com>, Rob Herring <robh@...nel.org>,
LKML <linux-kernel@...r.kernel.org>,
Krzysztof Wilczyński <kw@...ux.com>,
Linux-Renesas <linux-renesas-soc@...r.kernel.org>
Subject: Re: [PATCH 2/3] PCI: Do not coalesce host bridge resource structs in
place
On Mon, Oct 20, 2025 at 08:21:50PM +0300, Ilpo Järvinen wrote:
> On Wed, 15 Oct 2025, Andy Shevchenko wrote:
> > On Fri, Oct 10, 2025 at 05:42:30PM +0300, Ilpo Järvinen wrote:
...
> > > +/**
> > > + * resource_mergeable - Test if resources are contiguous and can be merged
> > > + * @r1: first resource
> > > + * @r2: second resource
> > > + *
> > > + * Tests @r1 is followed by @r2 contiguously and share the metadata.
> >
> > This needs an additional explanation about name equivalence that's not only by
> > pointers, but by a content.
>
> Okay. The point was to check names are the same, the pointer check was
> just an optimization as these resources are expected to carry the same
> name even on the pointer level.
>
> > > + * Return: %true if resources are mergeable non-destructively.
> > > + */
> > > +static bool resource_mergeable(struct resource *r1, struct resource *r2)
> > > +{
> > > + if ((r1->flags != r2->flags) ||
> > > + (r1->desc != r2->desc) ||
> > > + (r1->parent != r2->parent) ||
> > > + (r1->end + 1 != r2->start))
> > > + return false;
> >
> > > + if (r1->name == r2->name)
> > > + return true;
> > > +
> > > + if (r1->name && r2->name && !strcmp(r1->name, r2->name))
> > > + return true;
> > > +
> > > + return false;
> >
> > Hmm... Can we keep the logic more straight as in returning false cases as soon
> > as possible?
> >
> > I think of something like this:
> >
> > if (r1->name && r2->name)
> > return strcmp(r1->name, r2->name) == 0;
> >
> > return r1->name == r2->name;
>
> But the point the order above was to avoid strcmp() when the pointer
> itself is same which I think is quite common case. I don't think strcmp()
> itself checks whether the pointer is the same.
On the second thought I think comparing by the content is quite a behavioural
change here. Perhaps we may start without doing that first? Theoretically it
might be the case when the content of names is different, but resources are
the same. The case when name is the same (by content, but pointers) with the
idea of having different resources sounds to me quite an awkward case. TL;
DR: What are the cases that we have in practice now?
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists