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 AB301ECAAA3 for ; Fri, 26 Aug 2022 00:56:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A03BF6B0074; Thu, 25 Aug 2022 20:56:03 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9B1D66B0075; Thu, 25 Aug 2022 20:56:03 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8796D940007; Thu, 25 Aug 2022 20:56:03 -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 799A46B0074 for ; Thu, 25 Aug 2022 20:56:03 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 2695C1A056C for ; Fri, 26 Aug 2022 00:56:03 +0000 (UTC) X-FDA: 79839927006.03.944BC7A Received: from NAM02-DM3-obe.outbound.protection.outlook.com (mail-dm3nam02on2087.outbound.protection.outlook.com [40.107.95.87]) by imf16.hostedemail.com (Postfix) with ESMTP id 9F74F18000A for ; Fri, 26 Aug 2022 00:56:02 +0000 (UTC) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UPIIiNhCsZ+rA/QJ6wkuJ90VbJw274V6sOMGkvb93sPh6EdkLLreO84gV8acngbRtY9vLnSK114nTWYX5kfULGpZuGBF7wSh+DtoQQu1Pj3aA9XxuYg4TgARNmK80E4znZnMVsZGiLKHwbgJ2OM7XxGAUdZ0weR/dUZg037wLQxArGjWQPXAGINqQ52EBwhRYbuAIPqJzpydxw5z3LbnY/IJeXY+HRslT0BQDmzfOMixVR92pD9/2MYbz1qMxWwzP48x25nhROgo77jJFlboa18o37FPL4mSiUdP6YHYrIdwy4I5eED4kusmBKN/CwrL7vWtp/sxkuFkpyWkQ4esJQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=ubw+1Hl8ZNNJZrKj3aEFvm4Rv9aKgvy9JkbTsHmHIHk=; b=O7hEQ2IoqftrMipbMJe2PGdMKWy8TCXpANwG+m+6qM1MAmFfbUcJYAuOB6/lBzyhAi1jdPdzw2WQQrtuTp/dIPveFMJNz1DY+6C1jw8XGV/7UHx7Hce/mRPdXlffMvH61RmdpR8YhS7eQ4U7qRZRw+zSQ/BRm6dcnlCeDRYTs/668mpXSryjCPO9aF5uNWCqVosxAm5wcHI4AEYoCfLbaVF44SOoFb+1XSuDu4VP6LDIyX/8mQwoWNtgmuVheueyxDW/tLcEcZIryWJXiIwuRS7OzZsJl5RkVmHUtc6AQGqmL+3YRUD9CErZwQeMWq79z+cHhOufQEYV6zbIhFne+Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ubw+1Hl8ZNNJZrKj3aEFvm4Rv9aKgvy9JkbTsHmHIHk=; b=gH9RyGSxvjD/zOVlYZZf4PaczvPXTnUI3+VXUY1ymXBkhOKrCfxA3txnMNjEOQPD58ZXNMjPf6iaYbE9Axi/RI3nLJwHzk558MQojaQmT5QvjqOeAgY2NFg6SEKGNe37KfHox1N2QBrr2EII/elsDgSVst9rH56F+wA7WsJtkCaw44kfS7EcSkusROADIGtyuMffo2eTgVhnxU+46UaL6Gf9mIMVL8YGyjwDkT5tX1Enyglss9lrBsZUmWb50brxZhgCBzSVWfqjh5c09y5g/4S3gmi3OdcMbDDI/7BYF6JqfiZWNXysuehv/nA0RZgdgqi1gQ2Vf6+1vPzGC9QrFg== Received: from BYAPR12MB3176.namprd12.prod.outlook.com (2603:10b6:a03:134::26) by DM6PR12MB4435.namprd12.prod.outlook.com (2603:10b6:5:2a6::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5566.15; Fri, 26 Aug 2022 00:56:00 +0000 Received: from BYAPR12MB3176.namprd12.prod.outlook.com ([fe80::7432:2749:aa27:722c]) by BYAPR12MB3176.namprd12.prod.outlook.com ([fe80::7432:2749:aa27:722c%7]) with mapi id 15.20.5546.022; Fri, 26 Aug 2022 00:55:59 +0000 References: <20220825014905.977168-1-apopple@nvidia.com> User-agent: mu4e 1.6.9; emacs 27.1 From: Alistair Popple To: Peter Xu Cc: linux-mm@kvack.org, Andrew Morton , peterx@redat.com, John Hubbard , Ralph Campbell Subject: Re: [PATCH] mm/migrate_device.c: Fix a misleading and out-dated comment Date: Fri, 26 Aug 2022 10:34:15 +1000 In-reply-to: Message-ID: <877d2v7rhf.fsf@nvdebian.thelocal> Content-Type: text/plain X-ClientProxiedBy: SJ0PR05CA0086.namprd05.prod.outlook.com (2603:10b6:a03:332::31) To BYAPR12MB3176.namprd12.prod.outlook.com (2603:10b6:a03:134::26) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 73692d78-5e5a-4acd-c1e2-08da86fdbdb5 X-MS-TrafficTypeDiagnostic: DM6PR12MB4435:EE_ X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Br+U4U09vweZvQuW5vtMhMjX7wipPqWlTqX9jG6bu0KLV24b//U+PIpAhJd0I1gxeivWh0gdDJGKMNxcgRSPSQ4YlBPJ/Y23YOv4LT/tPaNrE9DcrCJFUtb9mltlSs8Q/gHPZ8nvjZqb2u1mPn8WBJGAx0o2zfw8ZSJ4gmaFUgzCxNWLJoKlcj554AE1bsv7BDupuMzGs1s4bAGQ5b5rcM8qSBm+JtysYOyWIAm4nentwn3KhlFYm7ngoweMsnGO30cBIDR1NVQVNqjZSBMFPbQ5qTunGPAZxzaY6WE5cYCh/W2+7ZbdcEBvqDFF+ko/VC5R7WFL/yaQ8Fx/d0ggl3ecAiNnNwofu+NU43afUetBshpYvRV3bPgbPBDG2XcY1zY8diAnUAHiMDji3wuwYuHZazLpKxQ70zaK6/j/9uab34ODUf2sddQ/mdvkxooo9EKPfU4Chrq9eIa6RnE8aGgj9omgZ0NKNxJQdlMxtzUxUZNpw0IAjJ6f+/eVcTDgDh14aQ+PI3rVQ5Vi/cc9PMesJCsjABbQSWWpSNER8ZRpkmm5NYQ01oJZ44pf4kAkwnIOZJGS1y0FRKLcS8O+8dZ4gL4sSbBxGhkjQe7jCxkeDfv/Jq6tAFO/ZcE/RN6vtrc+GQ8DTLrHX4KX9GGZRMAK0Dk4Audd6Xtc54FY8ZiIvr8VH+nOwXaaTbkUUodLO46tKibt+/mqVMXD2Jk3fA== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BYAPR12MB3176.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230016)(4636009)(376002)(396003)(136003)(39860400002)(346002)(366004)(2906002)(6666004)(9686003)(107886003)(6506007)(26005)(6512007)(41300700001)(83380400001)(38100700002)(54906003)(6916009)(316002)(6486002)(478600001)(5660300002)(186003)(8676002)(66476007)(86362001)(8936002)(4326008)(66946007)(66556008);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?uEz2L0A2nVhGrh6fRXln2Eai0vcLttAOOjLuYpGK8+fzvPQnRm2KGABjM3y5?= =?us-ascii?Q?Numz9HaAlSV8Buhs+vuFYmx7637bbjnE+qQrNu7IPInsR6fiYISwg52VL/2Q?= =?us-ascii?Q?+P8PA0Xj0LHQWmqirKHw2RwHsd6zyGSO8pjzG9EtTz8NsI3qMLE2q6BM6uHP?= =?us-ascii?Q?OGcvzMwHFsn+vkT79Swn3WM0cGjZNFyfzY/2UH/7yWbo39VXK3f8Z6o9WtfR?= =?us-ascii?Q?09o9axXvDzUxQ+LBfevBM1MTyGXRl2Nt9bSCXsKdMXL/7vvej07fQ5TzsLT5?= =?us-ascii?Q?alZvmlqnn20oc0eVffo21aono3XclxVMa7oDShVwQHn1Mo4FWT97r/qtemmD?= =?us-ascii?Q?N8aGgpBm7n7pXmHzGNS7p0CCtZAz6XdD3A6SYrYZsvsbvMLhVNZ0pYF07QrY?= =?us-ascii?Q?wQY/lDeIheGhnk043SnDkpL7bZVo0J0oOkcgE7z1CUL5P9zlceJpMMoK6zBU?= =?us-ascii?Q?ImFqLdmxHOGFFekDjsFpjb6O6ExEdULtIEJ6DAH+FscD9KLyg+NdYN8OS+l8?= =?us-ascii?Q?t9kmTRhgcQyWV6YcNi/XmDdu5KcMwEW94dS5PdlNZe6PnjNJ6MdefrK6i/fE?= =?us-ascii?Q?SIbISjN+dGL0zjAaa2eqxWBIE4Fn5C2Pe96cU+4dcGh1IIb48bPZGvBwKTVF?= =?us-ascii?Q?JQjxN80p6/fiWJT8MxVjoGc8H8WmU/VJBPPIBQ6Ao9x32ffqE+JmNJx50t6Y?= =?us-ascii?Q?w7LRmFliRaoCp4emIVAJ/7JtU/t4b5W9Tb7fxjcUh6cuZRI9IJJE6oim8Xbx?= =?us-ascii?Q?GM4tzakLyjQbdPrh4aCru6w3pXnomvzk3r7DSRd8m5KFoIDdrHH6Z0xdE1/Z?= =?us-ascii?Q?hxwucAeleKB7CjuWMriXa+ge+k+G2V5mMdmyFG+0EUpC+17Gm7TVp4kqCD9w?= =?us-ascii?Q?XY8FEIIfWz00469wEZ/L6XvWyL7p3pjGjqxyqeP9wKLLRKi4R5uFukShx94I?= =?us-ascii?Q?x8gnUDO7suFj9fzbF2cSCP/hdqqllYtB5RcRiIA/6CNDawln91TspasZ5HCL?= =?us-ascii?Q?L0xw9LMs3TtVKmlsua9DFEPPg8mfImieI0RyNAt3m2sXHLJyklr9yASWiswA?= =?us-ascii?Q?hmY29PVzrTbFeEp1gZBhdlxPUMe04ZfCTLJXeNLr40fogTwC2oIZxeJWs6Vh?= =?us-ascii?Q?M1GI7NCv0HUrltejOI3J+pTPx0pTTKh0CIP4+mZ2qUnlP/52TcvtvCPrlaTh?= =?us-ascii?Q?i+XqvVocJHgT3q3Xyybn0fKnXxu24it3U41bwNJl9OBCx0py6FAdFm0DcCZA?= =?us-ascii?Q?nhEjKUtqEV9Nv0YqPCvPDXSHtYUDT7MOwutqFSgAhjyLilaqSeng6++rLK1d?= =?us-ascii?Q?ux+Fb+eeLfBCK7cUcLuIS/7TDryq/K2qLziO1EWS+mXylHBdjVuL7OjKVz1+?= =?us-ascii?Q?eQlLufVGS/aRX1exVVBXucRGhhC34xyqxm0O2fbW9+xQAWwuWuia2gc6n1MO?= =?us-ascii?Q?bJVaXNPN+WGtzPtq2mfLWY8PQ0QZqvcJ6Yw/Xu3q4SR05b/ghc3SH0b3fin4?= =?us-ascii?Q?l9RCzR+m9gxsGhNY+tKBuqygRdvtgxC062tr+L6Ufah3V4NNEXLbRuOcFbyU?= =?us-ascii?Q?o0+cStHbQ7YSxXE7/pDVUO5835MEYXJjfyPLXmhZ?= X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-Network-Message-Id: 73692d78-5e5a-4acd-c1e2-08da86fdbdb5 X-MS-Exchange-CrossTenant-AuthSource: BYAPR12MB3176.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Aug 2022 00:55:59.5718 (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: 44ugmpF6ylCEwTxRDxSpqdeFHryhUpQoyljLNwH4WaQpAK1ljHPLESDHVDkIYnQqB6kCb/CzTBNYAyrp9G6TUg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4435 ARC-Authentication-Results: i=2; imf16.hostedemail.com; dkim=pass header.d=Nvidia.com header.s=selector2 header.b=gH9RyGSx; arc=pass ("microsoft.com:s=arcselector9901:i=1"); spf=pass (imf16.hostedemail.com: domain of apopple@nvidia.com designates 40.107.95.87 as permitted sender) smtp.mailfrom=apopple@nvidia.com; dmarc=pass (policy=reject) header.from=nvidia.com ARC-Seal: i=2; s=arc-20220608; d=hostedemail.com; t=1661475362; a=rsa-sha256; cv=pass; b=K96c3yOkgAJomrCk+QxLvvJZ5wRwtuvbVTfBmyNZ6w+s5JhjSwwVG5PeHh0xSyHVKetLQX wLT5sl3Zlekyu8CK1i39gcmZxeKW80E/OoJq9LLySGlDVdX9NNBcJyDIz/bVaDfokEfRr3 iYnhE0CK2vek4q+6VHrs0WW8Ok/nAA4= ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1661475362; 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=ubw+1Hl8ZNNJZrKj3aEFvm4Rv9aKgvy9JkbTsHmHIHk=; b=Uwztkie5C9RQX/8ERDe7ON8ETgItL2QIGKeH11eMjcMNMM+2wYOe1Vj6FnIwgZcNT1ubVP C0SvylVP54ozhTm8EJW69k98yxShmkd+ENA2yIiGgQQQeH2lAfCQX8zfshwOjyr7TGK44h oyqMqQmmiyLdv9p+efvG1bhCgjqwMrI= X-Rspam-User: X-Rspamd-Queue-Id: 9F74F18000A X-Stat-Signature: jztwffy5yb3e8cu14fga74wsmmwj897z Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=Nvidia.com header.s=selector2 header.b=gH9RyGSx; arc=pass ("microsoft.com:s=arcselector9901:i=1"); spf=pass (imf16.hostedemail.com: domain of apopple@nvidia.com designates 40.107.95.87 as permitted sender) smtp.mailfrom=apopple@nvidia.com; dmarc=pass (policy=reject) header.from=nvidia.com X-Rspamd-Server: rspam01 X-HE-Tag: 1661475362-895264 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: Peter Xu writes: > On Thu, Aug 25, 2022 at 11:49:05AM +1000, Alistair Popple wrote: >> Commit ab09243aa95a ("mm/migrate.c: remove MIGRATE_PFN_LOCKED") changed >> the way trylock_page() in migrate_vma_collect_pmd() works without >> updating the comment. Reword the comment to be less misleading and a >> better reflection of what happens. >> >> Signed-off-by: Alistair Popple >> Reported-by: Peter Xu >> Fixes: ab09243aa95a ("mm/migrate.c: remove MIGRATE_PFN_LOCKED") >> --- >> mm/migrate_device.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/mm/migrate_device.c b/mm/migrate_device.c >> index 5052093d0262..0736f846de0b 100644 >> --- a/mm/migrate_device.c >> +++ b/mm/migrate_device.c >> @@ -179,9 +179,11 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp, >> get_page(page); >> >> /* >> - * Optimize for the common case where page is only mapped once >> - * in one process. If we can lock the page, then we can safely >> - * set up a special migration page table entry now. >> + * If we can't lock the page we can't migrate it. If we can it's >> + * safe to set up a migration entry now. In the common case >> + * where the page is mapped once in a single process setting up >> + * the migration entry now is an optimisation to avoid walking >> + * the rmap later with try_to_migrate(). >> */ > > IMHO the last sentence can still be a bit confusing - here we 100% rely on > the trylock() to proceed or we'll stop migration right away. IMHO that > means this is not an optimization, since optimizations should always be > optional but not the case here. We have to lock the page here, we don't have to install the migration entries. Installing the migration entries here is optional and is the optimisation. > Meanwhile it'll be great to also mention about why trylock is needed and no > further attempt to use lock_page(). The comment in prepare() previously > was great but unfortunately that code clip was removed. Will add. > In short, do you think something like this might be clearer? I think it's important to mention the optimisation, otherwise the temptation is to remove the installation of migration entries here and rely on try_to_migrate() to do it later. I would actually like to be able to do that because it simplifies the code in many ways but based on my testing the optimisation turns out to be very worth while. > /* > * We rely on the trylock() to migrate the pte. If this > * fails, we'll fail the migration of this page. IOW, the > * migration is very much best-effort, just like we'll also > * bail out if we found page pinned by other users after > * page being locked. Honestly I think this describes what the code does rather than why and is likely to become outdated and confusing. IMHO it's quite clear from the code that the migration will fail here if we can't lock the page. And the fact that migration can fail is already covered in the existing documentation. See for example the extensive comment on migrate_vma_setup() about how the whole flow works. Eg: * Once migrate_vma_pages() returns the caller may inspect which pages were * successfully migrated, and which were not. Successfully migrated pages will * have the MIGRATE_PFN_MIGRATE flag set for their src array entry. > * > * We don't use lock_page() here or even later. It's > * because if to do so we may need to take the locks for > * multiple pages one by one, and the order to take the > * page locks is unclear. For example, if two processes > * want to lock multiple pages but in different order it > * can lead to deadlock. > */ > > Thanks,