All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: Axel Rasmussen <axelrasmussen@google.com>
Cc: Matthew Wilcox <willy@infradead.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Mike Kravetz <mike.kravetz@oracle.com>,
	Muchun Song <muchun.song@linux.dev>,
	Andrew Morton <akpm@linux-foundation.org>,
	Hugh Dickins <hughd@google.com>, Shuah Khan <shuah@kernel.org>,
	"Liam R. Howlett" <Liam.Howlett@oracle.com>,
	Mike Rapoport <rppt@kernel.org>, Nadav Amit <namit@vmware.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, linux-kselftest@vger.kernel.org,
	James Houghton <jthoughton@google.com>
Subject: Re: [PATCH] mm: userfaultfd: add UFFDIO_CONTINUE_MODE_WP to install WP PTEs
Date: Tue, 14 Feb 2023 19:32:29 -0500	[thread overview]
Message-ID: <Y+woHZ3AHe3quadT@x1n> (raw)
In-Reply-To: <CAJHvVciR=inDaKnmCfCQsxgBsJB6eQVDXQw67o0Foc9ofgbuow@mail.gmail.com>

On Tue, Feb 14, 2023 at 02:37:51PM -0800, Axel Rasmussen wrote:
> Agreed, it would likely be a nice cleanup. Peter, any objections? I
> wouldn't mind writing a commit to do this sort of refactor, and rebase
> my change on top of that.

No objection here.  Personally I actually prefer keeping the parameters
around if possible because it's straightforward and no thinking of any
possible indirect accesses all over the place. But maybe growing as long as
8 is still a moot point..  It's just that I don't really know whether it'll
look that good if we put everything into a struct*.

Things like src_start/dst_start/.. do not look good to be there: each layer
could loop over its own range of start/end/... so even if not in the
function parameter we'll need a variable to hold them anyway.

But I do see a few low hanging fruits:

  - I don't see why we need to pass over mmap_changing over all of the
    __mcopy_atomic() callers.  One chance is we simply pass in the ctx* to
    replace "dst_mm + mmap_changing".

  - Merge mcopy_atomic_mode and mode, having last 2 bits for the existing
    three modes, then bit 3 for WP, good enough to set it for the new case.

  - Optionally, we can avoid passing over dst_mm/src_mm all around, when
    dst_vma/src_vma is there?

How about we start from simple?

-- 
Peter Xu


  reply	other threads:[~2023-02-15  0:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-14 21:50 [PATCH] mm: userfaultfd: add UFFDIO_CONTINUE_MODE_WP to install WP PTEs Axel Rasmussen
2023-02-14 22:17 ` Matthew Wilcox
2023-02-14 22:37   ` Axel Rasmussen
2023-02-15  0:32     ` Peter Xu [this message]
2023-02-15  6:37       ` Mike Rapoport

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+woHZ3AHe3quadT@x1n \
    --to=peterx@redhat.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=axelrasmussen@google.com \
    --cc=hughd@google.com \
    --cc=jthoughton@google.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mike.kravetz@oracle.com \
    --cc=muchun.song@linux.dev \
    --cc=namit@vmware.com \
    --cc=rppt@kernel.org \
    --cc=shuah@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --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 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.