linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: Yang Shi <shy828301@gmail.com>
Cc: David Stevens <stevensd@chromium.org>,
	linux-mm@kvack.org, Matthew Wilcox <willy@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	"Kirill A . Shutemov" <kirill@shutemov.name>,
	David Hildenbrand <david@redhat.com>,
	Hugh Dickins <hughd@google.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] mm/khugepaged: skip shmem with userfaultfd
Date: Thu, 16 Feb 2023 18:07:25 -0500	[thread overview]
Message-ID: <Y+63LQo49CCyXRQn@x1n> (raw)
In-Reply-To: <CAHbLzkqUF8Y759ZBVGXWdKwDS4f+ZJakUhoqf8Dqvx0Jam4c1g@mail.gmail.com>

Hi, Yang,

On Thu, Feb 16, 2023 at 01:58:55PM -0800, Yang Shi wrote:
> > IIUC we released it before copying the pages:
> 
> The huge page is locked until the copy is done. It should be fine
> unless the users inspect the page content without acquiring page lock.

The current patch from David has replaced "insert hpage into holes" with
"insert RETRY entries into holes", so IMHO the hpage is not visible at all
when releasing page cache lock here.

All the accessors (including RCU protected ones to access page cache; those
may not need to take the page lock) should be spinning on the RETRY entry,
which it seems fine to me.  But my question was whether it's legal to keep
them spinning even after releasing the page cache lock.

Thanks,

> 
> >
> > xa_locked:
> >         xas_unlock_irq(&xas);  <-------------------------------- here
> > xa_unlocked:
> >
> >         /*
> >          * If collapse is successful, flush must be done now before copying.
> >          * If collapse is unsuccessful, does flush actually need to be done?
> >          * Do it anyway, to clear the state.
> >          */
> >         try_to_unmap_flush();
> >
> > Before insertion of the multi-index:
> >
> >         /* Join all the small entries into a single multi-index entry. */
> >         xas_set_order(&xas, start, HPAGE_PMD_ORDER);
> >         xas_store(&xas, hpage);
> >
> > Thanks,
> >
> > --
> > Peter Xu
> >
> 

-- 
Peter Xu


  reply	other threads:[~2023-02-16 23:08 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-14  7:57 [PATCH 1/2] mm/khugepaged: set THP as uptodate earlier for shmem David Stevens
2023-02-14  7:57 ` [PATCH 2/2] mm/khugepaged: skip shmem with userfaultfd David Stevens
2023-02-14 22:35   ` Peter Xu
2023-02-15  1:57     ` David Stevens
2023-02-15 22:27       ` Peter Xu
2023-02-15 22:48   ` Peter Xu
2023-02-16  1:37     ` David Stevens
2023-02-16 14:41       ` Peter Xu
2023-02-16 21:58         ` Yang Shi
2023-02-16 23:07           ` Peter Xu [this message]
2023-02-16 23:52             ` Yang Shi
2023-02-17  2:00         ` David Stevens
2023-02-17  3:20           ` Yang Shi
2023-02-14 15:44 ` [PATCH 1/2] mm/khugepaged: set THP as uptodate earlier for shmem Matthew Wilcox
2023-02-15  1:33   ` David Stevens
2023-02-15 22:05     ` Peter Xu

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=Y+63LQo49CCyXRQn@x1n \
    --to=peterx@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=hughd@google.com \
    --cc=kirill@shutemov.name \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=shy828301@gmail.com \
    --cc=stevensd@chromium.org \
    --cc=willy@infradead.org \
    /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;
as well as URLs for NNTP newsgroup(s).