public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: linux-pci@vger.kernel.org, "Bjorn Helgaas" <bhelgaas@google.com>,
	"Geert Uytterhoeven" <geert@linux-m68k.org>,
	"Kai-Heng Feng" <kaihengf@nvidia.com>,
	"Rob Herring" <robh@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>
Subject: Re: [PATCH 2/3] PCI: Do not coalesce host bridge resource structs in place
Date: Mon, 20 Oct 2025 20:44:21 +0300	[thread overview]
Message-ID: <aPZ09UZMfKhYSUZE@smile.fi.intel.com> (raw)
In-Reply-To: <8401388b-2957-0853-d80b-4479e02c47f0@linux.intel.com>

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



  reply	other threads:[~2025-10-20 17:44 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-10 14:42 [PATCH 0/3] PCI & resource: Make coalescing host bridge windows safer Ilpo Järvinen
2025-10-10 14:42 ` [PATCH 1/3] PCI: Refactor host bridge window coalescing loop to use prev Ilpo Järvinen
2025-10-10 14:42 ` [PATCH 2/3] PCI: Do not coalesce host bridge resource structs in place Ilpo Järvinen
2025-10-15 14:29   ` Andy Shevchenko
2025-10-20 17:21     ` Ilpo Järvinen
2025-10-20 17:44       ` Andy Shevchenko [this message]
2025-10-20 18:15         ` Ilpo Järvinen
2025-10-20 18:30           ` Andy Shevchenko
2025-10-10 14:42 ` [PATCH 3/3] resource, kunit: add test case for resource_coalesce() Ilpo Järvinen
2025-10-20 13:42 ` [PATCH 0/3] PCI & resource: Make coalescing host bridge windows safer Geert Uytterhoeven
2025-10-20 16:20   ` Ilpo Järvinen
2025-10-21  7:44     ` Geert Uytterhoeven
2025-10-21 11:54       ` Ilpo Järvinen
2025-10-21 15:49         ` Andy Shevchenko
2025-10-21 16:09           ` Ilpo Järvinen
2025-10-22  7:19           ` Geert Uytterhoeven
2025-10-22  7:45         ` Geert Uytterhoeven
2025-10-22 11:13           ` Ilpo Järvinen
2025-10-22 12:14           ` Ilpo Järvinen
2025-10-22 12:51             ` Rob Herring
2025-10-23 23:02             ` Bjorn Helgaas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aPZ09UZMfKhYSUZE@smile.fi.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=bhelgaas@google.com \
    --cc=geert@linux-m68k.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=kaihengf@nvidia.com \
    --cc=kw@linux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox