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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 8F420C001DE for ; Mon, 7 Aug 2023 22:46:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=RcRtKZ3YVSQHzk+0fa/msQ3SHqRjdehe1HeE1YI2HLE=; b=A+MRNtfhqKNsYZ NxWpF+611QqYZYdINcWenaRgZbPvzYvfNXHtqyJ7rwgfJ6K5Kz9Wp9Qz3FPaYlj3pWsXQEiWwgu1X JJwihOEiNJVnbnNFzM3c9ycz1+HWI0WgAQ6ubJQHhu291C2xjSRSdEEf0duRvxeWSuNNcGuvM6zMC 0aj1wKbFYAxaLScaK4Qna9dsOUvY8TiLVQ8ehHLPzFs55TN9skm+bV1y8SEC97DAulL7O6zmeu7mL /HoZ9MCfdK/luxnaxTWDuWnFiHTdKfHRQRwSDvynrKd+DQ+ZR9H5KFqk9lD2FoV+/g9i+jzr1r/Ka 237Z5bFMdwUpXQKG3D+A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qT8zN-0011KR-30; Mon, 07 Aug 2023 22:46:13 +0000 Received: from [2001:4d48:ad52:32c8:5054:ff:fe00:142] (helo=pandora.armlinux.org.uk) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qT8zL-0011Jg-0l for linux-arm-kernel@lists.infradead.org; Mon, 07 Aug 2023 22:46:13 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=DMAmkq+Z8+wqHN2hP7Ew/2ByzL7++kUWTu18KZLXSgw=; b=NVMn7DddBbFdq1vo2tm/Nu9M3o 6vfm9FQpSkkV31ClPcghujps2gR8aT0/hfPlZiGXbGQK2V7NXQQee+90QD17TzZuaSBxwTKXNcEaS xI+yJjIXwr/xatIfPSQzk/8NVotjWJ9tsu4HBlIqKPpn24JUdOVW9rTgfs2wHznIgabHkjN4Esx47 jgYAEv6K/H3z74YzzunIRpixbMp9GwCQ5LDSmE+AT2N+gCviaLt3PuJEA5nb1G1TbDzBv+tEHJ4hc XuhfKCx0aTZ6F4CHZbx9t08mcjromf0foM4lFo+NweciAHCv6Y5uAKIDgJBen0GlyRiIF3LYTK3+N 6ESlSrdw==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:58376) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1qT8zG-0005r6-2d; Mon, 07 Aug 2023 23:46:06 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1qT8zF-0007RD-MZ; Mon, 07 Aug 2023 23:46:05 +0100 Date: Mon, 7 Aug 2023 23:46:05 +0100 From: "Russell King (Oracle)" To: Matthew Wilcox Cc: Marek Szyprowski , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Andrew Morton , Mike Rapoport Subject: Re: [PATCH] arm: dma-mapping: fix potential endless loop in __dma_page_dev_to_cpu() Message-ID: References: <20230807152657.1692414-1-m.szyprowski@samsung.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230807_154611_301914_05FFF71A X-CRM114-Status: GOOD ( 22.04 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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") ? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel