linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: David Hildenbrand <david@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Nadav Amit <nadav.amit@gmail.com>,
	Mike Kravetz <mike.kravetz@oracle.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Axel Rasmussen <axelrasmussen@google.com>,
	Mike Rapoport <rppt@linux.vnet.ibm.com>
Subject: Re: [PATCH v2 1/3] mm/hugetlb: Fix race condition of uffd missing/minor handling
Date: Tue, 4 Oct 2022 09:49:37 -0400	[thread overview]
Message-ID: <Yzw58fSsSmxSqfYo@x1n> (raw)
In-Reply-To: <41fb1d6c-0d36-e88c-39fe-ea1e9d80a1fc@redhat.com>

On Tue, Oct 04, 2022 at 02:19:36PM +0200, David Hildenbrand wrote:
> That looks kind-of ugly now. I wonder if it would be worth factoring that
> handling out into a separate function and reusing it at two places. Would
> get rid of one level of code indent at least.
> 
> Apart from that, LGTM. Although the lockless reading of the PTE screams for
> more trouble in the future :)

Right there's potential to further rework it, I am just not sure whether
that could be common enough so that we can start to take pg lock for the
whole region (then we'll need to release for either page lock or alloc).
Not really sure whether that'll be worth the effort.

However, at least uffd minor doesn't really need the page lock so we can
optimize it with a find_get_page() earlier then the missing mode can be
moved over too (following a lock_page?).  Maybe I should give it a shot.

For this one I'll keep it simple since I think we should have it for stable
too.  Thanks for the review!

-- 
Peter Xu



  reply	other threads:[~2022-10-04 13:49 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-04  0:37 [PATCH v2 0/3] mm/hugetlb: Fix selftest failures with write check Peter Xu
2022-10-04  0:37 ` [PATCH v2 1/3] mm/hugetlb: Fix race condition of uffd missing/minor handling Peter Xu
2022-10-04  2:32   ` Mike Kravetz
2022-10-04 13:53     ` Peter Xu
2022-10-04 12:19   ` David Hildenbrand
2022-10-04 13:49     ` Peter Xu [this message]
2022-10-04  0:37 ` [PATCH v2 2/3] mm/hugetlb: Use hugetlb_pte_stable in migration race check Peter Xu
2022-10-04  2:35   ` Mike Kravetz
2022-10-04 12:20   ` David Hildenbrand
2022-10-04  0:37 ` [PATCH v2 3/3] mm/selftest: uffd: Explain the write missing fault check Peter Xu
2022-10-04  2:37   ` Mike Kravetz
2022-10-04 12:20   ` David Hildenbrand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Yzw58fSsSmxSqfYo@x1n \
    --to=peterx@redhat.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=axelrasmussen@google.com \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mike.kravetz@oracle.com \
    --cc=nadav.amit@gmail.com \
    --cc=rppt@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox