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;
References: <6e77914685ede036c419fa65b6adc27f25a6c3e9.1660635033.git-series.apopple@nvidia.com>
 <CAC=cRTPGiXWjk=CYnCrhJnLx3mdkGDXZpvApo6yTbeW7+ZGajA@mail.gmail.com>
 <Yvv/eGfi3LW8WxPZ@xz-m1.local>
User-agent: mu4e 1.6.9; emacs 27.1
From:   Alistair Popple <apopple@nvidia.com>
To:     Peter Xu <peterx@redhat.com>
Cc:     huang ying <huang.ying.caritas@gmail.com>, linux-mm@kvack.org,
        akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
        "Sierra Guiza, Alejandro (Alex)" <alex.sierra@amd.com>,
        Felix Kuehling <Felix.Kuehling@amd.com>,
        Jason Gunthorpe <jgg@nvidia.com>,
        John Hubbard <jhubbard@nvidia.com>,
        David Hildenbrand <david@redhat.com>,
        Ralph Campbell <rcampbell@nvidia.com>,
        Matthew Wilcox <willy@infradead.org>,
        Karol Herbst <kherbst@redhat.com>,
        Lyude Paul <lyude@redhat.com>, Ben Skeggs <bskeggs@redhat.com>,
        Logan Gunthorpe <logang@deltatee.com>, paulus@ozlabs.org,
        linuxppc-dev@lists.ozlabs.org, Huang Ying <ying.huang@intel.com>,
        stable@vger.kernel.org
Subject: Re: [PATCH v2 1/2] mm/migrate_device.c: Copy pte dirty bit to page
Date:   Wed, 17 Aug 2022 11:49:03 +1000
In-reply-to: <Yvv/eGfi3LW8WxPZ@xz-m1.local>
Message-ID: <871qtfvdlw.fsf@nvdebian.thelocal>
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?wsAn2SVxSqGIKJ9LCMnCHs1bTNFoawLOStQIgOQHbdXPVZsQhVXJymH60gga?=
 =?us-ascii?Q?+dgvKgwAHnzc7mNPiixj8oTtJ7Jc+Ivd/Jut7hkNeAgSioWUxAzdIVHea9i9?=
 =?us-ascii?Q?kDDjrUzbprdazSzq9zl0vNzJKaAo6W3iJTqvImmd9SSQ/pCa8iEMkykN0aZ4?=
 =?us-ascii?Q?jpemoX5XQ5MM+AgIxvUe/VJY9gKXWch14nYd3FC7P/A8r8Ja5ZavQW/+M1we?=
 =?us-ascii?Q?ZjI5NoPHvGNwIMSk0oqE+LoMqrfsHDJfaiVfhPeMw+xnUwOaFU6sWON8ACaW?=
 =?us-ascii?Q?X2Z8aKBjcIZnO/aEBBVBWH7EPQDLy0u32FwAFZ5Hsvu9w+6DsPe2AIS5k3c9?=
 =?us-ascii?Q?4kJ5+RdNssaa5lSvEtz7TmUj0G+H5YroWlSjvLLo7SQ1F+EFX+Qr9/i2WuNc?=
 =?us-ascii?Q?nx5Up/gvaIc3UAkbdlGMTsaCdel3ERMaTp5A9YNzBSCMmhC01A+QM0jIUH0Z?=
 =?us-ascii?Q?41/f5fKTZqq5VA5Z1fLH1OuVwkVEy95O7Thw/7Sqxd0jRa6hVGuc7adq3G0f?=
 =?us-ascii?Q?k3HiRxRisEJ/Tw+WF/FKXWZmiHBL0sdxf+wgPvoa17rdsPk7Aay/wRfIlv0Z?=
 =?us-ascii?Q?mu59zucgR+HuamDKPpOofoP6pw7qpQ2wgRDkXC6CjX81jm6qr3TXxdjmITwA?=
 =?us-ascii?Q?0Lx9cXAzt1+FTdmBw7tO9dSZtF7ugfQMVkWFUuT1/hW9lc/OdXoyesRy4i88?=
 =?us-ascii?Q?UQETfahgDTbYX6KIVEA3YbsEyqZ39WT49nH9AEARtXMT0YF8eZePLAKhBl2G?=
 =?us-ascii?Q?4Fm80+o7frSqjLJHZGNwmApu/bF2h1eyrAcLeAVdlcLUSegLmPumb4KokWtS?=
 =?us-ascii?Q?K2HWgHKkihw1vp/OgOQfuNDKRz+MUcBTGM+yWWDjmU6AjOcQ/gcONTGQ/LhA?=
 =?us-ascii?Q?02qzDE3oKwnNXsu/xMinbnN6UUXZ81ch+tBL6OjIkbbj+1Bm196zrLF68wt2?=
 =?us-ascii?Q?YWzq96CAfsX62GFsGGgtKNAknaDz0CVkP5oe/5yHEH48nI32D01G62DIeDy+?=
 =?us-ascii?Q?e9OxH03a+M8IHXS9zG5xRtZAOFMgGY7r3L+V8OMFcbbPtcp/hVxwTzm4W63z?=
 =?us-ascii?Q?yycJUfaPZO4ODsgCqVvDIPpj0JLprz5Pr/96k7BF3qndynYxLOpd1+QbBnq4?=
 =?us-ascii?Q?A6U+SHYz2zrWzwH8WnGQLB01o7cqnuF01nCkQENK091uoanf5Ej7Bz5On5Ao?=
 =?us-ascii?Q?kx0OMT+eX+seWPX3iA6Kf0t55QK84uogOSl5OAhVg61ht4Dn6eS0SpaoFfJ1?=
 =?us-ascii?Q?/KEBLvVYQG+hYyInhzijSDnET0JDLJSm/nidLMoQcs35RYcRNnphZkKdcONL?=
 =?us-ascii?Q?OWRmPoz9nJeCBaxntRbt14zwICia6z4VqHARWdQ9ZyGoIVnoUg7okGfEgIhc?=
 =?us-ascii?Q?+SGBiPdXVud8XwY93egqxRTiotz8oraht2aLQoY+O2nBKuOVLDGjEH/4Mxp/?=
 =?us-ascii?Q?j0rPEFByU4vTgNnfJCGdqI8yp8PDX399YxIKtklCXYIdDEtWeKHVqdD2Tl4i?=
 =?us-ascii?Q?f6R8G0BUQIjCHUYtVYCtTdFbM1lGhO2nVZvZPTgq812IRqShT7Gj1SnEmjxn?=
 =?us-ascii?Q?/wuGOsxVF3zl5yYeCUF02GD99PrwXouKLmiHYPUE?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 079f6b67-3404-4939-748c-08da7ff3720e
X-MS-Exchange-CrossTenant-AuthSource: BYAPR12MB3176.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Aug 2022 01:54:39.5239
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 6ERdGm4UwuXUO4du3bOsaXbD/0m2/jxM9wZ6IGnHvDS0g1iO1qVb8gUKn4/jCnwLQNWw1BIQpSvPw4eMnW1+vw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR12MB4276
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no 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


Peter Xu <peterx@redhat.com> 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?

> 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.

 - Alistair

> Thanks,