[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <tyoifr2ym3pzx4nwqhdwap57us3msusbsmql7do4pim5ku7qtm@wjyvh5bs633s>
Date: Mon, 8 Sep 2025 15:27:52 +0200
From: Jan Kara <jack@...e.cz>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Jonathan Corbet <corbet@....net>, Matthew Wilcox <willy@...radead.org>,
Guo Ren <guoren@...nel.org>, Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>,
Alexander Gordeev <agordeev@...ux.ibm.com>, Christian Borntraeger <borntraeger@...ux.ibm.com>,
Sven Schnelle <svens@...ux.ibm.com>, "David S . Miller" <davem@...emloft.net>,
Andreas Larsson <andreas@...sler.com>, Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Dan Williams <dan.j.williams@...el.com>,
Vishal Verma <vishal.l.verma@...el.com>, Dave Jiang <dave.jiang@...el.com>,
Nicolas Pitre <nico@...xnic.net>, Muchun Song <muchun.song@...ux.dev>,
Oscar Salvador <osalvador@...e.de>, David Hildenbrand <david@...hat.com>,
Konstantin Komarov <almaz.alexandrovich@...agon-software.com>, Baoquan He <bhe@...hat.com>, Vivek Goyal <vgoyal@...hat.com>,
Dave Young <dyoung@...hat.com>, Tony Luck <tony.luck@...el.com>,
Reinette Chatre <reinette.chatre@...el.com>, Dave Martin <Dave.Martin@....com>,
James Morse <james.morse@....com>, Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
"Liam R . Howlett" <Liam.Howlett@...cle.com>, Vlastimil Babka <vbabka@...e.cz>,
Mike Rapoport <rppt@...nel.org>, Suren Baghdasaryan <surenb@...gle.com>,
Michal Hocko <mhocko@...e.com>, Hugh Dickins <hughd@...gle.com>,
Baolin Wang <baolin.wang@...ux.alibaba.com>, Uladzislau Rezki <urezki@...il.com>,
Dmitry Vyukov <dvyukov@...gle.com>, Andrey Konovalov <andreyknvl@...il.com>,
Jann Horn <jannh@...gle.com>, Pedro Falcato <pfalcato@...e.de>, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org, linux-csky@...r.kernel.org,
linux-mips@...r.kernel.org, linux-s390@...r.kernel.org, sparclinux@...r.kernel.org,
nvdimm@...ts.linux.dev, linux-cxl@...r.kernel.org, linux-mm@...ck.org,
ntfs3@...ts.linux.dev, kexec@...ts.infradead.org, kasan-dev@...glegroups.com,
Jason Gunthorpe <jgg@...dia.com>
Subject: Re: [PATCH 00/16] expand mmap_prepare functionality, port more users
Hi Lorenzo!
On Mon 08-09-25 12:10:31, Lorenzo Stoakes wrote:
> Since commit c84bf6dd2b83 ("mm: introduce new .mmap_prepare() file
> callback"), The f_op->mmap hook has been deprecated in favour of
> f_op->mmap_prepare.
>
> This was introduced in order to make it possible for us to eventually
> eliminate the f_op->mmap hook which is highly problematic as it allows
> drivers and filesystems raw access to a VMA which is not yet correctly
> initialised.
>
> This hook also introduces complexity for the memory mapping operation, as
> we must correctly unwind what we do should an error arises.
>
> Overall this interface being so open has caused significant problems for
> us, including security issues, it is important for us to simply eliminate
> this as a source of problems.
>
> Therefore this series continues what was established by extending the
> functionality further to permit more drivers and filesystems to use
> mmap_prepare.
>
> After updating some areas that can simply use mmap_prepare as-is, and
> performing some housekeeping, we then introduce two new hooks:
>
> f_op->mmap_complete - this is invoked at the point of the VMA having been
> correctly inserted, though with the VMA write lock still held. mmap_prepare
> must also be specified.
>
> This expands the use of mmap_prepare to those callers which need to
> prepopulate mappings, as well as any which does genuinely require access to
> the VMA.
>
> It's simple - we will let the caller access the VMA, but only once it's
> established. At this point unwinding issues is simple - we just unmap the
> VMA.
>
> The VMA is also then correctly initialised at this stage so there can be no
> issues arising from a not-fully initialised VMA at this point.
>
> The other newly added hook is:
>
> f_op->mmap_abort - this is only valid in conjunction with mmap_prepare and
> mmap_complete. This is called should an error arise between mmap_prepare
> and mmap_complete (not as a result of mmap_prepare but rather some other
> part of the mapping logic).
>
> This is required in case mmap_prepare wishes to establish state or locks
> which need to be cleaned up on completion. If we did not provide this, then
> this could not be permitted as this cleanup would otherwise not occur
> should the mapping fail between the two calls.
So seeing these new hooks makes me wonder: Shouldn't rather implement
mmap(2) in a way more similar to how other f_op hooks behave like ->read or
->write? I.e., a hook called at rather high level - something like from
vm_mmap_pgoff() or similar similar level - which would just call library
functions from MM for the stuff it needs to do. Filesystems would just do
their checks and call the generic mmap function with the vm_ops they want
to use, more complex users could then fill in the VMA before releasing
mmap_lock or do cleanup in case of failure... This would seem like a more
understandable API than several hooks with rules when what gets called.
Honza
>
> We then add split remap_pfn_range*() functions which allow for PFN remap (a
> typical mapping prepopulation operation) split between a prepare/complete
> step, as well as io_mremap_pfn_range_prepare, complete for a similar
> purpose.
>
> From there we update various mm-adjacent logic to use this functionality as
> a first set of changes, as well as resctl and cramfs filesystems to round
> off the non-stacked filesystem instances.
>
>
> REVIEWER NOTE:
> ~~~~~~~~~~~~~~
>
> I considered putting the complete, abort callbacks in vm_ops, however this
> won't work because then we would be unable to adjust helpers like
> generic_file_mmap_prepare() (which provides vm_ops) to provide the correct
> complete, abort callbacks.
>
> Conceptually it also makes more sense to have these in f_op as they are
> one-off operations performed at mmap time to establish the VMA, rather than
> a property of the VMA itself.
>
> Lorenzo Stoakes (16):
> mm/shmem: update shmem to use mmap_prepare
> device/dax: update devdax to use mmap_prepare
> mm: add vma_desc_size(), vma_desc_pages() helpers
> relay: update relay to use mmap_prepare
> mm/vma: rename mmap internal functions to avoid confusion
> mm: introduce the f_op->mmap_complete, mmap_abort hooks
> doc: update porting, vfs documentation for mmap_[complete, abort]
> mm: add remap_pfn_range_prepare(), remap_pfn_range_complete()
> mm: introduce io_remap_pfn_range_prepare, complete
> mm/hugetlb: update hugetlbfs to use mmap_prepare, mmap_complete
> mm: update mem char driver to use mmap_prepare, mmap_complete
> mm: update resctl to use mmap_prepare, mmap_complete, mmap_abort
> mm: update cramfs to use mmap_prepare, mmap_complete
> fs/proc: add proc_mmap_[prepare, complete] hooks for procfs
> fs/proc: update vmcore to use .proc_mmap_[prepare, complete]
> kcov: update kcov to use mmap_prepare, mmap_complete
>
> Documentation/filesystems/porting.rst | 9 ++
> Documentation/filesystems/vfs.rst | 35 +++++++
> arch/csky/include/asm/pgtable.h | 5 +
> arch/mips/alchemy/common/setup.c | 28 +++++-
> arch/mips/include/asm/pgtable.h | 10 ++
> arch/s390/kernel/crash_dump.c | 6 +-
> arch/sparc/include/asm/pgtable_32.h | 29 +++++-
> arch/sparc/include/asm/pgtable_64.h | 29 +++++-
> drivers/char/mem.c | 80 ++++++++-------
> drivers/dax/device.c | 32 +++---
> fs/cramfs/inode.c | 134 ++++++++++++++++++--------
> fs/hugetlbfs/inode.c | 86 +++++++++--------
> fs/ntfs3/file.c | 2 +-
> fs/proc/inode.c | 13 ++-
> fs/proc/vmcore.c | 53 +++++++---
> fs/resctrl/pseudo_lock.c | 56 ++++++++---
> include/linux/fs.h | 4 +
> include/linux/mm.h | 53 +++++++++-
> include/linux/mm_types.h | 5 +
> include/linux/proc_fs.h | 5 +
> include/linux/shmem_fs.h | 3 +-
> include/linux/vmalloc.h | 10 +-
> kernel/kcov.c | 40 +++++---
> kernel/relay.c | 32 +++---
> mm/memory.c | 128 +++++++++++++++---------
> mm/secretmem.c | 2 +-
> mm/shmem.c | 49 +++++++---
> mm/util.c | 18 +++-
> mm/vma.c | 96 +++++++++++++++---
> mm/vmalloc.c | 16 ++-
> tools/testing/vma/vma_internal.h | 31 +++++-
> 31 files changed, 810 insertions(+), 289 deletions(-)
>
> --
> 2.51.0
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists