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 C2492C32772 for ; Wed, 17 Aug 2022 07:17:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 335B48D0002; Wed, 17 Aug 2022 03:17:23 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2E3296B0074; Wed, 17 Aug 2022 03:17:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1AC3C8D0002; Wed, 17 Aug 2022 03:17:23 -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 0E14D6B0073 for ; Wed, 17 Aug 2022 03:17:23 -0400 (EDT) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id D6E5C4059F for ; Wed, 17 Aug 2022 07:17:22 +0000 (UTC) X-FDA: 79808228724.01.65EF1A1 Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by imf24.hostedemail.com (Postfix) with ESMTP id 013031801E4 for ; Wed, 17 Aug 2022 07:17:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1660720642; x=1692256642; h=from:to:cc:subject:references:date:in-reply-to: message-id:mime-version; bh=ZS9/pnsjWMHpVVY5COIkT1/+PC3cjs3sNi1oR7AcMOA=; b=HMx+IFFgw0d33d1zicd5mJ+G0DzYEt5oMRAN6io7V2CqOTYqJoEED8tP RA4HiFV3MjsvDEG8DQjz+Zb1boLAdxrciTshLGYFxHgWVGzPkCSsmo7xA JOADs62meFs/pZyWIrMG5BZYkzk/NsCmRGAai2N7C+mbmUQBBRno1RUU6 clM1icFzDoI7EtLVWIa+PGdIrjPXGZfEVfdGrCnb83X4w95q/FX6J/Nft 5dOWdhej72HY3X3qeQ50t5KdDeP0y8ciHyUVgcXBQEc1LXR8DLDrpL/zj FtvBVftc+yz+H/0lvPvom5ZuOMyV2IyLKNPiwErm2YUZ8QxpTVzt+Uhtf w==; X-IronPort-AV: E=McAfee;i="6400,9594,10441"; a="354168111" X-IronPort-AV: E=Sophos;i="5.93,242,1654585200"; d="scan'208";a="354168111" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Aug 2022 00:17:20 -0700 X-IronPort-AV: E=Sophos;i="5.93,242,1654585200"; d="scan'208";a="749605160" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.238.208.55]) by fmsmga001-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Aug 2022 00:17:16 -0700 From: "Huang, Ying" To: Alistair Popple , Nadav Amit Cc: Peter Xu , huang ying , , , , "Sierra Guiza, Alejandro (Alex)" , Felix Kuehling , Jason Gunthorpe , John Hubbard , David Hildenbrand , Ralph Campbell , Matthew Wilcox , Karol Herbst , Lyude Paul , Ben Skeggs , Logan Gunthorpe , , , Subject: Re: [PATCH v2 1/2] mm/migrate_device.c: Copy pte dirty bit to page References: <6e77914685ede036c419fa65b6adc27f25a6c3e9.1660635033.git-series.apopple@nvidia.com> <871qtfvdlw.fsf@nvdebian.thelocal> <87o7wjtn2g.fsf@nvdebian.thelocal> Date: Wed, 17 Aug 2022 15:17:04 +0800 In-Reply-To: <87o7wjtn2g.fsf@nvdebian.thelocal> (Alistair Popple's message of "Wed, 17 Aug 2022 15:41:16 +1000") Message-ID: <87tu6bbaq7.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=none ("invalid DKIM record") header.d=intel.com header.s=Intel header.b=HMx+IFFg; spf=pass (imf24.hostedemail.com: domain of ying.huang@intel.com designates 134.134.136.31 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1660720642; a=rsa-sha256; cv=none; b=E1Mz8m8IFxsF3pFs2HU0yMXCX7oR4uLPmIw2r70j4vxUZ+2JLCNjN0V3SBCh6UYNNGMa8v c/Xt0H4/9IwclkAs/1QIvpKClo5ca11OVaQuSwhtc2bYIr9/I8Bv6v7MYMohmBPvwVJvvf 4xA718YG95ZU7nDCWMSA3fjdEy0RHXw= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1660720642; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=ZLQLzbkeTWolQQly/XsT1GIEwFhogIcDpKF6QPEFsGw=; b=yHqeluUU0eypsvU4ki9bzXFAH146KPc58aDLdEisFk1Xr97IlVfdba6iFJADrEo4H4lYuv SJH8aCfTBrsWWGVVPQNC4IykpdcbTNUmiCwtP8yNfr+lPL6SGVyCFr+TShhWHiRfD2Vr0e pJ7yYjTMsObiBP929tC5QpC/ugrxeXs= Authentication-Results: imf24.hostedemail.com; dkim=none ("invalid DKIM record") header.d=intel.com header.s=Intel header.b=HMx+IFFg; spf=pass (imf24.hostedemail.com: domain of ying.huang@intel.com designates 134.134.136.31 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com X-Rspam-User: X-Rspamd-Server: rspam12 X-Stat-Signature: nqdtzpw17rq747okwbmwmbdtgq7sswfa X-Rspamd-Queue-Id: 013031801E4 X-HE-Tag: 1660720641-208378 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: Alistair Popple writes: > Peter Xu writes: > >> On Wed, Aug 17, 2022 at 11:49:03AM +1000, Alistair Popple wrote: >>> >>> Peter Xu writes: >>> >>> > On Tue, Aug 16, 2022 at 04:10:29PM +0800, huang ying wrote: >>> >> > @@ -193,11 +194,10 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp, >>> >> > bool anon_exclusive; >>> >> > pte_t swp_pte; >>> >> > >>> >> > + flush_cache_page(vma, addr, pte_pfn(*ptep)); >>> >> > + pte = ptep_clear_flush(vma, addr, ptep); >>> >> >>> >> Although I think it's possible to batch the TLB flushing just before >>> >> unlocking PTL. The current code looks correct. >>> > >>> > If we're with unconditionally ptep_clear_flush(), does it mean we should >>> > probably drop the "unmapped" and the last flush_tlb_range() already since >>> > they'll be redundant? >>> >>> This patch does that, unless I missed something? >> >> Yes it does. Somehow I didn't read into the real v2 patch, sorry! >> >>> >>> > If that'll need to be dropped, it looks indeed better to still keep the >>> > batch to me but just move it earlier (before unlock iiuc then it'll be >>> > safe), then we can keep using ptep_get_and_clear() afaiu but keep "pte" >>> > updated. >>> >>> I think we would also need to check should_defer_flush(). Looking at >>> try_to_unmap_one() there is this comment: >>> >>> if (should_defer_flush(mm, flags) && !anon_exclusive) { >>> /* >>> * We clear the PTE but do not flush so potentially >>> * a remote CPU could still be writing to the folio. >>> * If the entry was previously clean then the >>> * architecture must guarantee that a clear->dirty >>> * transition on a cached TLB entry is written through >>> * and traps if the PTE is unmapped. >>> */ >>> >>> And as I understand it we'd need the same guarantee here. Given >>> try_to_migrate_one() doesn't do batched TLB flushes either I'd rather >>> keep the code as consistent as possible between >>> migrate_vma_collect_pmd() and try_to_migrate_one(). I could look at >>> introducing TLB flushing for both in some future patch series. >> >> should_defer_flush() is TTU-specific code? > > I'm not sure, but I think we need the same guarantee here as mentioned > in the comment otherwise we wouldn't see a subsequent CPU write that > could dirty the PTE after we have cleared it but before the TLB flush. > > My assumption was should_defer_flush() would ensure we have that > guarantee from the architecture, but maybe there are alternate/better > ways of enforcing that? >> IIUC the caller sets TTU_BATCH_FLUSH showing that tlb can be omitted since >> the caller will be responsible for doing it. In migrate_vma_collect_pmd() >> iiuc we don't need that hint because it'll be flushed within the same >> function but just only after the loop of modifying the ptes. Also it'll be >> with the pgtable lock held. > > Right, but the pgtable lock doesn't protect against HW PTE changes such > as setting the dirty bit so we need to ensure the HW does the right > thing here and I don't know if all HW does. This sounds sensible. But I take a look at zap_pte_range(), and find that it appears that the implementation requires the PTE dirty bit to be write-through. Do I miss something? Hi, Nadav, Can you help? Best Regards, Huang, Ying [snip]