All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nadav Amit <nadav.amit@gmail.com>
To: David Hildenbrand <david@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Linux-MM <linux-mm@kvack.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Peter Xu <peterx@redhat.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Minchan Kim <minchan@kernel.org>, Colin Cross <ccross@google.com>,
	Suren Baghdasarya <surenb@google.com>,
	Mike Rapoport <rppt@linux.vnet.ibm.com>
Subject: Re: [RFC PATCH 0/8] mm/madvise: support process_madvise(MADV_DONTNEED)
Date: Mon, 27 Sep 2021 05:00:11 -0700	[thread overview]
Message-ID: <AE756194-07D4-4467-92CA-9E986140D85D@gmail.com> (raw)
In-Reply-To: <2753a311-4d5f-8bc5-ce6f-10063e3c6167@redhat.com>



> On Sep 27, 2021, at 3:58 AM, David Hildenbrand <david@redhat.com> wrote:
> 
> On 27.09.21 12:41, Nadav Amit wrote:
>>> On Sep 27, 2021, at 2:24 AM, David Hildenbrand <david@redhat.com> wrote:
>>> 
>>> On 26.09.21 18:12, Nadav Amit wrote:
>>>> From: Nadav Amit <namit@vmware.com>
>>>> The goal of these patches is to add support for
>>>> process_madvise(MADV_DONTNEED). Yet, in the process some (arguably)
>>>> useful cleanups, a bug fix and performance enhancements are performed.
>>>> The patches try to consolidate the logic across different behaviors, and
>>>> to a certain extent overlap/conflict with an outstanding patch that does
>>>> something similar [1]. This consolidation however is mostly orthogonal
>>>> to the aforementioned one and done in order to clarify what is done in
>>>> respect to locks and TLB for each behavior and to batch these operations
>>>> more efficiently on process_madvise().
>>>> process_madvise(MADV_DONTNEED) is useful for two reasons: (a) it allows
>>>> userfaultfd monitors to unmap memory from monitored processes; and (b)
>>>> it is more efficient than madvise() since it is vectored and batches TLB
>>>> flushes more aggressively.
>>> 
>>> MADV_DONTNEED on MAP_PRIVATE memory is a target-visible operation; this is very different to all the other process_madvise() calls we allow, which are merely hints, but the target cannot be broken . I don't think this is acceptable.
>> This is a fair point, which I expected, but did not address properly.
>> I guess an additional capability, such as CAP_SYS_PTRACE needs to be
>> required in this case. Would that ease your mind?
> 
> I think it would be slightly better, but I'm still missing a clear use case that justifies messing with the page tables of other processes in that way, especially with MAP_PRIVATE mappings. Can you maybe elaborate a bit on a) and b)?
> 
> Especially, why would a) make sense or be required? When would it be a good idea to zap random pages of a target process, especially with MAP_PRIVATE? How would the target use case make sure that the target process doesn't suddenly lose data? I would have assume that you can really only do something sane with uffd() if 1) the process decided to give up on some pages (madvise(DONTNEED)) b) the process hasn't touched these pages yet.
> 
> Can you also comment a bit more on b)? Who cares about that? And would we suddenly expect users of madvise() to switch to process_madvise() because it's more effective? It sounds a bit weird to me TBH, but most probably I am missing details :)

Ok, ok, your criticism is fair. I tried to hold back some details in order to
prevent the discussion from digressing. I am going to focus on (a) which is
what I really have in mind.

The use-case that I explore is a userspace memory manager with some level of
cooperation of the monitored processes.

The manager is notified on memory regions that it should monitor
(through PTRACE/LD_PRELOAD/explicit-API). It then monitors these regions
using the remote-userfaultfd that you saw on the second thread. When it wants
to reclaim (anonymous) memory, it:

1. Uses UFFD-WP to protect that memory (and for this matter I got a vectored
   UFFD-WP to do so efficiently, a patch which I did not send yet).
2. Calls process_vm_readv() to read that memory of that process.
3. Write it back to “swap”.
4. Calls process_madvise(MADV_DONTNEED) to zap it.

Once the memory is accessed again, the manager uses UFFD-COPY to bring it
back. This is really work-in-progress, but eventually performance is not as
bad as you would imagine (some patches for efficient use of uffd with
iouring are needed for that matter).

