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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 065D6C6FD1D for ; Wed, 15 Mar 2023 12:18:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3293A6B0072; Wed, 15 Mar 2023 08:18:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2D9196B0074; Wed, 15 Mar 2023 08:18:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1A1D56B0075; Wed, 15 Mar 2023 08:18:42 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 0A5A76B0072 for ; Wed, 15 Mar 2023 08:18:42 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id BA1C9A0D9E for ; Wed, 15 Mar 2023 12:18:41 +0000 (UTC) X-FDA: 80571036042.06.9F9F21A Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf21.hostedemail.com (Postfix) with ESMTP id 6C08D1C0022 for ; Wed, 15 Mar 2023 12:18:39 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf21.hostedemail.com: domain of robin.murphy@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=robin.murphy@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1678882719; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=feUPVHkAjuRUK/nJ1rPQG4EDwSkyn+96bP9ROYHl83c=; b=1Cpb0Da81PMr3wF3dxeOK0UCxqLjG0HLSWYyTXFRHg9QKAPpMguYjweyzkbDXl02ybcc68 fWu7Kcus5Tcn2Rq10pjXxza5Yd0Tm85phXFZxg5AxeW/Rl75Psjl2Na2SIRPJMvFLy9qkB ftG1WRW3RRdy/INu1vtz2vt99L8Hl3I= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf21.hostedemail.com: domain of robin.murphy@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=robin.murphy@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1678882719; a=rsa-sha256; cv=none; b=VHk2vKVBE9TUvzWp/+XTW/I+xHomrLalGdQYACU3qcGBoG55LZSGxL7FhtwS4bEKSJHiBZ bSXew73IoGv/dqUMoyPX6LY25ba5Sc3iFlo0Q/nI+Vac0J+YK8wC0lGk41A+AkfAqtsJe8 2TWOgmVA9BlfSsrHHKP6vCMpqATKO+4= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E537F2F4; Wed, 15 Mar 2023 05:19:21 -0700 (PDT) Received: from [10.57.53.4] (unknown [10.57.53.4]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 916863F67D; Wed, 15 Mar 2023 05:18:36 -0700 (PDT) Message-ID: <97aced64-c0ac-6041-41cd-ae3439216089@arm.com> Date: Wed, 15 Mar 2023 12:18:31 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [PATCH 09/10] iommu: Fix MAX_ORDER usage in __iommu_dma_alloc_pages() To: "Kirill A. Shutemov" , Andrew Morton , Mel Gorman , Vlastimil Babka , David Hildenbrand Cc: linux-mm@kvack.org, linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org, Jacob Pan , Joerg Roedel References: <20230315113133.11326-1-kirill.shutemov@linux.intel.com> <20230315113133.11326-10-kirill.shutemov@linux.intel.com> Content-Language: en-GB From: Robin Murphy In-Reply-To: <20230315113133.11326-10-kirill.shutemov@linux.intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 6C08D1C0022 X-Stat-Signature: grxmdekmwdubi8myi36p5opnir6fsak8 X-Rspam-User: X-HE-Tag: 1678882719-173631 X-HE-Meta: U2FsdGVkX1+BHSGbcEgSdzsWCESx9MgVeoBk2uUgDUbouQNWN2x9o1HpLZgcAVn/EpxlZfDQpSBWNlMh3by3T2SVsGHWtZSLUKSwLHK/MeF+RtB0CrLCZIV75oCdBeDRSx3xU/sJanXRdsKruGak2+DSbbHdQCmPSR4ZftOfeT1GG6nYW3ltMmK0/Dgt7687RHSWWdfwmoy0HU+gBPPHTCSkWgBzAqDr062dR0l1HMhE3BTxeLTurWTNeG6TfutqSRi6OtceAx7S3KdsnyAIQQohcX+YVX8I06kyhsJItARTqWJsy4ZA74PyzV/ao6w/irZXgDOPpaTtsAAMOuUFlb6P7TkXdcwySq5onmKyqs0aazxyZJaJSbFHKnkYZQAoh3AykviIG9SAH2mK67/rfBTi4FPi7uW4RJgGFDwvNNj3frsxdLcc+Q7EJBnj1h7lDREniwjw7+g9NZPJtPeBlUi/hJvW8XC1Bvbz6b1pLRuf/W6nrHWcTnyX0RDDQeynnRueSmZky1wIyl5V23jE6uNSQ6+A57ycSLmjEcFdW7/ylMf+dowVQz+FCLTQhwzd9ubjJMe06OeFm1KNy8NFmo6PeP0TcISPNpkP4xKW9+gfhtGtAyIXdQORr+vjL0UeetAoUCXEBY+rgH9jDvrdEd3AskxLo7Bn1w6p0zmF97l2VBaUByWcWN9JiNYAV1F3cF5Qz7+Tvoxdpe+EZ6OWvSgaAy4BWbsVsmHN/a4/lbwxiLbd+rpIjT+nZ+x/Rcg60e6wh2I9B98IL0ThvmG9/uxIM2dGJiXdWX8u+GPlvEAfu10uqa1bFW/s1LbeHTiKWSmRy3/rz/UWEYMl6fKC/RdzQWceD9tIR/8vDPi1sE/kgdJmGhdHJ9jtpcmczsukCgLGM0a/yGiw95erC0sLdR/6EeKniO6lsln/MblKtVmAnMaB5mDAGaPZp2G8tb+gEo8UiTkRhGBIZjPt1Yw aePmSBxh O9mUICjTh5okWLnYqdBIKNl6zDF1bDy8WjLmMoOf/5o+Mb8B8mUnC/WAmk5pPK2t4t3HPA9Ypl/aMOKGqoSd+g+5srkF8SU4q5YCAF+geg6VtIAymb3G9yKt91V1+HBSXwicvDW6DPveL34+PMAi9ikaly0CRUr0qkR/o03qahgPMighe3EgxA2IarN1SkFAgM2+32Xgat+pg/3sxXBrkXLecvxieh282GErjgQ+rmn7+Z3X7Oklo5Zrt6L+kyL2cPLxm 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: 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 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 > Cc: Robin Murphy > Cc: Jacob Pan > --- > 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;