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 ACECCECAAA3 for ; Thu, 25 Aug 2022 23:51:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 249E16B0074; Thu, 25 Aug 2022 19:51:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1F9466B0075; Thu, 25 Aug 2022 19:51:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 09B48940007; Thu, 25 Aug 2022 19:51:28 -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 EB9106B0074 for ; Thu, 25 Aug 2022 19:51:27 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id B895280B1F for ; Thu, 25 Aug 2022 23:51:27 +0000 (UTC) X-FDA: 79839764214.19.FCA9BCF Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf15.hostedemail.com (Postfix) with ESMTP id 5A686A0017 for ; Thu, 25 Aug 2022 23:51:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1661471486; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=kd7QsOCtA78qYWYxH71l7L5caTVW93Ytg6pE6bnldGo=; b=eY4WfWkYJ9PZmj+FUbNVMLbp50IDAGFltTEUvZihlySOuBSvBShIwXBFZxY/Dyryufn1tb sfHpdqfzo1wBgPjrM4BkgTqCskyByalEhcqmoAwgE7hjbPxQ39yvAfuwg8ChKEZe94fSHr OoKUnmtmRL4hIuBGIt80EapRSfl1DYo= Received: from mail-qv1-f71.google.com (mail-qv1-f71.google.com [209.85.219.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-64-GncBjjZwOEK7DJioGSw9Kg-1; Thu, 25 Aug 2022 19:51:25 -0400 X-MC-Unique: GncBjjZwOEK7DJioGSw9Kg-1 Received: by mail-qv1-f71.google.com with SMTP id d10-20020a0cf6ca000000b00496744bc8e6so12644202qvo.2 for ; Thu, 25 Aug 2022 16:51:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc; bh=kd7QsOCtA78qYWYxH71l7L5caTVW93Ytg6pE6bnldGo=; b=i4iI+R68CNl9jGZYNRs2OG4ASABh+eJOB+RUza0S1SkU9xP8loeZFIXLqJ3vYt4MtD ys7xe54bsBchtOa3GDt1vx9JxSD4wVFRNkcTvtRECYCgq6Zwpk5NXIoW+7GX8WX0qD5x po6SpPDqn745Lj+q8mzYMsYhcMJmg+L9nnUMurlZ11OYFKaoXa+v8OAcU9ak31JAMfdF 1U3E+SSAjfWs7At3/R25c+NBBEHJ17vof27A6dKKtJQxKTNbdKEyKeYR1QgrzwYQxPK8 SRE5k6LNVoEZzxHQswsJDJ4jpM31rDcA24YNSRxLPG3a9DGwhDktPMDDxD86Tv5xlX3F /HeA== X-Gm-Message-State: ACgBeo0iXmw159Po+cyrDPc54AjMqhh0lnhsvTU3pl07adhwZ9b9o/gG mm8WpWmJXk5mW4pQIjlKZRxMTHuahfOCuhsZBSDm7nul3ptZjyzGuhirokaandk/61KBD1jYap2 NHr/OnXLYXzY= X-Received: by 2002:a05:6214:240f:b0:497:5626:d378 with SMTP id fv15-20020a056214240f00b004975626d378mr1755907qvb.46.1661471484834; Thu, 25 Aug 2022 16:51:24 -0700 (PDT) X-Google-Smtp-Source: AA6agR78qXWty1/35VoJZKz1g/qlFtZdcWl6o2tzdq6e3/BzstaGTLpNryxXZJoj1qO+sDgNASuwyQ== X-Received: by 2002:a05:6214:240f:b0:497:5626:d378 with SMTP id fv15-20020a056214240f00b004975626d378mr1755895qvb.46.1661471484600; Thu, 25 Aug 2022 16:51:24 -0700 (PDT) Received: from xz-m1.local (bras-base-aurron9127w-grc-35-70-27-3-10.dsl.bell.ca. [70.27.3.10]) by smtp.gmail.com with ESMTPSA id e2-20020ac80642000000b00342fc6a8e25sm207921qth.50.2022.08.25.16.51.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Aug 2022 16:51:24 -0700 (PDT) Date: Thu, 25 Aug 2022 19:51:22 -0400 From: Peter Xu To: Alistair Popple 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 Message-ID: References: <20220825014905.977168-1-apopple@nvidia.com> MIME-Version: 1.0 In-Reply-To: <20220825014905.977168-1-apopple@nvidia.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1661471487; 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=kd7QsOCtA78qYWYxH71l7L5caTVW93Ytg6pE6bnldGo=; b=0a775M6RrA89k8OvIArGzmzRmZQUuKX2/v21n9p9+0lV2ujMBGZkwTmMCJz6jQftupjYsg p+k6wIuh9N+c36sc5FivHMqngm624lcz3G3YlFHWxOR8U3nWyZEKL+7R107g3ql8ZwuwEq 5aiMS78mnooqEOfiyXJfZ4GtQf1AbWc= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=eY4WfWkY; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf15.hostedemail.com: domain of peterx@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=peterx@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1661471487; a=rsa-sha256; cv=none; b=qif5++Ow31JK8/P+g7EHoV1h4vcfVEc1xdbZALxcbBiLRl1HE0AzYJP4FehSZ7/bUgFtHz DGndO+VpIz2ZDQw5uFa0Chor/BrXxFHxTTuYV6eCHbGV+M1mZ38hwixx+fDOzaqMr5H/47 0/CIAh5acUwXIBmnRZPZNZDeb4zgNC4= Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=eY4WfWkY; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf15.hostedemail.com: domain of peterx@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=peterx@redhat.com X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 5A686A0017 X-Stat-Signature: qc8jzyjkwwrjs9krmwez9ywbz3qumwhd X-Rspam-User: X-HE-Tag: 1661471486-40937 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 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. 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. In short, do you think something like this might be clearer? /* * 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. * * 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, -- Peter Xu