Re: [PATCH v2 1/9] mm: Add msharefs filesystem

[Khalid Aziz]

On 6/30/22 15:53, Darrick J. Wong wrote:
> On Wed, Jun 29, 2022 at 04:53:52PM -0600, Khalid Aziz wrote:
> > Add a ram-based filesystem that contains page table sharing
> > information and files that enables processes to share page tables.
> > This patch adds the basic filesystem that can be mounted.
> >
> > Signed-off-by: Khalid Aziz <khalid.aziz@xxxxxxxxxx>
> > ---
> >   Documentation/filesystems/msharefs.rst |  19 +++++
> >   include/uapi/linux/magic.h             |   1 +
> >   mm/Makefile                            |   2 +-
> >   mm/mshare.c                            | 103 +++++++++++++++++++++++++
> >   4 files changed, 124 insertions(+), 1 deletion(-)
> >   create mode 100644 Documentation/filesystems/msharefs.rst
> >   create mode 100644 mm/mshare.c
> >
> > diff --git a/Documentation/filesystems/msharefs.rst b/Documentation/filesystems/msharefs.rst
> > new file mode 100644
> > index 000000000000..fd161f67045d
> > --- /dev/null
> > +++ b/Documentation/filesystems/msharefs.rst
> > @@ -0,0 +1,19 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +=====================================================
> > +msharefs - a filesystem to support shared page tables
> > +=====================================================
> > +
> > +msharefs is a ram-based filesystem that allows multiple processes to
> > +share page table entries for shared pages.
> > +
> > +msharefs is typically mounted like this::
> > +
> > +	mount -t msharefs none /sys/fs/mshare
> > +
> > +When a process calls mshare syscall with a name for the shared address
> > +range,
>
> You mean creat()?
>
> > a file with the same name is created under msharefs with that
> > +name. This file can be opened by another process, if permissions
> > +allow, to query the addresses shared under this range. These files are
> > +removed by mshare_unlink syscall and can not be deleted directly.
>
> Oh?

msharefs.rst needs to be updated.

> > +Hence these files are created as immutable files.
> > diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h
> > index f724129c0425..2a57a6ec6f3e 100644
> > --- a/include/uapi/linux/magic.h
> > +++ b/include/uapi/linux/magic.h
> > @@ -105,5 +105,6 @@
> >   #define Z3FOLD_MAGIC		0x33
> >   #define PPC_CMM_MAGIC		0xc7571590
> >   #define SECRETMEM_MAGIC		0x5345434d	/* "SECM" */
> > +#define MSHARE_MAGIC		0x4d534852	/* "MSHR" */
> >   
> >   #endif /* __LINUX_MAGIC_H__ */
> > diff --git a/mm/Makefile b/mm/Makefile
> > index 6f9ffa968a1a..51a2ab9080d9 100644
> > --- a/mm/Makefile
> > +++ b/mm/Makefile
> > @@ -37,7 +37,7 @@ CFLAGS_init-mm.o += $(call cc-disable-warning, override-init)
> >   CFLAGS_init-mm.o += $(call cc-disable-warning, initializer-overrides)
> >   
> >   mmu-y			:= nommu.o
> > -mmu-$(CONFIG_MMU)	:= highmem.o memory.o mincore.o \
> > +mmu-$(CONFIG_MMU)	:= highmem.o memory.o mincore.o mshare.o \
> >   			   mlock.o mmap.o mmu_gather.o mprotect.o mremap.o \
> >   			   msync.o page_vma_mapped.o pagewalk.o \
> >   			   pgtable-generic.o rmap.o vmalloc.o
> > diff --git a/mm/mshare.c b/mm/mshare.c
> > new file mode 100644
> > index 000000000000..c8fab3869bab
> > --- /dev/null
> > +++ b/mm/mshare.c
>
> Filesystems are usually supposed to live under fs/; is there some reason
> to put it in mm/?
>
> I guess shmfs is in mm so maybe this isn't much of an objection.
>
> Also, should this fs be selectable via a Kconfig option?

Since this filesystem is meant to support an mm feature, I felt it was more appropriate for it to reside under mm/, 
similar to shmfs.

I could add a Kconfig option. The option would be to enable mshare feature. msharefs would automatically be enabled when 
mshare is enabled, i.e. msharefs shouldn't be a visible Kconfig option. Do we see a reason to make mshare an optional 
feature? If we can base hugetlbfs page table sharing on mshare in future, this will not be an optional feature at that 
time and mshare kconfig option will have to be removed.

Thanks,
Khalid