I am aware that there are some caveats, as zapping the memory does not
guarantee that the memory would be freed since it might be pinned for a
variety of reasons. That's the reason I mentioned the processes have "some
level of cooperation" with the manager. It is not intended to deal with
adversaries or uncommon corner cases (e.g., processes that use UFFD for
their own reasons).

Putting aside my use-case (which I am sure people would be glad to criticize),
I can imagine debuggers or emulators may also find use for similar schemes
(although I do not have concrete use-cases for them).


  reply	other threads:[~2021-09-27 12:00 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-26 16:12 [RFC PATCH 0/8] mm/madvise: support process_madvise(MADV_DONTNEED) Nadav Amit
2021-09-26 16:12 ` [RFC PATCH 1/8] mm/madvise: propagate vma->vm_end changes Nadav Amit
2021-09-27  9:08   ` Kirill A. Shutemov
2021-09-27 10:11     ` Nadav Amit
2021-09-27 11:55       ` Kirill A. Shutemov
2021-09-27 12:33         ` Nadav Amit
2021-09-27 12:45           ` Kirill A. Shutemov
2021-09-27 12:59             ` Nadav Amit
2021-09-26 16:12 ` [RFC PATCH 2/8] mm/madvise: remove unnecessary check on madvise_dontneed_free() Nadav Amit
2021-09-27  9:11   ` Kirill A. Shutemov
2021-09-27 11:05     ` Nadav Amit
2021-09-27 12:19       ` Kirill A. Shutemov
2021-09-27 12:52         ` Nadav Amit
2021-09-26 16:12 ` [RFC PATCH 3/8] mm/madvise: remove unnecessary checks on madvise_free_single_vma() Nadav Amit
2021-09-27  9:17   ` Kirill A. Shutemov
2021-09-27  9:24     ` Kirill A. Shutemov
2021-09-26 16:12 ` [RFC PATCH 4/8] mm/madvise: define madvise behavior in a struct Nadav Amit
2021-09-27  9:31   ` Kirill A. Shutemov
2021-09-27 10:31     ` Nadav Amit
2021-09-27 12:14       ` Kirill A. Shutemov
2021-09-27 20:36         ` Nadav Amit
2021-09-26 16:12 ` [RFC PATCH 5/8] mm/madvise: perform certain operations once on process_madvise() Nadav Amit
2021-09-26 16:12 ` [RFC PATCH 6/8] mm/madvise: more aggressive TLB batching Nadav Amit
2021-09-26 16:12 ` [RFC PATCH 7/8] mm/madvise: deduplicate code in madvise_dontneed_free() Nadav Amit
2021-09-26 16:12 ` [RFC PATCH 8/8] mm/madvise: process_madvise(MADV_DONTNEED) Nadav Amit
2021-09-27  9:24 ` [RFC PATCH 0/8] mm/madvise: support process_madvise(MADV_DONTNEED) David Hildenbrand
2021-09-27 10:41   ` Nadav Amit
2021-09-27 10:58     ` David Hildenbrand
2021-09-27 12:00       ` Nadav Amit [this message]
2021-09-27 12:16         ` Michal Hocko
2021-09-27 19:12           ` Nadav Amit
2021-09-29  7:52             ` Michal Hocko
2021-09-29 18:31               ` Nadav Amit
2021-10-12 23:14                 ` Peter Xu
2021-10-13 15:47                   ` Nadav Amit
2021-10-13 23:09                     ` Peter Xu
2021-09-27 17:05         ` David Hildenbrand
2021-09-27 19:59           ` Nadav Amit
2021-09-28  8:53             ` David Hildenbrand
2021-09-28 22:56               ` Nadav Amit
2021-10-04 17:58                 ` David Hildenbrand
2021-10-07 16:19                   ` Nadav Amit
2021-10-07 16:46                     ` 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=AE756194-07D4-4467-92CA-9E986140D85D@gmail.com \
    --to=nadav.amit@gmail.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=ccross@google.com \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=minchan@kernel.org \
    --cc=peterx@redhat.com \
    --cc=rppt@linux.vnet.ibm.com \
    --cc=surenb@google.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.