| Message ID | 20230315113133.11326-10-kirill.shutemov@linux.intel.com |
|---|---|
| State | New |
| Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org>
Delivered-To: ouuuleilei@gmail.com
Received: by 2002:a5d:5915:0:0:0:0:0 with SMTP id v21csp2273970wrd;
Wed, 15 Mar 2023 04:35:23 -0700 (PDT)
X-Google-Smtp-Source:
AK7set+mz3RFawyks8GWgmBfCr6fICIecmWJgSLW7AaMtRryJc2tBerom1uf8l61xcFyikb4h3G5
X-Received: by 2002:a9d:461d:0:b0:68b:d34d:8ac1 with SMTP id
y29-20020a9d461d000000b0068bd34d8ac1mr19405510ote.23.1678880123046;
Wed, 15 Mar 2023 04:35:23 -0700 (PDT)
ARC-Seal: i=1; a=rsa-sha256; t=1678880123; cv=none;
d=google.com; s=arc-20160816;
b=nnL1re0l+Cbmk46AE0neVV1OU35Y7nEwDSECu+ryAumhl5749a+ocvoea463NP3c46
fi5RqLglgELUFSQDokNNkGpfS/5mgm8aiC0RXLP/4vOhOgeEl/xlTx55VKa9UHdJeOEF
eTobj2Tg6TD1xUpwA3AJ5oZy+KC5h70KUfWLVVIIg9Vmqu234zlHhCh5hIQSsKhqea8Q
rBCadD4/HToVc700NtEv4m3C51Yc1ygPlCpaDzeVR8ipG8BBhD0DGgDKKbK5KWflPYOL
tFdIP3UjEJrF0POvjlld8/qeoe9A8Rqvg+7v4wJINhNk5t4f8Md3dfzrWwNzxmxK4eMS
Z/bg==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;
s=arc-20160816;
h=list-id:precedence:content-transfer-encoding:mime-version
:references:in-reply-to:message-id:date:subject:cc:to:from
:dkim-signature;
bh=PbeII/iXoJsPjptKNiOA/dG/Ar/LiN8R6+pedl24yA8=;
b=fa5Res1VgOXPijuvocNgtDhUp+WnrQCQpJYn2o2H82Ve3SguM50qLGflz62534ukh3
qukNOSSNeHVmXozv57Y7W/bewCLopHhMwMLQCbqBd78FDAFB00LE8Ve5AbtGzmY1P1TV
doJARI7DjxSFAQaQzM+WJtZLxn/GtmHYYkEQhynh5DUA8zd9B0PAqC4SaNJMQObStQ2f
p6iKB5PpI9CYaJJ8g52mFZzwuCV/uhDC1nupXuySU8I+Wb1zB0xra3Uj9i8sMoE/YTQA
vdmh5y6rMiZPK9pUYvcUbfmnju+kkpyi8t+PI497YZOWL/pFYnoFRXJH6JA5U07GbCdy
ft7w==
ARC-Authentication-Results: i=1; mx.google.com;
dkim=pass header.i=@intel.com header.s=Intel header.b=TljsWKvV;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com
Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20])
by mx.google.com with ESMTP id
m22-20020a9d73d6000000b0068d4d7b4694si4436242otk.232.2023.03.15.04.35.09;
Wed, 15 Mar 2023 04:35:23 -0700 (PDT)
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;
Authentication-Results: mx.google.com;
dkim=pass header.i=@intel.com header.s=Intel header.b=TljsWKvV;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S231819AbjCOLcJ (ORCPT <rfc822;ruipengqi7@gmail.com> + 99 others);
Wed, 15 Mar 2023 07:32:09 -0400
Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60012 "EHLO
lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S231506AbjCOLb4 (ORCPT
<rfc822;linux-kernel@vger.kernel.org>);
Wed, 15 Mar 2023 07:31:56 -0400
Received: from mga03.intel.com (mga03.intel.com [134.134.136.65])
by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A7AAC65C7D;
Wed, 15 Mar 2023 04:31:46 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple;
d=intel.com; i=@intel.com; q=dns/txt; s=Intel;
t=1678879906; x=1710415906;
h=from:to:cc:subject:date:message-id:in-reply-to:
references:mime-version:content-transfer-encoding;
bh=wxXpVFa3aHUh2uZGVGV6zRauliwfaR2PzVjPmjMqjVA=;
b=TljsWKvVt86ufMoAeafYZk4YW/phJKKvSxtJ2QhpA/oxgfeY3UEFH6E0
D5THCmMYX5kcDqZ1YqNAswb4qv9mJQWKcoptMUvEUuOwTvB9x88J+4Vi2
GRk4KOUmQTQpbeeGQ4PJDB7PsvgZ8KQmkYnJ/qokpHnVebg8FNm/LQA9O
50E3tIlf+6mvOHVuUr8tILQGD5D5lcigw478nTz7IcjJVIlWPQMu+cl/R
lKSPnjqRhjinX8GoyWZxjY22DRv403DvjQnhYuek3LRbj5A8vegqy2/uS
lf7x/9byjXw/EJqStyqtR8tWcklNMyGd3JPkCoYHmNR+Xto3RH0IsFb3l
Q==;
X-IronPort-AV: E=McAfee;i="6500,9779,10649"; a="340040134"
X-IronPort-AV: E=Sophos;i="5.98,262,1673942400";
d="scan'208";a="340040134"
Received: from fmsmga003.fm.intel.com ([10.253.24.29])
by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
15 Mar 2023 04:31:45 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=McAfee;i="6500,9779,10649"; a="768456013"
X-IronPort-AV: E=Sophos;i="5.98,262,1673942400";
d="scan'208";a="768456013"
Received: from nopopovi-mobl1.ger.corp.intel.com (HELO box.shutemov.name)
([10.252.33.48])
by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
15 Mar 2023 04:31:43 -0700
Received: by box.shutemov.name (Postfix, from userid 1000)
id ED58B10D690; Wed, 15 Mar 2023 14:31:35 +0300 (+03)
From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
To: Andrew Morton <akpm@linux-foundation.org>,
Mel Gorman <mgorman@suse.de>, Vlastimil Babka <vbabka@suse.cz>,
David Hildenbrand <david@redhat.com>
Cc: linux-mm@kvack.org, linux-arch@vger.kernel.org,
linux-kernel@vger.kernel.org,
"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
Robin Murphy <robin.murphy@arm.com>,
Jacob Pan <jacob.jun.pan@linux.intel.com>
Subject: [PATCH 09/10] iommu: Fix MAX_ORDER usage in __iommu_dma_alloc_pages()
Date: Wed, 15 Mar 2023 14:31:32 +0300
Message-Id: <20230315113133.11326-10-kirill.shutemov@linux.intel.com>
X-Mailer: git-send-email 2.39.2
In-Reply-To: <20230315113133.11326-1-kirill.shutemov@linux.intel.com>
References: <20230315113133.11326-1-kirill.shutemov@linux.intel.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE,
SPF_NONE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?=
X-GMAIL-THRID: =?utf-8?q?1760433404172667215?=
X-GMAIL-MSGID: =?utf-8?q?1760433404172667215?=
|
| Series |
Fix confusion around MAX_ORDER
|
|
Commit Message
Kirill A. Shutemov
March 15, 2023, 11:31 a.m. UTC
MAX_ORDER is not inclusive: the maximum allocation order buddy allocator
can deliver is MAX_ORDER-1.
Fix MAX_ORDER usage in __iommu_dma_alloc_pages().
Also use GENMASK() instead of hard to read "(2U << order) - 1" magic.
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
drivers/iommu/dma-iommu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Comments
On 2023-03-15 11:31, Kirill A. Shutemov wrote: > MAX_ORDER is not inclusive: the maximum allocation order buddy allocator > can deliver is MAX_ORDER-1. > > Fix MAX_ORDER usage in __iommu_dma_alloc_pages(). Technically this isn't a major issue - all it means is that if we did happen to have a suitable page size which lined up with MAX_ORDER, we'd unsuccessfully try the allocation once before falling back to the order of the next-smallest page size anyway. Semantically you're correct though, and I probably did still misunderstand MAX_ORDER 7 years ago :) > Also use GENMASK() instead of hard to read "(2U << order) - 1" magic. ISTR that GENMASK() had a habit of generating pretty terrible code for non-constant arguments, but a GCC9 build for arm64 looks fine - in fact if anything it seems to be able to optimise out more of the __fls() this way and save a couple more instructions, which is nice, so: Acked-by: Robin Murphy <robin.murphy@arm.com> I'm guessing you probably want to take this through the mm tree - that should be fine since I don't expect any conflicting changes in the IOMMU tree for now (cc'ing Joerg just as a heads-up). Cheers, Robin. > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > Cc: Robin Murphy <robin.murphy@arm.com> > Cc: Jacob Pan <jacob.jun.pan@linux.intel.com> > --- > drivers/iommu/dma-iommu.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index 99b2646cb5c7..ac996fd6bd9c 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -736,7 +736,7 @@ static struct page **__iommu_dma_alloc_pages(struct device *dev, > struct page **pages; > unsigned int i = 0, nid = dev_to_node(dev); > > - order_mask &= (2U << MAX_ORDER) - 1; > + order_mask &= GENMASK(MAX_ORDER - 1, 0); > if (!order_mask) > return NULL; > > @@ -756,7 +756,7 @@ static struct page **__iommu_dma_alloc_pages(struct device *dev, > * than a necessity, hence using __GFP_NORETRY until > * falling back to minimum-order allocations. > */ > - for (order_mask &= (2U << __fls(count)) - 1; > + for (order_mask &= GENMASK(__fls(count), 0); > order_mask; order_mask &= ~order_size) { > unsigned int order = __fls(order_mask); > gfp_t alloc_flags = gfp;
Hi Kirill, On Wed, 15 Mar 2023 14:31:32 +0300, "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> wrote: > MAX_ORDER is not inclusive: the maximum allocation order buddy allocator > can deliver is MAX_ORDER-1. > > Fix MAX_ORDER usage in __iommu_dma_alloc_pages(). > > Also use GENMASK() instead of hard to read "(2U << order) - 1" magic. > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > Cc: Robin Murphy <robin.murphy@arm.com> > Cc: Jacob Pan <jacob.jun.pan@linux.intel.com> > --- > drivers/iommu/dma-iommu.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index 99b2646cb5c7..ac996fd6bd9c 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -736,7 +736,7 @@ static struct page **__iommu_dma_alloc_pages(struct > device *dev, struct page **pages; > unsigned int i = 0, nid = dev_to_node(dev); > > - order_mask &= (2U << MAX_ORDER) - 1; > + order_mask &= GENMASK(MAX_ORDER - 1, 0); > if (!order_mask) > return NULL; > > @@ -756,7 +756,7 @@ static struct page **__iommu_dma_alloc_pages(struct > device *dev, > * than a necessity, hence using __GFP_NORETRY until > * falling back to minimum-order allocations. > */ > - for (order_mask &= (2U << __fls(count)) - 1; > + for (order_mask &= GENMASK(__fls(count), 0); > order_mask; order_mask &= ~order_size) { > unsigned int order = __fls(order_mask); > gfp_t alloc_flags = gfp; Reviewed-by: Jacob Pan <jacob.jun.pan@linux.intel.com> (For VT-d part, there is no functional impact at all. We only have 2M and 1G page sizes, no SZ_8M page) Thanks, Jacob
On 3/15/23 12:31, Kirill A. Shutemov wrote: > MAX_ORDER is not inclusive: the maximum allocation order buddy allocator > can deliver is MAX_ORDER-1. > > Fix MAX_ORDER usage in __iommu_dma_alloc_pages(). > > Also use GENMASK() instead of hard to read "(2U << order) - 1" magic. > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> Reviewed-by: Vlastimil Babka <vbabka@suse.cz> > Cc: Robin Murphy <robin.murphy@arm.com> > Cc: Jacob Pan <jacob.jun.pan@linux.intel.com> > --- > drivers/iommu/dma-iommu.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index 99b2646cb5c7..ac996fd6bd9c 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -736,7 +736,7 @@ static struct page **__iommu_dma_alloc_pages(struct device *dev, > struct page **pages; > unsigned int i = 0, nid = dev_to_node(dev); > > - order_mask &= (2U << MAX_ORDER) - 1; > + order_mask &= GENMASK(MAX_ORDER - 1, 0); > if (!order_mask) > return NULL; > > @@ -756,7 +756,7 @@ static struct page **__iommu_dma_alloc_pages(struct device *dev, > * than a necessity, hence using __GFP_NORETRY until > * falling back to minimum-order allocations. > */ > - for (order_mask &= (2U << __fls(count)) - 1; > + for (order_mask &= GENMASK(__fls(count), 0); > order_mask; order_mask &= ~order_size) { > unsigned int order = __fls(order_mask); > gfp_t alloc_flags = gfp;
On Wed, Mar 15, 2023 at 12:18:31PM +0000, Robin Murphy wrote: > I'm guessing you probably want to take this through the mm tree - that > should be fine since I don't expect any conflicting changes in the IOMMU > tree for now (cc'ing Joerg just as a heads-up). Yes, mm tree is fine for this: Acked-by: Joerg Roedel <jroedel@suse.de>
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 99b2646cb5c7..ac996fd6bd9c 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -736,7 +736,7 @@ static struct page **__iommu_dma_alloc_pages(struct device *dev, struct page **pages; unsigned int i = 0, nid = dev_to_node(dev); - order_mask &= (2U << MAX_ORDER) - 1; + order_mask &= GENMASK(MAX_ORDER - 1, 0); if (!order_mask) return NULL; @@ -756,7 +756,7 @@ static struct page **__iommu_dma_alloc_pages(struct device *dev, * than a necessity, hence using __GFP_NORETRY until * falling back to minimum-order allocations. */ - for (order_mask &= (2U << __fls(count)) - 1; + for (order_mask &= GENMASK(__fls(count), 0); order_mask; order_mask &= ~order_size) { unsigned int order = __fls(order_mask); gfp_t alloc_flags = gfp;