Received-SPF: pass (google.com: domain of linux-kernel+bounces-200029-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240531053704.2009827-1-olvaffe@gmail.com> <ZlmQ3_wcL3cgp4Hb@smile.fi.intel.com>
 <CAPaKu7SsD+X7KAO=3vEYU_7YGM_f+7k1fdC9nEK=-NaJw8oYaA@mail.gmail.com>
 <Zlw1_n20oqchAYxH@smile.fi.intel.com> <783e9d9a-e408-c6f0-9a4b-050e1979b919@linux.intel.com>
In-Reply-To: <783e9d9a-e408-c6f0-9a4b-050e1979b919@linux.intel.com>
From: Chia-I Wu <olvaffe@gmail.com>
Date: Mon, 3 Jun 2024 22:04:49 -0700
Message-ID: <CAPaKu7R8avTfAZq-V5H6t6AKAGG1kGbdNK019-DQ5sOR=UOo-w@mail.gmail.com>
Subject: Re: [PATCH] kernel/resource: optimize find_next_iomem_res
To: =?UTF-8?Q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>, amd-gfx@lists.freedesktop.org, 
	dri-devel@lists.freedesktop.org, christian.koenig@amd.com, 
	alexander.deucher@amd.com, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, 
	Alison Schofield <alison.schofield@intel.com>, Dave Jiang <dave.jiang@intel.com>, 
	Baoquan He <bhe@redhat.com>, LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Jun 3, 2024 at 12:24=E2=80=AFAM Ilpo J=C3=A4rvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Sun, 2 Jun 2024, Andy Shevchenko wrote:
>
> > On Fri, May 31, 2024 at 02:31:45PM -0700, Chia-I Wu wrote:
> > > On Fri, May 31, 2024 at 1:57=E2=80=AFAM Andy Shevchenko <
> > > andriy.shevchenko@linux.intel.com> wrote:
> > > > On Thu, May 30, 2024 at 10:36:57PM -0700, Chia-I Wu wrote:
> >
> > ...
> >
> > > > P.S> I'm not so sure about this change. It needs a thoroughly testi=
ng, esp.
> > > > in PCI case. Cc'ing to Ilpo.
> >
> > > What's special about PCI?
> >
> > PCI, due to its nature, may rebuild resources either by shrinking or ex=
panding
> > of the entire subtree after the PCI bridge in question. And this may ha=
ppen at
> > run-time due to hotplug support. But I'm not a deep expert in this area=
, Ilpo
> > knows much more than me.
>
> There is code which clearly tries to do expanding resource but that
> usually fails to work as intended because of a parent resource whose size
> is fixed because it's already assigned.
>
> Some other code might block shrinking too under certain conditions.
>
> This area would need to be reworked in PCI core but it's massive and
> scary looking change.
Given the nature of this change (skip checking against children when
the parent does not match), unless a child resource can exceed its
parent resource, I don't think this change affects correctness.

The walk does not hold the resource lock outside of
find_next_iomem_res().  Updating the tree while the walk is in
progress has always been a bit ill-defined.  The patch does not change
that (but it might change the timing a bit).

I can export __walk_iomem_res_desc() and write some unit tests against
it.  Would that be enough to justify this change?

>
> --
>  i.