Message ID | 20230807152657.1692414-1-m.szyprowski@samsung.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:c44e:0:b0:3f2:4152:657d with SMTP id w14csp1565447vqr; Mon, 7 Aug 2023 09:29:33 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEiF5JooHpnjcOuxV3cW3dfqnIe9/uIYpAFLL5OpzUawnKQ1qGAf8F43TXS0Y+0ZCCXOPXj X-Received: by 2002:a05:6a00:990:b0:668:81c5:2f8d with SMTP id u16-20020a056a00099000b0066881c52f8dmr13342354pfg.3.1691425773021; Mon, 07 Aug 2023 09:29:33 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691425773; cv=none; d=google.com; s=arc-20160816; b=OfRNn8gCfXuATwaSKzz4zmcLmlXcMBa75S7wVYaaSo3V9AOxDHZVI6yHAJ4pajE+eI F5zJ/8xJzVN2OElv/+RH/E1wKp85KsHa3fiLNR44fMp2Y2nLqTHeJzzZwoDlGRbyfNN3 ZQoLYj2AZZM3GCRetOtmTHcmb9sO95i4Fev9/Dyds1fRNkGvtxyKqooFm+Vo5SJt+6Q6 GAGXCxfNSt0utQuWryqQ9QZxs8OceI4sHfHQRzBU/JGOZTtANWfxUpkGnLiCKyqrQiN2 +dsRJRYPftofxWZihZtyYaUU15wcxDYGc1DBViiF6SuvfGsvBV0I4DvyjppVRpGhkal/ N1fw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:references:cms-type:content-transfer-encoding :mime-version:message-id:date:subject:cc:to:from:dkim-signature :dkim-filter; bh=iklEzqWKqrQegkJPehfsGPKu4Pefo/8e0IoAZmUUDW0=; fh=lUJYPoX6nwk7fo9JaX+26+3WTVy906URwanUSJ329aI=; b=ZRpo4UlcqW0qfExnvMTF98kXH0W0ikwT1pNtf/USKKP7z3f5ZCXh19+EH8s1+bwSSJ CaMqo3PimNdbH2lP0glOIJ1Q1/Qjx2YigzbfgLEmacCHQseOBRrXMXZqhvjoxjFzOR3R vue+5rRYgkR2LNR59EAtpHUcj7LFir4O9PXxEWHgzgBU1KlhyNIzcJp0++DClewOEar1 JO9+7MZKTCBmb7ECSS60ubcNx/K3rd8Hx6lR1NLL/2u1wrGzSk258zVyP5wmENiFYwcU GnUJmMmQ8sq51bqIbldS2TGZLsfZLG0eB096p0OOMEDcy7GbEvpmSkT7eCTMF0cTyd0I Ikwg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@samsung.com header.s=mail20170921 header.b=OWsfl2Pj; 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=samsung.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id ay5-20020a056a00300500b00687501ac7dasi5859692pfb.363.2023.08.07.09.29.19; Mon, 07 Aug 2023 09:29:33 -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=@samsung.com header.s=mail20170921 header.b=OWsfl2Pj; 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=samsung.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230061AbjHGP1S (ORCPT <rfc822;aaronkmseo@gmail.com> + 99 others); Mon, 7 Aug 2023 11:27:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50626 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230020AbjHGP1M (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 7 Aug 2023 11:27:12 -0400 Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C3DED83 for <linux-kernel@vger.kernel.org>; Mon, 7 Aug 2023 08:27:07 -0700 (PDT) Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20230807152705euoutp02627ca0a7cd99d022baef4f86a00e2be9~5I17kWr0U2981829818euoutp02S for <linux-kernel@vger.kernel.org>; Mon, 7 Aug 2023 15:27:05 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20230807152705euoutp02627ca0a7cd99d022baef4f86a00e2be9~5I17kWr0U2981829818euoutp02S DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1691422025; bh=iklEzqWKqrQegkJPehfsGPKu4Pefo/8e0IoAZmUUDW0=; h=From:To:Cc:Subject:Date:References:From; b=OWsfl2Pj6A8tYOuPRjQ+KUwt6uALILfTYNxjUU5kobvaL+WgAVBmKpmfXa9IGotBm iBpi7oCLS0n9Nsz9ng3Xnw6Pas09X+Bzo8k35mintxUhGMNI+F/tY6hqdRnQGgRC4S uGCjS3csMFhT9Cq91E4a3qa+ATsQwIo/edUuODso= Received: from eusmges3new.samsung.com (unknown [203.254.199.245]) by eucas1p2.samsung.com (KnoxPortal) with ESMTP id 20230807152705eucas1p2a346041402ee3b5ec69c1cec9beb30ba~5I17Uii1p1871418714eucas1p2I; Mon, 7 Aug 2023 15:27:05 +0000 (GMT) Received: from eucas1p2.samsung.com ( [182.198.249.207]) by eusmges3new.samsung.com (EUCPMTA) with SMTP id 83.A3.37758.94D01D46; Mon, 7 Aug 2023 16:27:05 +0100 (BST) Received: from eusmtrp2.samsung.com (unknown [182.198.249.139]) by eucas1p1.samsung.com (KnoxPortal) with ESMTPA id 20230807152704eucas1p1bbe08af4559a7d2984198fe8ba487a2e~5I17B7BLU0529705297eucas1p18; Mon, 7 Aug 2023 15:27:04 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp2.samsung.com (KnoxPortal) with ESMTP id 20230807152704eusmtrp2104109381cbc9401b455281f01abe001~5I17BYZmd0297802978eusmtrp29; Mon, 7 Aug 2023 15:27:04 +0000 (GMT) X-AuditID: cbfec7f5-815ff7000002937e-cb-64d10d497adf Received: from eusmtip2.samsung.com ( [203.254.199.222]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id CE.A2.14344.84D01D46; Mon, 7 Aug 2023 16:27:04 +0100 (BST) Received: from AMDC4653.eu.corp.samsungelectronics.net (unknown [106.120.51.32]) by eusmtip2.samsung.com (KnoxPortal) with ESMTPA id 20230807152704eusmtip25c0ce70cae15fa0e1f2078cb6b3c7482~5I16jbo5f1091410914eusmtip20; Mon, 7 Aug 2023 15:27:04 +0000 (GMT) From: Marek Szyprowski <m.szyprowski@samsung.com> To: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Cc: Marek Szyprowski <m.szyprowski@samsung.com>, Russell King <linux@armlinux.org.uk>, "Matthew Wilcox (Oracle)" <willy@infradead.org>, Andrew Morton <akpm@linux-foundation.org>, Mike Rapoport <rppt@kernel.org> Subject: [PATCH] arm: dma-mapping: fix potential endless loop in __dma_page_dev_to_cpu() Date: Mon, 7 Aug 2023 17:26:57 +0200 Message-Id: <20230807152657.1692414-1-m.szyprowski@samsung.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Brightmail-Tracker: H4sIAAAAAAAAA01Se0hTcRjld+92d1teuc5snyuzRmkt0iKLRWkPKq5FIRJBRuRwNzN1xuaj B5VR1lwhrvLRtpZW5BzZSu0xI2TLWjbUVBolqUWllfRSI0t7uF0r/zvnfOf8vvPBj8RFRr6E TFFlsmqVIk1KCHk3H3xvmRdLtSrnN/VNlJtsVwh59SsPX95eZyLkzqK7SF7V0CmQN9huYfLh IROxQsC0e1pxpsYiY6qt+QTzsHSYx9RcOsQU1FoRM1A9LU6QIFymZNNSsll1ZEyicGfetxr+ 7rf+e4aaEnOR20+HJpBAR4F+0CrQISEpoi0ImuvNBEcGEVjtRXyODCB40vdO8DdiatONRSoQ fCp0YRzJw6DCrOV5XQS9AHQfdIQXT6LXwtPDWp8Jpz0IbtWdwr2DQDoBvn58xvdiHj0LnBeP +sIUvRzyB+w8bl0o1DuacE4PgMazr306PqofuWHEvY8CXUmC7foQnwusBnepY6xrILx31Y7h qfDbfh7jAscRlA13j5FCBLm9HYhzLYXnzT9Ge5OjK+aArS6Sk1dC040unwy0Pzz9EMCV8IdT N0twTqZAe0zEucPA4Lr6b63jcRvOYQZaDV2+/iJ6G9iN3Vghmm4Yd5ph3GmG/x3KEG5FYjZL k57Mahaq2JwIjSJdk6VKjkjKSK9Go//H/cv19TayvP8S4UQYiZwISFw6iTK/blGKKKVi7z5W nbFdnZXGapxoCsmTiqm50Y1JIjpZkcmmsuxuVv13ipETJLlYMZMnybqcP3S/Ib47bM2cZXce 3Q3IXtOx6GKl/CDlt5+eqPb7WZFp+/1opPhk0EbPig5zT0QiobgcF7IolZSJ+8/1jbyLnVFV XmLZVMiGyVzKhMD6Y0LLmw3B4SUyLO4NJVH6tzXGRhe5pi8eseVXhau0M5PWUeoCovaQ1ul2 E3ar2FkRNFzs0ce7zWc6c3r26Xfd7ujty1lP/thMh56QRi/Upn0Jca6M6uRt7AFzwzVxRqg+ 6GWLMaC3M/fA5zuRgwUJ/cmOYDa7f7J59o4oleGeKeZn11VHWXj83BMj5WjLi8OnV1UGztMv kZSHM/oHUVUXPgYFG5OmhGRv3SzlaXYqFshwtUbxB+1zgRSuAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrALMWRmVeSWpSXmKPExsVy+t/xe7oevBdTDDY9NrKYs34Nm8Wmx9dY LS7vmsNmcWjqXkaLtUfuslscWb+dyeL3jzlsDuwel69dZPbYvELLY9OqTjaPEzN+s3hsXlLv 0bdlFaPH501yAexRejZF+aUlqQoZ+cUltkrRhhZGeoaWFnpGJpZ6hsbmsVZGpkr6djYpqTmZ ZalF+nYJehmt3zezFrzgq/hxNqGB8TRPFyMnh4SAicScS13sXYxcHEICSxklFn/+zQiRkJE4 Oa2BFcIWlvhzrYsNoqiZSaJty0ewIjYBQ4mutyAJTg4RAQ+Jtn/3mEGKmAVuMUpM2P6OHSQh LBAh0TTtFNgkFgFViUOLW1hAbF4Be4nOzztZIDbIS+w/eJYZIi4ocXLmE7A4M1C8eets5gmM fLOQpGYhSS1gZFrFKJJaWpybnltspFecmFtcmpeul5yfu4kRGPTbjv3csoNx5auPeocYmTgY DzFKcDArifDOe3I+RYg3JbGyKrUoP76oNCe1+BCjKdB9E5mlRJPzgXGXVxJvaGZgamhiZmlg amlmrCTO61nQkSgkkJ5YkpqdmlqQWgTTx8TBKdXAtP/l+2U12ZeFL+r/uPluj9eRfY9zDuxw C3s15Y+v1vKTt2JPFlpxTVEytgnvqD/D8u2UAYup7pOH3hGCTvsOGH6Qsnz1NXJGkuKEahmG X9zdue9M+XeuKAo60inatzBOpv/J/J+z1r0R8pXMVku7vra4PXXm5E+2hQUWuZoaTk5Om8vb vKqfSDy5LJ0UO2ueX/aGSK4jjv0LbyiJbd3//cWT8PVhs7TFerIM81q8zX8s3r3saFqaScW6 xepb/+a1L34UJMSzg/dcZlpEz/Lv8pN+yb7wP3rzqZHWo6Vv32/tL5xy/MNP3eXr32inySV9 my/t8kW3hvXlksWiF5elHnosLtRXLRSV90do+tuT65RYijMSDbWYi4oTAa23kCMDAwAA X-CMS-MailID: 20230807152704eucas1p1bbe08af4559a7d2984198fe8ba487a2e X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20230807152704eucas1p1bbe08af4559a7d2984198fe8ba487a2e X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20230807152704eucas1p1bbe08af4559a7d2984198fe8ba487a2e References: <CGME20230807152704eucas1p1bbe08af4559a7d2984198fe8ba487a2e@eucas1p1.samsung.com> X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL, SPF_HELO_PASS,SPF_PASS 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: INBOX X-GMAIL-THRID: 1773588470915536912 X-GMAIL-MSGID: 1773588470915536912 |
Series |
arm: dma-mapping: fix potential endless loop in __dma_page_dev_to_cpu()
|
|
Commit Message
Marek Szyprowski
Aug. 7, 2023, 3:26 p.m. UTC
It is possible that the folio_size() of the next folio returns zero, so
avoid looping with 'left' equals to zero in D-cache cleaning loop.
This fixes the following endless loop observed by RCU stall:
--->8---
rcu: INFO: rcu_sched self-detected stall on CPU
rcu: 0-....: (27320 ticks this GP) idle=e414/1/0x40000002 softirq=36/36 fqs=13044
rcu: (t=27385 jiffies g=-1067 q=34 ncpus=8)
CPU: 0 PID: 93 Comm: kworker/0:1H Not tainted 6.5.0-rc5-next-20230807 #6981
Hardware name: Samsung Exynos (Flattened Device Tree)
Workqueue: mmc_complete mmc_blk_mq_complete_work
PC is at _set_bit+0x28/0x44
LR is at __dma_page_dev_to_cpu+0xdc/0x170
..
_set_bit from __dma_page_dev_to_cpu+0xdc/0x170
__dma_page_dev_to_cpu from dma_direct_unmap_sg+0x100/0x130
dma_direct_unmap_sg from dw_mci_post_req+0x68/0x6c
dw_mci_post_req from mmc_blk_mq_post_req+0x34/0x100
mmc_blk_mq_post_req from mmc_blk_mq_complete_work+0x50/0x60
mmc_blk_mq_complete_work from process_one_work+0x20c/0x4d8
process_one_work from worker_thread+0x58/0x54c
worker_thread from kthread+0xe0/0xfc
kthread from ret_from_fork+0x14/0x2c
--->8---
Fixes: cc24e9c0895c ("arm: implement the new page table range API")
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
arch/arm/mm/dma-mapping.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On Mon, Aug 07, 2023 at 05:26:57PM +0200, Marek Szyprowski wrote: > It is possible that the folio_size() of the next folio returns zero, so What? How can folio_size() return zero? return PAGE_SIZE << folio_order(folio); It is a minimum of PAGE_SIZE. > avoid looping with 'left' equals to zero in D-cache cleaning loop. > > This fixes the following endless loop observed by RCU stall: > --->8--- > rcu: INFO: rcu_sched self-detected stall on CPU > rcu: 0-....: (27320 ticks this GP) idle=e414/1/0x40000002 softirq=36/36 fqs=13044 > rcu: (t=27385 jiffies g=-1067 q=34 ncpus=8) > CPU: 0 PID: 93 Comm: kworker/0:1H Not tainted 6.5.0-rc5-next-20230807 #6981 > Hardware name: Samsung Exynos (Flattened Device Tree) > Workqueue: mmc_complete mmc_blk_mq_complete_work > PC is at _set_bit+0x28/0x44 > LR is at __dma_page_dev_to_cpu+0xdc/0x170 > .. > _set_bit from __dma_page_dev_to_cpu+0xdc/0x170 > __dma_page_dev_to_cpu from dma_direct_unmap_sg+0x100/0x130 > dma_direct_unmap_sg from dw_mci_post_req+0x68/0x6c > dw_mci_post_req from mmc_blk_mq_post_req+0x34/0x100 I don't know what you've actually hit here, but the explanation is wrong.
On Mon, Aug 07, 2023 at 05:26:57PM +0200, Marek Szyprowski wrote: > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c > index 70cb7e63a9a5..02250106e5ed 100644 > --- a/arch/arm/mm/dma-mapping.c > +++ b/arch/arm/mm/dma-mapping.c > @@ -718,7 +718,7 @@ static void __dma_page_dev_to_cpu(struct page *page, unsigned long off, > folio = folio_next(folio); > } > > - while (left >= (ssize_t)folio_size(folio)) { > + while (left && left >= (ssize_t)folio_size(folio)) { > set_bit(PG_dcache_clean, &folio->flags); > left -= folio_size(folio); > folio = folio_next(folio); I've been thinking about this and I think this is the right fix for the wrong reason. I don't understand how it can produce the failure you saw, but we shouldn't be calling folio_next() if left is zero, let alone calling folio_size() on it. So I'd rather see this fix: while (left >= (ssize_t)folio_size(folio)) { set_bit(PG_dcache_clean, &folio->flags); left -= folio_size(folio); + if (!left) + break; folio = folio_next(folio); }
On Mon, Aug 07, 2023 at 11:14:13PM +0100, Matthew Wilcox wrote: > On Mon, Aug 07, 2023 at 05:26:57PM +0200, Marek Szyprowski wrote: > > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c > > index 70cb7e63a9a5..02250106e5ed 100644 > > --- a/arch/arm/mm/dma-mapping.c > > +++ b/arch/arm/mm/dma-mapping.c > > @@ -718,7 +718,7 @@ static void __dma_page_dev_to_cpu(struct page *page, unsigned long off, > > folio = folio_next(folio); > > } > > > > - while (left >= (ssize_t)folio_size(folio)) { > > + while (left && left >= (ssize_t)folio_size(folio)) { > > set_bit(PG_dcache_clean, &folio->flags); > > left -= folio_size(folio); > > folio = folio_next(folio); > > I've been thinking about this and I think this is the right fix for the > wrong reason. I don't understand how it can produce the failure you > saw, but we shouldn't be calling folio_next() if left is zero, let alone > calling folio_size() on it. So I'd rather see this fix: > > while (left >= (ssize_t)folio_size(folio)) { > set_bit(PG_dcache_clean, &folio->flags); > left -= folio_size(folio); > + if (!left) > + break; Given that set_bit() involves atomics, wouldn't it be better if this had been written as: while (left >= folio_size(folio)) { left -= folio_size(folio); set_bit(PG_dcache_clean, &folio->flags); if (!left) break; > folio = folio_next(folio); > } That likely means that folio_size() will only be evaluated once per loop rather than twice. I may be wrong though, I didn't check the generated code. Also, I'm wondering what that ssize_t cast is doing there - "left" is a size_t, which is unsigned. folio_size() returns a size_t, so is also unsigned. Why convert folio_size() to a signed number to then be compared with an unsigned number? Or did "left" get converted to ssize_t along with the folio conversion? Even if it did, how could "left" be negative (except through casting a large positive number as "size" that in 2's complement would be negative after casting to "left") ?
Hi, On 07.08.2023 18:23, Matthew Wilcox wrote: > On Mon, Aug 07, 2023 at 05:26:57PM +0200, Marek Szyprowski wrote: >> It is possible that the folio_size() of the next folio returns zero, so > What? How can folio_size() return zero? > > return PAGE_SIZE << folio_order(folio); > > It is a minimum of PAGE_SIZE. Well, the folio_order() on that next folio returns 255, so folio_size() overflows to zero. However, the main source of this issue is relying on the properties of the folio beyond the requested sync region. > ... Best regards
On Mon, Aug 07, 2023 at 11:46:05PM +0100, Russell King (Oracle) wrote: > On Mon, Aug 07, 2023 at 11:14:13PM +0100, Matthew Wilcox wrote: > > On Mon, Aug 07, 2023 at 05:26:57PM +0200, Marek Szyprowski wrote: > > > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c > > > index 70cb7e63a9a5..02250106e5ed 100644 > > > --- a/arch/arm/mm/dma-mapping.c > > > +++ b/arch/arm/mm/dma-mapping.c > > > @@ -718,7 +718,7 @@ static void __dma_page_dev_to_cpu(struct page *page, unsigned long off, > > > folio = folio_next(folio); > > > } > > > > > > - while (left >= (ssize_t)folio_size(folio)) { > > > + while (left && left >= (ssize_t)folio_size(folio)) { > > > set_bit(PG_dcache_clean, &folio->flags); > > > left -= folio_size(folio); > > > folio = folio_next(folio); > > > > I've been thinking about this and I think this is the right fix for the > > wrong reason. I don't understand how it can produce the failure you > > saw, but we shouldn't be calling folio_next() if left is zero, let alone > > calling folio_size() on it. So I'd rather see this fix: > > > > while (left >= (ssize_t)folio_size(folio)) { > > set_bit(PG_dcache_clean, &folio->flags); > > left -= folio_size(folio); > > + if (!left) > > + break; > > Given that set_bit() involves atomics, wouldn't it be better if this > had been written as: > > while (left >= folio_size(folio)) { > left -= folio_size(folio); > set_bit(PG_dcache_clean, &folio->flags); > if (!left) > break; > > folio = folio_next(folio); > > } > > That likely means that folio_size() will only be evaluated once per > loop rather than twice. I may be wrong though, I didn't check the > generated code. I'd really like it if gcc did notice that folio_size() could be CSE. Unfortunately, I don't think it can. +long rmk(struct folio *folio, long size) +{ + while (size >= folio_size(folio)) { + size -= folio_size(folio); + folio_set_workingset(folio); + if (size < 0) + return size; + folio = folio_next(folio); + } + + return size; +} 000039d4 <rmk>: 39d4: e92d41f0 push {r4, r5, r6, r7, r8, lr} 39d8: e1a04000 mov r4, r0 39dc: e1a05001 mov r5, r1 39e0: e3a06a01 mov r6, #4096 @ 0x1000 39e4: e3a07020 mov r7, #32 39e8: ea000010 b 3a30 <rmk+0x5c> 39ec: e5943000 ldr r3, [r4] 39f0: e1a01004 mov r1, r4 39f4: e3a00009 mov r0, #9 39f8: e3130040 tst r3, #64 @ 0x40 39fc: 03a03a01 moveq r3, #4096 @ 0x1000 3a00: 15d43020 ldrbne r3, [r4, #32] 3a04: 11a03316 lslne r3, r6, r3 3a08: e0455003 sub r5, r5, r3 3a0c: ebfffffe bl 0 <_set_bit> 3a0c: R_ARM_CALL _set_bit 3a10: e3550000 cmp r5, #0 3a14: ba00000c blt 3a4c <rmk+0x78> 3a18: e5943000 ldr r3, [r4] 3a1c: e3130040 tst r3, #64 @ 0x40 3a20: 03a03020 moveq r3, #32 3a24: 15d43020 ldrbne r3, [r4, #32] 3a28: 11a03317 lslne r3, r7, r3 3a2c: e0844003 add r4, r4, r3 3a30: e5943000 ldr r3, [r4] 3a34: e3130040 tst r3, #64 @ 0x40 3a38: 03a03a01 moveq r3, #4096 @ 0x1000 3a3c: 15d43020 ldrbne r3, [r4, #32] 3a40: 11a03316 lslne r3, r6, r3 3a44: e1550003 cmp r5, r3 3a48: 2affffe7 bcs 39ec <rmk+0x18> 3a4c: e1a00005 mov r0, r5 3a50: e8bd81f0 pop {r4, r5, r6, r7, r8, pc} Certainly seems to me like it's calculating folio_size() twice. And actually it's redone the ordering to put the calculation after the call to set_bit! > Also, I'm wondering what that ssize_t cast is doing there - "left" > is a size_t, which is unsigned. folio_size() returns a size_t, so > is also unsigned. Why convert folio_size() to a signed number to > then be compared with an unsigned number? Because earlier we did: + if (offset) { + left -= folio_size(folio) - offset; + folio = folio_next(folio); + } so left might now be negative. If we did an unsigned comparison, we'd go round this loop. Er. And the fix from Marek didn't accommodate this problem. So we need a fix-fix: if (offset) { left -= folio_size(folio) - offset; + if (left <= 0) + return; folio = folio_next(folio); } Marek, can you do the honours here? > Or did "left" get converted to ssize_t along with the folio > conversion? > > Even if it did, how could "left" be negative (except through casting > a large positive number as "size" that in 2's complement would be > negative after casting to "left") ? > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 70cb7e63a9a5..02250106e5ed 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -718,7 +718,7 @@ static void __dma_page_dev_to_cpu(struct page *page, unsigned long off, folio = folio_next(folio); } - while (left >= (ssize_t)folio_size(folio)) { + while (left && left >= (ssize_t)folio_size(folio)) { set_bit(PG_dcache_clean, &folio->flags); left -= folio_size(folio); folio = folio_next(folio);