[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cigo2r2x22bk7wzr6qvazcdkmt5kfqhbgb7nslpuff7djufucg@f6xucfuntz3q>
Date: Thu, 18 Sep 2025 15:43:34 -0400
From: "Liam R. Howlett" <Liam.Howlett@...cle.com>
To: Peter Xu <peterx@...hat.com>
Cc: David Hildenbrand <david@...hat.com>,
Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
Nikita Kalyazin <kalyazin@...zon.com>, Mike Rapoport <rppt@...nel.org>,
Suren Baghdasaryan <surenb@...gle.com>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, Vlastimil Babka <vbabka@...e.cz>,
Muchun Song <muchun.song@...ux.dev>, Hugh Dickins <hughd@...gle.com>,
Andrew Morton <akpm@...ux-foundation.org>,
James Houghton <jthoughton@...gle.com>, Michal Hocko <mhocko@...e.com>,
Andrea Arcangeli <aarcange@...hat.com>,
Oscar Salvador <osalvador@...e.de>,
Axel Rasmussen <axelrasmussen@...gle.com>,
Ujwal Kundur <ujwal.kundur@...il.com>
Subject: Re: [PATCH v2 1/4] mm: Introduce vm_uffd_ops API
* Peter Xu <peterx@...hat.com> [250918 14:21]:
> On Thu, Sep 18, 2025 at 07:53:46PM +0200, David Hildenbrand wrote:
> > Re Nikita: If we could just reuse fault() for userfaultfd purposes, that
> > might actually be pretty nice.
>
> I commented on that.
>
> https://lore.kernel.org/all/aEiwHjl4tsUt98sh@x1.local/
>
> That'll need to leak FAULT_FLAG_USERFAULT_CONTINUE which isn't necessary,
> make it extremely hard to know when to set the flag, and comlicates the
> fault path which isn't necessary.
>
> I think Mike's comment was spot on, that the new API is literally
> do_fault() for shmem, but only used in userfaultfd context so it's even an
> oneliner.
>
> I do not maintain mm, so above is only my two cents, so I don't make
> decisions. Personally I still prefer the current approach of keep the mm
> main fault path clean.
What we are trying to say is you can have a fault path that takes a type
enum that calls into a function that does whatever you want. It can
even live in mm/userfaultfd.c. It can then jump off to mm/guest-memfd.c
which can contain super unique copying of memory if that's needed.
That way driver/i_know_better_that_everyone.c or fs/stature.c don't
decide they can register their uffd and do cool stuff that totally won't
tank the system in random strange ways.
Seriously, how many fault handlers are you expecting to have here?
I'd be surprised if a lot of the code in these memory types isn't
shared, but I guess if they are all over the kernel they'll just clone
the code and bugs (like the arch code does, but with less of a reason).
> Besides, this series also cleans up other places all over the places, the
> vm_uffd_ops is a most simplified version of description for a memory type.
6 files changed, 207 insertions(+), 76 deletions(-)
Can you please point me to which patch has clean up?
The mm has uffd code _everywhere_, including mm/userfaultfd.c that jumps
to fs/userfaultfd.c and back. Every entry has a LIST_HEAD(uf) [1] [2]
[3] in it that is passed through the entire call stack in case it is
needed [4] [5] [6] [7] [8]. It has if statements in core mm functions
in the middle of loops [9]. It complicates error handling and has
tricky locking [10] (full disclosure, I helped with the locking..
totally worth the complication).
This is a seriously complicated feature.
How is a generic callback that splits out into, probably 4?, functions
the deal breaker here?
How is leaking a flag the line that we won't cross?
> So IMHO it's beneficial in other aspects as well. If uffd_copy() is a
> concern, fine, we drop it. We don't plan to have more use of UFFDIO_COPY
> outside of the known three memory types after all.
EXACTLY! There are three memory types and we're going to the most
flexible interface possible, with the most danger. With guest_memfd
we're up to four functions we'd need. Why not keep the mm code in the
mm and have four functions to choose from? If you want 5 we can always
add another.
Regards,
Liam
[1]. https://elixir.bootlin.com/linux/v6.17-rc6/source/mm/mmap.c#L122
[2]. https://elixir.bootlin.com/linux/v6.17-rc6/source/mm/vma.c#L3149
[3]. https://elixir.bootlin.com/linux/v6.17-rc6/source/mm/util.c#L572
[4]. https://elixir.bootlin.com/linux/v6.17-rc6/source/mm/mmap.c#L338
[5]. https://elixir.bootlin.com/linux/v6.17-rc6/source/mm/mmap.c#L1063
[6]. https://elixir.bootlin.com/linux/v6.17-rc6/source/mm/vma.c#L1478
[7]. https://elixir.bootlin.com/linux/v6.17-rc6/source/mm/vma.c#L1517
[8]. https://elixir.bootlin.com/linux/v6.17-rc6/source/mm/vma.c#L1563
[9]. https://elixir.bootlin.com/linux/v6.17-rc6/source/mm/vma.c#L1407
[10]. https://elixir.bootlin.com/linux/v6.17-rc6/source/mm/userfaultfd.c#L69
Powered by blists - more mailing lists