From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4108EC4338F for ; Wed, 11 Aug 2021 20:47:13 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id C7B0961058 for ; Wed, 11 Aug 2021 20:47:12 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org C7B0961058 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 420666B006C; Wed, 11 Aug 2021 16:47:12 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3CEEF6B0071; Wed, 11 Aug 2021 16:47:12 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2BD9A6B0072; Wed, 11 Aug 2021 16:47:12 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0138.hostedemail.com [216.40.44.138]) by kanga.kvack.org (Postfix) with ESMTP id 0E80A6B006C for ; Wed, 11 Aug 2021 16:47:12 -0400 (EDT) Received: from smtpin39.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 9BDDD231A3 for ; Wed, 11 Aug 2021 20:47:11 +0000 (UTC) X-FDA: 78463984662.39.01C3200 Received: from mail-pj1-f53.google.com (mail-pj1-f53.google.com [209.85.216.53]) by imf18.hostedemail.com (Postfix) with ESMTP id 4FA5D40033B2 for ; Wed, 11 Aug 2021 20:47:11 +0000 (UTC) Received: by mail-pj1-f53.google.com with SMTP id lw7-20020a17090b1807b029017881cc80b7so11607472pjb.3 for ; Wed, 11 Aug 2021 13:47:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=KEwB6Qw4lEC0fIXNQl09WBpglFRwigNGMZ/Ov5z4gtg=; b=gXj0y2a7cVYw7Ywo0ZOEnWi9MmcZOG9+YfaS4PzdpBf6tuJNaT4s+T7hCWXPxPYxWU ufEs/bMJhoHT5rBxckc+jJS13LOtpchF972E7dvlhO4DQVQze3qldmGS235LZ+T75z1h YQqmjWgI9jcmDFUFUKRUpG1xmL16CwB1Zg3d9dJM5tXg5ZrUNX7kyfZR7kM0zbiKT4Sv 8grH/06YzB4/S+CRb05bnBFwx/Mv7/9kUKY/XPS5XGl4ZiGw24uSpLuslk+ODkj7DrTH 3767gDlWWYQpwup14mQuzo9EMyeqyPbBCjp3EjCpCQUx2F+R07UjsGUihc7rhXELrgJm /VJw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=KEwB6Qw4lEC0fIXNQl09WBpglFRwigNGMZ/Ov5z4gtg=; b=UtlCm11OYKp27gq7dtknMWR3aYYOfEaT52y2iJdLxi06zs1vRzTAKIcA165KoNJqsj O3j+wP6kyeLcuqYc4vxjfDS9xkDsEXH6G1TPBtJnb+QcJ0HPglgl5/YvrvX89w16TyaM 5VyW6y60ewWKmdpFItLlsOaDBmc2ChYezvgx0DcdlFR2Kn7VIgaNu8MjxzD05dtFT3JP OD9BFOjQseKO5UIVvaEqUd5WawMDJjafVJEH36w+cCjYZMda/VCbR+T+lmZi+HZbfQnU L/iT7z+lUfpDlSoKzqelUcDriEY8DznO7ncQQNpRN7KsGvI52o17DHKv+PiF03Od4HhY x26A== X-Gm-Message-State: AOAM533nwMCdSoNNzDVryhvVbtiGf2JzwErXhEyhemX/7Bha/VOM0UEo /TN933d4Qg5SP+qE1/rt+M9uBgzvqLUHtc2py5Y= X-Google-Smtp-Source: ABdhPJxlg4PW+VNSqZvn06H62b3WVz9yN6fw3zxPgJOsrR0ZxYMRhZRRVpCUTUKHopfsnNH3lwWYsHZhWGm3KuMk5rI= X-Received: by 2002:a17:90a:604e:: with SMTP id h14mr9384064pjm.181.1628714830139; Wed, 11 Aug 2021 13:47:10 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a17:90b:4c4e:0:0:0:0 with HTTP; Wed, 11 Aug 2021 13:47:09 -0700 (PDT) In-Reply-To: <20210811203612.138506-4-david@redhat.com> References: <20210811203612.138506-1-david@redhat.com> <20210811203612.138506-4-david@redhat.com> From: Andy Shevchenko Date: Wed, 11 Aug 2021 23:47:09 +0300 Message-ID: Subject: Re: [PATCH v1 3/3] kernel/resource: cleanup and optimize iomem_is_exclusive() To: David Hildenbrand Cc: "linux-kernel@vger.kernel.org" , Arnd Bergmann , Greg Kroah-Hartman , "Michael S. Tsirkin" , Jason Wang , "Rafael J. Wysocki" , Andrew Morton , Dan Williams , Hanjun Guo , Andy Shevchenko , "virtualization@lists.linux-foundation.org" , "linux-mm@kvack.org" Content-Type: multipart/alternative; boundary="000000000000e4135305c94eb805" X-Rspamd-Queue-Id: 4FA5D40033B2 Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=gmail.com header.s=20161025 header.b=gXj0y2a7; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf18.hostedemail.com: domain of andyshevchenko@gmail.com designates 209.85.216.53 as permitted sender) smtp.mailfrom=andyshevchenko@gmail.com X-Rspamd-Server: rspam04 X-Stat-Signature: zqinwjabztorhr8udx89kqskqgc76rud X-HE-Tag: 1628714831-536626 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: --000000000000e4135305c94eb805 Content-Type: text/plain; charset="UTF-8" On Wednesday, August 11, 2021, David Hildenbrand wrote: > Let's clean it up a bit, removing the unnecessary usage of r_next() by > next_resource(), and use next_range_resource() in case we are not > interested in a certain subtree. > > Signed-off-by: David Hildenbrand > --- > kernel/resource.c | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/kernel/resource.c b/kernel/resource.c > index 2938cf520ca3..ea853a075a83 100644 > --- a/kernel/resource.c > +++ b/kernel/resource.c > @@ -1754,9 +1754,8 @@ static int strict_iomem_checks; > */ > bool iomem_is_exclusive(u64 addr) > { > - struct resource *p = &iomem_resource; > + struct resource *p; > bool err = false; > - loff_t l; > int size = PAGE_SIZE; > > if (!strict_iomem_checks) > @@ -1765,27 +1764,31 @@ bool iomem_is_exclusive(u64 addr) > addr = addr & PAGE_MASK; > > read_lock(&resource_lock); > - for (p = p->child; p ; p = r_next(NULL, p, &l)) { > + for (p = iomem_resource.child; p ;) { I consider the ordinal part of p initialization is slightly better and done outside of read lock. Something like p= &iomem_res...; read lock for (p = p->child; ...) { > /* > * We can probably skip the resources without > * IORESOURCE_IO attribute? > */ > if (p->start >= addr + size) > break; > - if (p->end < addr) > + if (p->end < addr) { > + /* No need to consider children */ > + p = next_resource_skip_children(p); > continue; > + } > + > /* > * A resource is exclusive if IORESOURCE_EXCLUSIVE is set > * or CONFIG_IO_STRICT_DEVMEM is enabled and the > * resource is busy. > */ > - if ((p->flags & IORESOURCE_BUSY) == 0) > - continue; > - if (IS_ENABLED(CONFIG_IO_STRICT_DEVMEM) > - || p->flags & IORESOURCE_EXCLUSIVE) { > + if (p->flags & IORESOURCE_BUSY && > + (IS_ENABLED(CONFIG_IO_STRICT_DEVMEM) || > + p->flags & IORESOURCE_EXCLUSIVE)) { > err = true; > break; > } > + p = next_resource(p); > } > read_unlock(&resource_lock); > > -- > 2.31.1 > > -- With Best Regards, Andy Shevchenko --000000000000e4135305c94eb805 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable

