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 EC79EC77B7A for ; Fri, 19 May 2023 19:54:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 75F9A900006; Fri, 19 May 2023 15:54:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 70FAA900003; Fri, 19 May 2023 15:54:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5D7A3900006; Fri, 19 May 2023 15:54:42 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 4B25B900003 for ; Fri, 19 May 2023 15:54:42 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id E8032160B4B for ; Fri, 19 May 2023 19:54:41 +0000 (UTC) X-FDA: 80808057162.22.4C00281 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf29.hostedemail.com (Postfix) with ESMTP id 22E88120009 for ; Fri, 19 May 2023 19:54:39 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="P8KMX/ai"; spf=pass (imf29.hostedemail.com: domain of sj@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sj@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1684526080; 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:dkim-signature; bh=CtsUnGfi9YOfY881KZwMb/Kr+CMhHhSbL/apqkCYx/s=; b=MEZP37zXXCwnhEoeh5bKJJSFqstP/GnGfyCT8H+sUPaKbVsMojP2buVPeJiy8eJmv50Twq f2VK19r0oQPSpo8VMDs6k+giOXuEK9SbgV9hVmMUh+f9iUleU9+U+CxMc7gphvtLNoTV3c fbp/61bTIAzlYKhJAlfnFBjV6jVPGdk= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1684526080; a=rsa-sha256; cv=none; b=aRk+bDZwqGdfkxzfvHTltney2J4tMFPXyPNZPK1C7UgNNbQ48dX5XnZnNNAvWyw4XJOOyc hUU1ViREQuWbj4nDe02x26fg0Ywh2sg07Y6PLofzIpBa371SBcaqx9mporHdVDfLlCGO1A LOjtLqWkuueeSzSwhQUQ0kWd/zpTXRE= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="P8KMX/ai"; spf=pass (imf29.hostedemail.com: domain of sj@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sj@kernel.org; dmarc=pass (policy=none) header.from=kernel.org Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id EB164658E5; Fri, 19 May 2023 19:54:38 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 689D2C433D2; Fri, 19 May 2023 19:54:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1684526078; bh=Ns58burMT4c0EWSkncF1uaZbw4bRG/N+tROJuYPPCN0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=P8KMX/ai33gl6aKSfnz60iRj7pvKbWY8KlEhg+AuilM3+bx0LTdJM8E8IDhtFAgYB xp4kwuVLeseRxiseh4Z+d09U4VU9Xbx3ikgnmARoV3hM+ZTjjyiW5Hie+hZWsoA/ZC hFoGkh+eH8c1Vy/MDAtB2AWJrXrzojr0dZJJKvfhltDuImPO57LK57qqQgQ3lALLT8 dp6+Ea7/5Y7CNVHnZ6Tx4wmVQMPIHOCZiCMfXAPwuoCVxAeGclnGM8UDaqMy3Aur+s QPMGAdzWtofTat96uz60Q7fRCiqhl6wSSkgtYKNXdCeJafzPWxTJPnkHXnBq6T1PY4 WYiDOwjNPmAuQ== From: SeongJae Park To: Ryan Roberts Cc: Yu Zhao , Andrew Morton , SeongJae Park , Christoph Hellwig , "Matthew Wilcox (Oracle)" , "Kirill A. Shutemov" , Lorenzo Stoakes , Uladzislau Rezki , Zi Yan , linux-kernel@vger.kernel.org, linux-mm@kvack.org, damon@lists.linux.dev Subject: Re: [PATCH v2 2/5] mm: damon must atomically clear young on ptes and pmds Date: Fri, 19 May 2023 19:54:35 +0000 Message-Id: <20230519195435.90394-1-sj@kernel.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <88c125fd-2294-4650-65b1-6be56589120e@arm.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Stat-Signature: cpezowoax7f1au86ys511o7hcroq4tne X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 22E88120009 X-Rspam-User: X-HE-Tag: 1684526079-557306 X-HE-Meta: U2FsdGVkX18mfCqO9WT6rvDpi00cQS2ZQ2nra36OLIVp+0v/Ogk031kgcLPg5jKOJI8qRfaOLhGhLaCFj3itjbfPl8DlH7wiqAwdvsNmp80h2kirRrqLmByk2WD59IJ3EuLzJeqXmeddm3DkF4hirDZpJtBm8ePdBm9dKu1XI9R9qt+gTdxUjrp6giPZ4DUvlLm8VB+Rz02puJr3jYy1gGOlStAiOxA56Qy7j7KlQIs8ZIfTXiddw/rjuWXpIQ3DF51absHLiK+J8BhSR038Y795obfFum71OoFV1KqHvusKYnydydyuCceiBazFPeiQFROVmGvCDyt2xzzuWlIaPRfoOiESrBuMqLvsmdsAmItkj6JQx2bmNV6HO5s7JiQL7hD/CGe53rkElOxGcte2uvlvYOW5p2nInZLxaSF9D2SQG2Wgl2sbL/ty+0UG1B1MqYnUS9pfTAeoDQy3sCETZ8Vw7MIVePYqkIORl58skM0BdnzfIgbVo99BwoemZsbiLJJ/u3VK9J/RhN0/7BNMukNpxVAjJeq8+hxCqiA/uBwi6PJOJriyx36F1TUZLjL5jBs/afrkauoCPwWhZFLABdQjbKtuWJiwyYCSkWnzhpSHkv1d5JDnRRGbI0fDnAKV42Sn8uY8P9ulDCnpDBusH/84/tzdg1fmwc8GlvZkjCmAQBDHY3xqA9njDzrMNZCZyUYRVeEowO6KIMgrE6Xs37HjU/oXcSvfU2/5S9UCHvyTVI0h6+TX4h1yMQlg4eL6/c9MXI22YxdeD1jU+DNeIcumklBI//nWKcIH/AWplkIKNGvGawAyYftWel5lvec7O7AcwCgKC4oaYwZewAk5M5yamuhNTm0Da1HHaARAVlllVmT4G0b+sQJxe95Z8Z7tlncAXmxGMeSsDHySKXG+fyh1LygHUTo9x3WUBmu8tXs4dRnu8ZY9AG4iyqjVd/5yBo0V4nArz3xD1IMf33+ XqjrGfUH +t/3LSChnwkgcG4iPv+lpaKeNYUCqXr9uHnS+aW04ZfcCaMt/yO99kFqXbZfvb8b1cFgSjEsroNf+TKT7EQB7uQN4M5yJJJXpYR9Q7t3KjXDRTmktRbo3zmuEY8TBHQ/fCnTjDnQiQDRwVIoCj+eTD1ULEnT8VN6SWjhyVCarAK7vI+i3zabQRFmxyi4GczbOKabR3/7F+uRmGMnT/zoQer64ENlezMSaDhBwMPc5l6n5/rnbKXiiyYnVk2zaei4QUI9CZziGAcA00upaJz5K9TKs9lvgDAYyJAFlrGu8OLeN3QlY/oJjrHzzhnMubXoIASMzraO8wtlx9GOkFaUHZAmcHY2JAmdT5Z7DGuHsQnW1j3W7Gg7XSXcXKw== 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 Fri, 19 May 2023 10:02:48 +0100 Ryan Roberts wrote: > On 19/05/2023 00:19, Yu Zhao wrote: > > On Thu, May 18, 2023 at 5:07 AM Ryan Roberts wrote: > >> > >> It is racy to non-atomically read a pte, then clear the young bit, then > >> write it back as this could discard dirty information. Further, it is > >> bad practice to directly set a pte entry within a table. Instead > >> clearing young must go through the arch-provided helper, > >> ptep_test_and_clear_young() to ensure it is modified atomically and to > >> give the arch code visibility and allow it to check (and potentially > >> modify) the operation. > >> > >> Fixes: 46c3a0accdc4 ("mm/damon/vaddr: separate commonly usable functions") > > > > This should be a separate patch, since it's independent from what the > > series tries to do. > > > > And that patch should cc stable, since it fixes user data corruption. > > Fair point. The first 3 patches are fixes for issues I found during the > conversion. So if you're ok with it, I'll split into 2 series; the first 3 > patches in the first, and the conversion to ptep_deref() (or ptep_get(), as per > your comments in the other thread) in the second. I guess the whole first series > should go to stable. > > Let me know if you disagree. > > > > >> Signed-off-by: Ryan Roberts > >> Reviewed-by: Zi Yan > >> --- > >> mm/damon/ops-common.c | 16 ++++++---------- > >> mm/damon/ops-common.h | 4 ++-- > >> mm/damon/paddr.c | 4 ++-- > >> mm/damon/vaddr.c | 4 ++-- > >> 4 files changed, 12 insertions(+), 16 deletions(-) > >> > >> diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c > >> index cc63cf953636..acc264b97903 100644 > >> --- a/mm/damon/ops-common.c > >> +++ b/mm/damon/ops-common.c > >> @@ -37,7 +37,7 @@ struct folio *damon_get_folio(unsigned long pfn) > >> return folio; > >> } > >> > >> -void damon_ptep_mkold(pte_t *pte, struct mm_struct *mm, unsigned long addr) > >> +void damon_ptep_mkold(pte_t *pte, struct vm_area_struct *vma, unsigned long addr) > >> { > >> bool referenced = false; > >> struct folio *folio = damon_get_folio(pte_pfn(*pte)); > >> @@ -45,13 +45,11 @@ void damon_ptep_mkold(pte_t *pte, struct mm_struct *mm, unsigned long addr) > >> if (!folio) > >> return; > >> > >> - if (pte_young(*pte)) { > >> + if (ptep_test_and_clear_young(vma, addr, pte)) > >> referenced = true; > >> - *pte = pte_mkold(*pte); > >> - } > >> > >> #ifdef CONFIG_MMU_NOTIFIER > >> - if (mmu_notifier_clear_young(mm, addr, addr + PAGE_SIZE)) > >> + if (mmu_notifier_clear_young(vma->vm_mm, addr, addr + PAGE_SIZE)) > >> referenced = true; > >> #endif /* CONFIG_MMU_NOTIFIER */ > > > > Use ptep_clear_young_notify(). Similar below. > > This looks sensible but I'd like clarification from SeongJae: Are you happy for > me to do this refactoring as part of the patch? Yes, I would be happy for that :) Nevertheless, because that's a refactoring rather than a fix, I'd be happier if you could do that with a separate patch. Thanks, SJ > > Thanks, > Ryan >