LKML Archive on
help / color / mirror / Atom feed
From: Peter Xu <>
To: David Hildenbrand <>
	Alistair Popple <>,
	Tiberiu Georgescu <>,,
	Mike Rapoport <>,
	Hugh Dickins <>,
	Matthew Wilcox <>,
	Andrea Arcangeli <>,
	"Kirill A . Shutemov" <>,
	Andrew Morton <>,
	Mike Kravetz <>
Subject: Re: [PATCH RFC 0/4] mm: Enable PM_SWAP for shmem with PTE_MARKER
Date: Tue, 17 Aug 2021 16:24:11 -0400	[thread overview]
Message-ID: <YRwa6+jx5PukCn53@t490s> (raw)
In-Reply-To: <>

On Tue, Aug 17, 2021 at 08:46:45PM +0200, David Hildenbrand wrote:
> > Please have a look at current pagemap impl in pte_to_pagemap_entry().  It's not
> > accurate from the 1st day, imho.  E.g., when a page is being migrated from numa
> > node 1 to node 2, we'll mark it PM_SWAP but I think it's not the case.  We can
> > make it more accurate, but I think it's fine, because it's a hint.
> That inconsistency doesn't really matter as you can determine if something
> is present and worth dumping if it's either swapped or present. As long as
> it's one of both but not simply nothing.
> I will shamelessly reference
> tools/testing/selftests/vm/madv_populate.c:pagemap_is_populated() that
> checks exactly for that (the test case uses only private anonymous memory).

Then I think the MADV_POPULATE_READ|WRITE test cases shouldn't depend on
PM_SWAP for that when it goes beyond anonymous private memories - when shmem
swapped out the pte can be none, then the test case can fail even if it
shouldn't, imho.

The mincore() syscall seems to be ideally the thing you may want to make it
accurate, but again it's not a problem for current anonymous private memories.

> > 
> > > Take CRIU as an example, it has to be correct even if a process would remap a
> > > memory region, fork() and unmap in the parent as far as I understand, ...
> > 
> > Are you talking about dirty bit or swap bit?  I'm a bit confused on why swap
> > bit needs to be accurate.  Maybe you mean the dirty bit?
> "Dumping present pages"
> "... CRIU does not dump all of the data. Instead, it determines which pages
> contain it, and only dumps those pages. This is done similarly to how
> regular memory dumping and restoring works, i.e. by looking for PRESENT or
> SWAPPED bits in owners' pagemap entries."
> -> Neither PRESENT nor SWAPPED results in memory not getting dumped, which
> makes perfect sense.
> 1) Process A sets up shared memory and writes data to it.
> 2) System swaps out memory, hints are setup.
> 3) Process A forks Process B, hints are not copied.
> 4) Process A unmaps shared memory, hints are dropped.
> 5) CRIU migrates process A and B and migrates only PRESENT or SWAPPED in
> pagemap.
> 6) Process B uses memory in shared memory region. Pages were not migrated.
> Just one example; feel free to correct me.

I think pte marker won't crash criu, what will happen is that it'll see more
ptes that used to be none that become the pte markers.  This reminded me that
maybe I should teach up mincore() syscall to also be aware of the pte marker at
least, and all non_swap_entry() callers.

> There is notion of the mincore() systemcall:
> "There is one particular feature of shared memory dumps worth mentioning.
> Sometimes, a shared memory page can exist in the kernel, but it is not
> mapped to any process. CRIU detects such pages by calling mincore() on the
> shmem segment, which reports back the page in-memory status. The mincore
> bitmap is when ANDed with the per-process ones. "
> Not sure if they actually mean ORed, because otherwise they'd be losing
> pages that have been swapped out. "mincore() returns a vector that indicates
> whether pages of the calling process's virtual memory are resident in core
> (RAM)"

I am wildly guessing they ORed the two just because PM_SWAP is not working
properly for shmem, so the OR happens only for shmem.  Criu may not only rely
on mincore() because they also want the dirty bits.

Btw, I noticed in 2016 criu switched from mincore() to lseek():

Criu should want to know "whether this page has valid data" not "whether this
page has swapped out", so lseek() seems to be more suitable, which I'm not
aware of before.

I'm now wondering whether for Tiberiu's case mincore() can also be used.  It
should just still be a bit slow because it'll look up the cache too, but it
should work similarly like the original proposal.


Peter Xu

  reply	other threads:[~2021-08-17 20:24 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-07  3:25 Peter Xu
2021-08-07  3:25 ` [PATCH RFC 1/4] mm: Introduce PTE_MARKER swap entry Peter Xu
2021-08-07  3:25 ` [PATCH RFC 2/4] mm: Check against orig_pte for finish_fault() Peter Xu
2021-08-07  3:25 ` [PATCH RFC 3/4] mm: Handle PTE_MARKER page faults Peter Xu
2021-08-07  3:25 ` [PATCH RFC 4/4] mm: Install marker pte when page out for shmem pages Peter Xu
2021-08-13 15:18   ` Tiberiu Georgescu
2021-08-13 16:01     ` Peter Xu
2021-08-18 18:02       ` Tiberiu Georgescu
2021-08-17  9:04 ` [PATCH RFC 0/4] mm: Enable PM_SWAP for shmem with PTE_MARKER David Hildenbrand
2021-08-17 17:09   ` Peter Xu
2021-08-17 18:46     ` David Hildenbrand
2021-08-17 20:24       ` Peter Xu [this message]
2021-08-18  8:24         ` David Hildenbrand
2021-08-18 17:52           ` Tiberiu Georgescu
2021-08-18 18:13             ` David Hildenbrand
2021-08-19 14:54               ` Tiberiu Georgescu
2021-08-19 17:26                 ` David Hildenbrand
2021-08-20 16:49                   ` Tiberiu Georgescu
2021-08-20 19:12                     ` Peter Xu
2021-08-25 13:40                       ` Tiberiu Georgescu
2021-08-25 14:59                         ` 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:

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

  git send-email \
    --in-reply-to=YRwa6+jx5PukCn53@t490s \ \ \ \ \ \ \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH RFC 0/4] mm: Enable PM_SWAP for shmem with PTE_MARKER' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).