On May 31, 2017 3:08:22 PM GMT+03:00, Michal Hocko [off-list ref] wrote:

On Tue 30-05-17 17:43:26, Andrea Arcangeli wrote:

quoted

On Tue, May 30, 2017 at 04:39:41PM +0200, Michal Hocko wrote:

quoted

I sysctl for the mapcount can be increased, right? I also assume

that

quoted

quoted

those vmas will get merged after the post copy is done.

Assuming you enlarge the sysctl to the worst possible case, with

64bit

quoted

address space you can have billions of VMAs if you're migrating 4T of
RAM and you're unlucky and the address space gets fragmented. The
unswappable kernel memory overhead would be relatively large
(i.e. dozen gigabytes of RAM in vm_area_struct slab), and each
find_vma operation would need to walk ~40 steps across that large vma
rbtree. There's a reason the sysctl exist. Not to tell all those
unnecessary vma mangling operations would be protected by the

mmap_sem

quoted

for writing.

Not creating a ton of vmas and enabling vma-less pte mangling with a
single large vma and only using mmap_sem for reading during all the
pte mangling, is one of the primary design motivations for
userfaultfd.

Yes, I am aware of fallouts of too many vmas. I was asking merely to
learn whether this will really happen under the the specific usecase
Mike is after.

That depends on the application access pattern in the period between the pre-dump is finished and the application is frozen. If the accesses are random enough, the dirty pages that would be post copied could get spread all over the address space.

quoted

quoted

I understand that part but it sounds awfully one purpose thing to

me.

quoted

quoted

Are we going to add other MADVISE_RESET_$FOO to clear other flags

just

quoted

quoted

because we can race in this specific use case?

Those already exists, see for example MADV_NORMAL, clearing
~VM_RAND_READ & ~VM_SEQ_READ after calling MADV_SEQUENTIAL or
MADV_RANDOM.

I would argue that MADV_NORMAL is everything but a clear madvise
command. Why doesn't it clear all the sticky MADV* flags?

That would be helpful :)
Still, the problem here is more with the naming that with the action. If it was called MADV_DEFAULT_READ or something, it would be fine, wouldn't it?

quoted

Or MADV_DOFORK after MADV_DONTFORK. MADV_DONTDUMP after MADV_DODUMP.

Etc..

quoted

quoted

But we already have MADV_HUGEPAGE, MADV_NOHUGEPAGE and prctl to
enable/disable thp. Doesn't that sound little bit too much for a

single

quoted

quoted

feature to you?

MADV_NOHUGEPAGE doesn't mean clearing the flag set with
MADV_HUGEPAGE. MADV_NOHUGEPAGE disables THP on the region if the
global sysfs "enabled" tune is set to "always". MADV_HUGEPAGE enables
THP if the global "enabled" sysfs tune is set to "madvise". The two
MADV_NOHUGEPAGE and MADV_HUGEPAGE are needed to leverage the

three-way

quoted

setting of "never" "madvise" "always" of the global tune.

The "madvise" global tune exists if you want to save RAM and you

don't

quoted

care much about performance but still allowing apps like QEMU where

no

quoted

memory is lost by enabling THP, to use THP.

There's no way to clear either of those two flags and bring back the
default behavior of the global sysfs tune, so it's not redundant at
the very least.

Yes I am not a huge fan of the current MADV*HUGEPAGE semantic but I
would really like to see a strong usecase for adding another command on
top. 

Well, another command makes the semantic a bit better, IMHO...

From what Mike said a global disable THP for the whole process
while the post-copy is in progress is a better solution anyway.

For the CRIU usecase, disabling THP for a while and re-enabling it back will do the trick, provided VMAs flags are not affected, like in the patch you've sent.
Moreover, we may even get away with ioctl(UFFDIO_COPY) if it's overhead shows to be negligible​.
Still, I believe that MADV_RESET_HUGEPAGE (or some better named) command has the value on its own.
--
Sincerely yours,
Mike.