On Wednesday, August 11, 2021, David Hildenbrand <david@redhat.com> wrote:
Let's clean it up a bit, removing the unnecessary usage o= f r_next() by
next_resource(), and use next_range_resource() in case we are not
interested in a certain subtree.

Signed-off-by: David Hildenbrand <da= vid@redhat.com>
---
=C2=A0kernel/resource.c | 19 +++++++++++--------
=C2=A01 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index 2938cf520ca3..ea853a075a83 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1754,9 +1754,8 @@ static int strict_iomem_checks;
=C2=A0 */
=C2=A0bool iomem_is_exclusive(u64 addr)
=C2=A0{
-=C2=A0 =C2=A0 =C2=A0 =C2=A0struct resource *p =3D &iomem_resource;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0struct resource *p;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 bool err =3D false;
-=C2=A0 =C2=A0 =C2=A0 =C2=A0loff_t l;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 int size =3D PAGE_SIZE;

=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!strict_iomem_checks)
@@ -1765,27 +1764,31 @@ bool iomem_is_exclusive(u64 addr)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 addr =3D addr & PAGE_MASK;

=C2=A0 =C2=A0 =C2=A0 =C2=A0 read_lock(&resource_lock);
-=C2=A0 =C2=A0 =C2=A0 =C2=A0for (p =3D p->child; p ; p =3D r_next(NULL, = p, &l)) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0for (p =3D iomem_resource.child; p ;) {

I consider the ordinal part of p initialization is= slightly better and done outside of read lock.

So= mething like
p=3D &iomem_res...;
read lock
for (p =3D p->child; ...) {

=C2=A0
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /*
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* We can prob= ably skip the resources without
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* IORESOURCE_= IO attribute?
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (p->start >= ;=3D addr + size)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 break;
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (p->end < = addr)
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (p->end < = addr) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0/* No need to consider children */
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0p =3D next_resource_skip_children(p);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 continue;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
+
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /*
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* A resource = is exclusive if IORESOURCE_EXCLUSIVE is set
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* or CONFIG_I= O_STRICT_DEVMEM is enabled and the
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* resource is= busy.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if ((p->flags &a= mp; IORESOURCE_BUSY) =3D=3D 0)
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0continue;
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (IS_ENABLED(CONF= IG_IO_STRICT_DEVMEM)
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|| p->flags & IORESOURCE_EXCLU= SIVE) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (p->flags &am= p; IORESOURCE_BUSY &&
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(IS_E= NABLED(CONFIG_IO_STRICT_DEVMEM) ||
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 p-&g= t;flags & IORESOURCE_EXCLUSIVE)) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 err =3D true;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 break;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0p =3D next_resource= (p);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
=C2=A0 =C2=A0 =C2=A0 =C2=A0 read_unlock(&resource_lock);
=C2=A0
--
2.31.1



--
With Best Regards,
Andy Shevchenko

--000000000000e4135305c94eb805--