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;
Date:   Wed, 7 Jun 2023 14:22:17 -0400
From:   Peter Xu <peterx@redhat.com>
To:     Yang Shi <shy828301@gmail.com>
Cc:     David Hildenbrand <david@redhat.com>, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org, Alistair Popple <apopple@nvidia.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Andrea Arcangeli <aarcange@redhat.com>,
        "Kirill A . Shutemov" <kirill@shutemov.name>,
        Johannes Weiner <hannes@cmpxchg.org>,
        John Hubbard <jhubbard@nvidia.com>,
        Naoya Horiguchi <naoya.horiguchi@nec.com>,
        Muhammad Usama Anjum <usama.anjum@collabora.com>,
        Hugh Dickins <hughd@google.com>,
        Mike Rapoport <rppt@kernel.org>
Subject: Re: [PATCH 0/4] mm: Fix pmd_trans_unstable() call sites on retry
Message-ID: <ZIDK2a6qf1SYF/kW@x1n>
References: <20230602230552.350731-1-peterx@redhat.com>
 <ZICK3uqTeUxeIlc9@x1n>
 <97b5657d-e09d-b3a9-c09d-eaff9fb7929d@redhat.com>
 <ZICukdhrxcaA74Pp@x1n>
 <CAHbLzkoiSW3jOhFdEU5w567SyxhB05dBfTrJvKg4X59nPURz3Q@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <CAHbLzkoiSW3jOhFdEU5w567SyxhB05dBfTrJvKg4X59nPURz3Q@mail.gmail.com>
Precedence: bulk

On Wed, Jun 07, 2023 at 09:39:44AM -0700, Yang Shi wrote:
> I don't think this is an important thing. There are plenty of other
> conditions that could make the accounting inaccurate, for example,
> isolating page from LRU fails, force charge, etc. And it seems like
> nobody was bothered by this either.

Yes, I read that a bit more and I agree.  So let me summarize after I read
Hugh's series just now..

With the pre-requisite of the new __pte_map_offset() that Hugh proposed
here:

[PATCH 04/31] mm/pgtable: allow pte_offset_map[_lock]() to fail
https://lore.kernel.org/r/8218ffdc-8be-54e5-0a8-83f5542af283@google.com

We should not need pmd_trans_unstable() anymore as Hugh pointed out, which
I fully agree.  I think Hugh has covered all the issues that this series
wanted to address alongside, namely:

  Patch 1 (mprotect) is covered in:

  [PATCH 18/31] mm/mprotect: delete pmd_none_or_clear_bad_unless_trans_huge()
  https://lore.kernel.org/r/4a834932-9064-9ed7-3cd1-99466f549486@google.com

  No way to move spinlock outside, so one -EAGAIN needed which makes sense.

  Patch 2 (migrate_device) is covered in:

  [PATCH 24/31] mm/migrate_device: allow pte_offset_map_lock() to fail
  https://lore.kernel.org/r/ea51bb69-189c-229b-fc0-9d3e7be5d6b@google.com

  By a direct retry, and more code unified so even better.

  Patch 3 () is covered in:

  [PATCH 19/31] mm/mremap: retry if either pte_offset_map_*lock() fails
  https://lore.kernel.org/r/2d3fbfea-5884-8211-0cc-954afe25ae9c@google.com

  Instead of WARN_ON_ONCE(), it got dropped which looks all good, too.

  Most of patch 4's changes are done in:

  [PATCH 09/31] mm/pagewalkers: ACTION_AGAIN if pte_offset_map_lock() fails
  https://lore.kernel.org/r/6265ac58-6018-a8c6-cf38-69cba698471@google.com

  There're some different handling on memcg changes, where in Hugh's
  series it was put separately here:

  [PATCH 17/31] mm/various: give up if pte_offset_map[_lock]() fails
  https://lore.kernel.org/r/c299eba-4e17-c645-1115-ccd1fd9956bd@google.com

  After double check, I agree with Yang's comment and Hugh's approach,
  that no retry is needed for memcg.

Let's ignore this series, not needed anymore.

Thanks,

-- 
Peter Xu