Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1718723 > unrolled thread
| Started by | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| First post | 2017-08-24 02:00 +0200 |
| Last post | 2017-08-24 19:50 +0200 |
| Articles | 20 on this page of 31 — 7 participants |
Back to article view | Back to linux.kernel
[PATCH v6 0/5] MAP_DIRECT and block-map-atomic files Dan Williams <dan.j.williams@intel.com> - 2017-08-24 02:00 +0200
[PATCH v6 4/5] fs, xfs: introduce MAP_DIRECT for creating block-map-atomic file ranges Dan Williams <dan.j.williams@intel.com> - 2017-08-24 02:00 +0200
Re: [PATCH v6 4/5] fs, xfs: introduce MAP_DIRECT for creating block-map-atomic file ranges Christoph Hellwig <hch@lst.de> - 2017-08-24 18:20 +0200
Re: [PATCH v6 4/5] fs, xfs: introduce MAP_DIRECT for creating block-map-atomic file ranges Christoph Hellwig <hch@lst.de> - 2017-08-24 18:40 +0200
Re: [PATCH v6 4/5] fs, xfs: introduce MAP_DIRECT for creating block-map-atomic file ranges Dan Williams <dan.j.williams@intel.com> - 2017-08-24 22:30 +0200
Re: [PATCH v6 4/5] fs, xfs: introduce MAP_DIRECT for creating block-map-atomic file ranges Dan Williams <dan.j.williams@intel.com> - 2017-08-24 18:40 +0200
[PATCH v6 5/5] fs, fcntl: add F_MAP_DIRECT Dan Williams <dan.j.williams@intel.com> - 2017-08-24 02:00 +0200
[PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags Dan Williams <dan.j.williams@intel.com> - 2017-08-24 02:00 +0200
Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags Jan Kara <jack@suse.cz> - 2017-08-24 15:10 +0200
Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags Christoph Hellwig <hch@infradead.org> - 2017-08-24 19:00 +0200
Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags Dan Williams <dan.j.williams@intel.com> - 2017-08-24 19:40 +0200
Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags Christoph Hellwig <hch@infradead.org> - 2017-08-25 15:10 +0200
Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-08-25 18:00 +0200
Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags Christoph Hellwig <hch@infradead.org> - 2017-08-25 18:10 +0200
Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-08-25 18:20 +0200
Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags Helge Deller <deller@gmx.de> - 2017-08-25 18:30 +0200
Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2017-08-25 19:00 +0200
Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags Dan Williams <dan.j.williams@intel.com> - 2017-08-25 22:30 +0200
Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags Helge Deller <deller@gmx.de> - 2017-08-26 09:50 +0200
Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags Dan Williams <dan.j.williams@intel.com> - 2017-08-26 17:20 +0200
Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags Helge Deller <deller@gmx.de> - 2017-08-26 22:00 +0200
Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags Dan Williams <dan.j.williams@intel.com> - 2017-08-27 00:50 +0200
Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-08-27 02:00 +0200
[PATCH v6 2/5] fs, xfs: introduce S_IOMAP_SEALED Dan Williams <dan.j.williams@intel.com> - 2017-08-24 02:00 +0200
Re: [PATCH v6 2/5] fs, xfs: introduce S_IOMAP_SEALED Christoph Hellwig <hch@lst.de> - 2017-08-24 18:20 +0200
Re: [PATCH v6 2/5] fs, xfs: introduce S_IOMAP_SEALED Dan Williams <dan.j.williams@intel.com> - 2017-08-25 08:10 +0200
Re: [PATCH v6 2/5] fs, xfs: introduce S_IOMAP_SEALED Dan Williams <dan.j.williams@intel.com> - 2017-08-25 21:50 +0200
Re: [PATCH v6 0/5] MAP_DIRECT and block-map-atomic files Christoph Hellwig <hch@lst.de> - 2017-08-24 18:10 +0200
Re: [PATCH v6 0/5] MAP_DIRECT and block-map-atomic files Dan Williams <dan.j.williams@intel.com> - 2017-08-24 18:30 +0200
Re: [PATCH v6 1/5] vfs: add flags parameter to ->mmap() in 'struct file_operations' Christoph Hellwig <hch@infradead.org> - 2017-08-24 19:00 +0200
Re: [PATCH v6 1/5] vfs: add flags parameter to ->mmap() in 'struct file_operations' Dan Williams <dan.j.williams@intel.com> - 2017-08-24 19:50 +0200
Page 1 of 2 [1] 2 Next page →
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-24 02:00 +0200 |
| Subject | [PATCH v6 0/5] MAP_DIRECT and block-map-atomic files |
| Message-ID | <uhNIZ-78j-1@gated-at.bofh.it> |
Changes since v5 [1]:
* Compile fixes from a much improved coccinelle semantic patch (thanks
Julia!) that adds a 'flags' argument to all the ->mmap()
implementations in the kernel. (0day-kbuild-robot)
* Make the deprecated MAP_DENYWRITE and MAP_EXECUTABLE flags return
EOPNOTSUPP with the new mmap3() syscall. (Kirill)
* Minor changelog updates.
* Updated cover letter with a clarified summary and checklist of
questions to answer before proceeding further.
---
MAP_DIRECT is a mechanism to ask the kernel to atomically manage the
file-offset to physical-address block relationship of a mapping relative
to any memory-mapped access. It is complimentary to the proposed
MAP_SYNC mechanism which makes the same guarantee relative to cpu
faults. MAP_DIRECT goes a step further and makes this guarantee for
agents that may not generate mmu faults, but at the cost of restricting
the kernel's ability to mutate the block-map at will.
MAP_SYNC is preferred for scenarios that want full filesystem feature
support while avoiding fsync/msync overhead, but also do not need to
contend with hypervisors or RDMA agents that do not give the kernel an
mmu fault. In other words, the need for MAP_DIRECT is driven by the
scarcity of SVM capable hardware (Shared Virtual Memory, where hardware
generates mmu faults), hypervisors like Xen that need to interrogate the
physical address layout of a file to maintain their own physical-address
mapping metadata outside the kernel, and peer-to-peer DMA use cases that
always bypass the mmu.
The MAP_DIRECT mechanism allows a filesystem to be used for capacity
provisioning and access control where these aforementioned applications
would otherwise be forced to roll a custom solution on top of a raw
device-file.
Questions:
1/ Is the definition of MAP_DIRECT constrained enough to allow us to
make the restrictions it imposes on the kernel finer grained over time?
2/ Do the XFS changes look sane? They attempt to avoid adding any
overhead to the non-MAP_DIRECT case at the expense of the new
i_mapdcount atomic counter in the XFS inode.
3/ While the generic MAP_DIRECT description warns that the block-map may
not be actually be immutable for the lifetime of the mapping it also
does not preclude a filesystem from making that guarantee. In fact,
Dave wants to be able to get a stable view of the physical mapping
[2], and Xen has a need to do the same [3]. Do we want userspace to
start making "XFS + MAP_DIRECT == Immutable" assumptions, or do we
need a separate mechanism for that guarantee?
[1]: https://lkml.org/lkml/2017/8/16/114
[2]: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1467677.html
[3]: https://lists.xen.org/archives/html/xen-devel/2017-04/msg00419.html
---
Dan Williams (5):
vfs: add flags parameter to ->mmap() in 'struct file_operations'
fs, xfs: introduce S_IOMAP_SEALED
mm: introduce mmap3 for safely defining new mmap flags
fs, xfs: introduce MAP_DIRECT for creating block-map-atomic file ranges
fs, fcntl: add F_MAP_DIRECT
arch/arc/kernel/arc_hostlink.c | 3 -
arch/mips/kernel/vdso.c | 2
arch/powerpc/kernel/proc_powerpc.c | 3 -
arch/powerpc/kvm/book3s_64_vio.c | 3 -
arch/powerpc/platforms/cell/spufs/file.c | 21 ++--
arch/powerpc/platforms/powernv/opal-prd.c | 3 -
arch/um/drivers/mmapper_kern.c | 3 -
arch/x86/entry/syscalls/syscall_32.tbl | 1
arch/x86/entry/syscalls/syscall_64.tbl | 1
drivers/android/binder.c | 3 -
drivers/char/agp/frontend.c | 3 -
drivers/char/bsr.c | 3 -
drivers/char/hpet.c | 6 +
drivers/char/mbcs.c | 3 -
drivers/char/mbcs.h | 3 -
drivers/char/mem.c | 11 +-
drivers/char/mspec.c | 9 +-
drivers/char/uv_mmtimer.c | 6 +
drivers/dax/device.c | 3 -
drivers/dma-buf/dma-buf.c | 4 +
drivers/firewire/core-cdev.c | 3 -
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 -
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 3 -
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 5 +
drivers/gpu/drm/arc/arcpgu_drv.c | 5 +
drivers/gpu/drm/ast/ast_drv.h | 3 -
drivers/gpu/drm/ast/ast_ttm.c | 3 -
drivers/gpu/drm/bochs/bochs.h | 3 -
drivers/gpu/drm/bochs/bochs_mm.c | 3 -
drivers/gpu/drm/cirrus/cirrus_drv.h | 3 -
drivers/gpu/drm/cirrus/cirrus_ttm.c | 3 -
drivers/gpu/drm/drm_gem.c | 3 -
drivers/gpu/drm/drm_gem_cma_helper.c | 6 +
drivers/gpu/drm/drm_vm.c | 3 -
drivers/gpu/drm/etnaviv/etnaviv_drv.h | 3 -
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 5 +
drivers/gpu/drm/exynos/exynos_drm_gem.c | 5 +
drivers/gpu/drm/exynos/exynos_drm_gem.h | 3 -
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 3 -
drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 3 -
drivers/gpu/drm/i810/i810_dma.c | 3 -
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 2
drivers/gpu/drm/mediatek/mtk_drm_gem.c | 5 +
drivers/gpu/drm/mediatek/mtk_drm_gem.h | 3 -
drivers/gpu/drm/mgag200/mgag200_drv.h | 3 -
drivers/gpu/drm/mgag200/mgag200_ttm.c | 3 -
drivers/gpu/drm/msm/msm_drv.h | 3 -
drivers/gpu/drm/msm/msm_gem.c | 5 +
drivers/gpu/drm/nouveau/nouveau_ttm.c | 5 +
drivers/gpu/drm/nouveau/nouveau_ttm.h | 2
drivers/gpu/drm/omapdrm/omap_drv.h | 3 -
drivers/gpu/drm/omapdrm/omap_gem.c | 5 +
drivers/gpu/drm/qxl/qxl_drv.h | 3 -
drivers/gpu/drm/qxl/qxl_ttm.c | 3 -
drivers/gpu/drm/radeon/radeon_drv.c | 3 -
drivers/gpu/drm/radeon/radeon_ttm.c | 3 -
drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 5 +
drivers/gpu/drm/rockchip/rockchip_drm_gem.h | 3 -
drivers/gpu/drm/tegra/gem.c | 5 +
drivers/gpu/drm/tegra/gem.h | 3 -
drivers/gpu/drm/udl/udl_drv.h | 3 -
drivers/gpu/drm/udl/udl_gem.c | 5 +
drivers/gpu/drm/vc4/vc4_bo.c | 5 +
drivers/gpu/drm/vc4/vc4_drv.h | 3 -
drivers/gpu/drm/vgem/vgem_drv.c | 7 +
drivers/gpu/drm/virtio/virtgpu_drv.h | 3 -
drivers/gpu/drm/virtio/virtgpu_ttm.c | 3 -
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 3 -
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 3 -
drivers/hsi/clients/cmt_speech.c | 3 -
drivers/hwtracing/intel_th/msu.c | 3 -
drivers/hwtracing/stm/core.c | 3 -
drivers/infiniband/core/uverbs_main.c | 3 -
drivers/infiniband/hw/hfi1/file_ops.c | 6 +
drivers/infiniband/hw/qib/qib_file_ops.c | 5 +
drivers/media/v4l2-core/v4l2-dev.c | 3 -
drivers/misc/aspeed-lpc-ctrl.c | 3 -
drivers/misc/cxl/api.c | 5 +
drivers/misc/cxl/cxl.h | 3 -
drivers/misc/cxl/file.c | 3 -
drivers/misc/genwqe/card_dev.c | 3 -
drivers/misc/mic/scif/scif_fd.c | 3 -
drivers/misc/mic/vop/vop_vringh.c | 3 -
drivers/misc/sgi-gru/grufile.c | 3 -
drivers/mtd/mtdchar.c | 3 -
drivers/pci/proc.c | 3 -
drivers/rapidio/devices/rio_mport_cdev.c | 3 -
drivers/sbus/char/flash.c | 3 -
drivers/sbus/char/jsflash.c | 3 -
drivers/scsi/cxlflash/superpipe.c | 5 +
drivers/scsi/sg.c | 3 -
drivers/staging/android/ashmem.c | 3 -
drivers/staging/comedi/comedi_fops.c | 3 -
.../staging/lustre/lustre/llite/llite_internal.h | 3 -
drivers/staging/lustre/lustre/llite/llite_mmap.c | 5 +
drivers/staging/vboxvideo/vbox_drv.h | 3 -
drivers/staging/vboxvideo/vbox_ttm.c | 3 -
drivers/staging/vme/devices/vme_user.c | 3 -
drivers/uio/uio.c | 3 -
drivers/usb/core/devio.c | 3 -
drivers/usb/mon/mon_bin.c | 3 -
drivers/vfio/vfio.c | 7 +
drivers/video/fbdev/core/fbmem.c | 3 -
drivers/video/fbdev/pxa3xx-gcu.c | 3 -
drivers/xen/gntalloc.c | 3 -
drivers/xen/gntdev.c | 3 -
drivers/xen/privcmd.c | 3 -
drivers/xen/xenbus/xenbus_dev_backend.c | 3 -
drivers/xen/xenfs/xenstored.c | 3 -
fs/9p/vfs_file.c | 10 +-
fs/aio.c | 3 -
fs/attr.c | 10 ++
fs/btrfs/file.c | 3 -
fs/ceph/addr.c | 3 -
fs/ceph/super.h | 3 -
fs/cifs/cifsfs.h | 6 +
fs/cifs/file.c | 10 +-
fs/coda/file.c | 5 +
fs/ecryptfs/file.c | 5 +
fs/ext2/file.c | 5 +
fs/ext4/file.c | 3 -
fs/f2fs/file.c | 3 -
fs/fcntl.c | 15 +++
fs/fuse/file.c | 8 +
fs/gfs2/file.c | 3 -
fs/hugetlbfs/inode.c | 3 -
fs/kernfs/file.c | 3 -
fs/ncpfs/mmap.c | 3 -
fs/ncpfs/ncp_fs.h | 2
fs/nfs/file.c | 5 +
fs/nfs/internal.h | 2
fs/nilfs2/file.c | 3 -
fs/ocfs2/mmap.c | 3 -
fs/ocfs2/mmap.h | 3 -
fs/open.c | 6 +
fs/orangefs/file.c | 5 +
fs/proc/inode.c | 7 +
fs/proc/vmcore.c | 6 +
fs/ramfs/file-nommu.c | 6 +
fs/read_write.c | 3 +
fs/romfs/mmap-nommu.c | 3 -
fs/ubifs/file.c | 5 +
fs/xfs/libxfs/xfs_bmap.c | 5 +
fs/xfs/xfs_bmap_util.c | 3 +
fs/xfs/xfs_file.c | 114 +++++++++++++++++++-
fs/xfs/xfs_inode.h | 1
fs/xfs/xfs_ioctl.c | 6 +
fs/xfs/xfs_super.c | 1
include/drm/drm_gem.h | 3 -
include/drm/drm_gem_cma_helper.h | 3 -
include/drm/drm_legacy.h | 3 -
include/linux/fs.h | 21 ++--
include/linux/mm.h | 2
include/linux/mman.h | 46 ++++++++
include/linux/syscalls.h | 3 +
include/misc/cxl.h | 3 -
include/uapi/asm-generic/mman.h | 1
include/uapi/linux/fcntl.h | 5 +
ipc/shm.c | 5 +
kernel/events/core.c | 3 -
kernel/kcov.c | 3 -
kernel/relay.c | 3 -
mm/filemap.c | 19 ++-
mm/mmap.c | 56 +++++++++-
mm/nommu.c | 4 -
mm/shmem.c | 3 -
net/socket.c | 6 +
security/selinux/selinuxfs.c | 6 +
sound/core/compress_offload.c | 3 -
sound/core/hwdep.c | 3 -
sound/core/info.c | 3 -
sound/core/init.c | 3 -
sound/core/oss/pcm_oss.c | 3 -
sound/core/pcm_native.c | 3 -
sound/oss/soundcard.c | 3 -
sound/oss/swarm_cs4297a.c | 3 -
virt/kvm/kvm_main.c | 3 -
177 files changed, 689 insertions(+), 234 deletions(-)
[toc] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-24 02:00 +0200 |
| Subject | [PATCH v6 4/5] fs, xfs: introduce MAP_DIRECT for creating block-map-atomic file ranges |
| Message-ID | <uhNIZ-78j-11@gated-at.bofh.it> |
| In reply to | #1718723 |
MAP_DIRECT is an mmap(2) flag with the following semantics:
MAP_DIRECT
When specified with MAP_SHARED a successful fault in this range
indicates that the kernel is maintaining the block map (user linear
address to file offset to physical address relationship) in a manner
that no external agent can observe any inconsistent changes. In other
words, the block map of the mapping is effectively pinned, or the kernel
is otherwise able to exchange a new physical extent atomically with
respect to any hardware / software agent. As implied by this definition
a successful fault in a MAP_DIRECT range bypasses kernel indirections
like the page-cache, and all updates are carried directly through to the
underlying file physical-address blocks (modulo cpu cache effects).
ETXTBSY may be returned to any third party operation on the file that
attempts to update the block map (allocate blocks / convert unwritten
extents / break shared extents). However, whether a filesystem returns
EXTBSY for a certain state of the block relative to a MAP_DIRECT mapping
is filesystem and kernel version dependent.
Some filesystems may extend these operation restrictions outside the
mapped range and return ETXTBSY to any file operations that might mutate
the block map. MAP_DIRECT faults may fail with a SIGBUS if the
filesystem needs to write the block map to satisfy the fault. For
example, if the mapping was established over a hole in a sparse file.
ERRORS
EACCES A MAP_DIRECT mapping was requested and PROT_WRITE was not set,
or the requesting process is missing CAP_LINUX_IMMUTABLE.
EINVAL MAP_ANONYMOUS or MAP_PRIVATE was specified with MAP_DIRECT.
EOPNOTSUPP The filesystem explicitly does not support the flag
SIGBUS Attempted to write a MAP_DIRECT mapping at a file offset that
might require block-map updates.
Cc: Jan Kara <jack@suse.cz>
Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Dave Chinner <david@fromorbit.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
fs/xfs/xfs_file.c | 115 ++++++++++++++++++++++++++++++++++++++-
fs/xfs/xfs_inode.h | 1
fs/xfs/xfs_super.c | 1
include/linux/mman.h | 6 ++
include/uapi/asm-generic/mman.h | 1
mm/mmap.c | 23 ++++++++
6 files changed, 142 insertions(+), 5 deletions(-)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index cacc0162a41a..f82bf9416200 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -40,6 +40,7 @@
#include "xfs_iomap.h"
#include "xfs_reflink.h"
+#include <linux/mman.h>
#include <linux/dcache.h>
#include <linux/falloc.h>
#include <linux/pagevec.h>
@@ -1001,6 +1002,25 @@ xfs_file_llseek(
return vfs_setpos(file, offset, inode->i_sb->s_maxbytes);
}
+static const struct vm_operations_struct xfs_file_vm_direct_ops;
+
+STATIC int
+xfs_vma_checks(
+ struct vm_area_struct *vma,
+ struct inode *inode)
+{
+ if (vma->vm_ops != &xfs_file_vm_direct_ops)
+ return 0;
+
+ if (xfs_is_reflink_inode(XFS_I(inode)))
+ return VM_FAULT_SIGBUS;
+
+ if (!IS_DAX(inode))
+ return VM_FAULT_SIGBUS;
+
+ return 0;
+}
+
/*
* Locking for serialisation of IO during page faults. This results in a lock
* ordering of:
@@ -1031,6 +1051,10 @@ xfs_filemap_page_mkwrite(
file_update_time(vmf->vma->vm_file);
xfs_ilock(XFS_I(inode), XFS_MMAPLOCK_SHARED);
+ ret = xfs_vma_checks(vmf->vma, inode);
+ if (ret)
+ goto out_unlock;
+
if (IS_DAX(inode)) {
ret = dax_iomap_fault(vmf, PE_SIZE_PTE, &xfs_iomap_ops);
} else {
@@ -1038,6 +1062,7 @@ xfs_filemap_page_mkwrite(
ret = block_page_mkwrite_return(ret);
}
+out_unlock:
xfs_iunlock(XFS_I(inode), XFS_MMAPLOCK_SHARED);
sb_end_pagefault(inode->i_sb);
@@ -1058,10 +1083,15 @@ xfs_filemap_fault(
return xfs_filemap_page_mkwrite(vmf);
xfs_ilock(XFS_I(inode), XFS_MMAPLOCK_SHARED);
+ ret = xfs_vma_checks(vmf->vma, inode);
+ if (ret)
+ goto out_unlock;
+
if (IS_DAX(inode))
ret = dax_iomap_fault(vmf, PE_SIZE_PTE, &xfs_iomap_ops);
else
ret = filemap_fault(vmf);
+out_unlock:
xfs_iunlock(XFS_I(inode), XFS_MMAPLOCK_SHARED);
return ret;
@@ -1094,7 +1124,9 @@ xfs_filemap_huge_fault(
}
xfs_ilock(XFS_I(inode), XFS_MMAPLOCK_SHARED);
- ret = dax_iomap_fault(vmf, pe_size, &xfs_iomap_ops);
+ ret = xfs_vma_checks(vmf->vma, inode);
+ if (ret == 0)
+ ret = dax_iomap_fault(vmf, pe_size, &xfs_iomap_ops);
xfs_iunlock(XFS_I(inode), XFS_MMAPLOCK_SHARED);
if (vmf->flags & FAULT_FLAG_WRITE)
@@ -1137,6 +1169,61 @@ xfs_filemap_pfn_mkwrite(
}
+STATIC void
+xfs_filemap_direct_open(
+ struct vm_area_struct *vma)
+{
+ struct file *filp = vma->vm_file;
+ struct inode *inode = file_inode(filp);
+ struct xfs_inode *ip = XFS_I(inode);
+
+ atomic_inc(&ip->i_mapdcount);
+}
+
+STATIC int
+atomic_dec_and_xfs_ilock(
+ atomic_t *atomic,
+ struct xfs_inode *ip,
+ uint lock_flags)
+{
+ /* Subtract 1 from counter unless that drops it to 0 (ie. it was 1) */
+ if (atomic_add_unless(atomic, -1, 1))
+ return 0;
+
+ /* Otherwise do it the slow way */
+ xfs_ilock(ip, lock_flags);
+ if (atomic_dec_and_test(atomic))
+ return 1;
+ xfs_iunlock(ip, lock_flags);
+ return 0;
+}
+
+STATIC void
+xfs_filemap_direct_close(
+ struct vm_area_struct *vma)
+{
+ struct file *filp = vma->vm_file;
+ struct inode *inode = file_inode(filp);
+ struct xfs_inode *ip = XFS_I(inode);
+
+ if (!atomic_dec_and_xfs_ilock(&ip->i_mapdcount, ip,
+ XFS_MMAPLOCK_EXCL | XFS_IOLOCK_EXCL))
+ return;
+ inode->i_flags &= ~S_IOMAP_SEALED;
+ xfs_iunlock(ip, XFS_MMAPLOCK_EXCL | XFS_IOLOCK_EXCL);
+}
+
+static const struct vm_operations_struct xfs_file_vm_direct_ops = {
+ .fault = xfs_filemap_fault,
+ .huge_fault = xfs_filemap_huge_fault,
+ .map_pages = filemap_map_pages,
+ .page_mkwrite = xfs_filemap_page_mkwrite,
+ .pfn_mkwrite = xfs_filemap_pfn_mkwrite,
+
+ .open = xfs_filemap_direct_open,
+ .close = xfs_filemap_direct_close,
+};
+
static const struct vm_operations_struct xfs_file_vm_ops = {
.fault = xfs_filemap_fault,
.huge_fault = xfs_filemap_huge_fault,
@@ -1145,14 +1232,33 @@ static const struct vm_operations_struct xfs_file_vm_ops = {
.pfn_mkwrite = xfs_filemap_pfn_mkwrite,
};
+#define XFS_MAP_SUPPORTED (LEGACY_MAP_SUPPORTED_MASK | MAP_DIRECT)
+
STATIC int
-xfs_file_mmap(struct file *filp, struct vm_area_struct *vma,
- unsigned long map_flags)
+xfs_file_mmap(
+ struct file *filp,
+ struct vm_area_struct *vma,
+ unsigned long map_flags)
{
+ struct inode *inode = file_inode(filp);
+ struct xfs_inode *ip = XFS_I(inode);
+
+ if (map_flags & ~(XFS_MAP_SUPPORTED))
+ return -EOPNOTSUPP;
+
file_accessed(filp);
- vma->vm_ops = &xfs_file_vm_ops;
if (IS_DAX(file_inode(filp)))
vma->vm_flags |= VM_MIXEDMAP | VM_HUGEPAGE;
+
+ if (map_flags & MAP_DIRECT) {
+ xfs_ilock(ip, XFS_MMAPLOCK_EXCL|XFS_IOLOCK_EXCL);
+ vma->vm_ops = &xfs_file_vm_direct_ops;
+ inode->i_flags |= S_IOMAP_SEALED;
+ atomic_inc(&ip->i_mapdcount);
+ xfs_iunlock(ip, XFS_MMAPLOCK_EXCL|XFS_IOLOCK_EXCL);
+ } else
+ vma->vm_ops = &xfs_file_vm_ops;
+
return 0;
}
@@ -1174,6 +1280,7 @@ const struct file_operations xfs_file_operations = {
.fallocate = xfs_file_fallocate,
.clone_file_range = xfs_file_clone_range,
.dedupe_file_range = xfs_file_dedupe_range,
+ .mmap_supported_mask = XFS_MAP_SUPPORTED,
};
const struct file_operations xfs_dir_file_operations = {
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 0ee453de239a..50d3e1bca1a9 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -58,6 +58,7 @@ typedef struct xfs_inode {
mrlock_t i_lock; /* inode lock */
mrlock_t i_mmaplock; /* inode mmap IO lock */
atomic_t i_pincount; /* inode pin count */
+ atomic_t i_mapdcount; /* inode MAP_DIRECT count */
spinlock_t i_flags_lock; /* inode i_flags lock */
/* Miscellaneous state. */
unsigned long i_flags; /* see defined flags below */
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 38aaacdbb8b3..88711e01e504 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1011,6 +1011,7 @@ xfs_fs_inode_init_once(
/* xfs inode */
atomic_set(&ip->i_pincount, 0);
+ atomic_set(&ip->i_mapdcount, 0);
spin_lock_init(&ip->i_flags_lock);
mrlock_init(&ip->i_mmaplock, MRLOCK_ALLOW_EQUAL_PRI|MRLOCK_BARRIER,
diff --git a/include/linux/mman.h b/include/linux/mman.h
index 64b6cb3dec70..4bebb4ca0f7b 100644
--- a/include/linux/mman.h
+++ b/include/linux/mman.h
@@ -20,6 +20,9 @@
#ifndef MAP_HUGE_1GB
#define MAP_HUGE_1GB 0
#endif
+#ifndef MAP_DIRECT
+#define MAP_DIRECT 0
+#endif
#ifndef MAP_UNINITIALIZED
#define MAP_UNINITIALIZED 0
#endif
@@ -47,7 +50,8 @@
| MAP_HUGE_2MB \
| MAP_HUGE_1GB)
-#define MAP_SUPPORTED_MASK (LEGACY_MAP_SUPPORTED_MASK)
+#define MAP_SUPPORTED_MASK (LEGACY_MAP_SUPPORTED_MASK \
+ | MAP_DIRECT)
extern int sysctl_overcommit_memory;
extern int sysctl_overcommit_ratio;
diff --git a/include/uapi/asm-generic/mman.h b/include/uapi/asm-generic/mman.h
index 7162cd4cca73..1e7dda3bc56a 100644
--- a/include/uapi/asm-generic/mman.h
+++ b/include/uapi/asm-generic/mman.h
@@ -12,6 +12,7 @@
#define MAP_NONBLOCK 0x10000 /* do not block on IO */
#define MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */
#define MAP_HUGETLB 0x40000 /* create a huge page mapping */
+#define MAP_DIRECT 0x80000 /* shared, sealed, and no page cache */
/* Bits [26:31] are reserved, see mman-common.h for MAP_HUGETLB usage */
diff --git a/mm/mmap.c b/mm/mmap.c
index 386706831d67..32417b2a668c 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1393,6 +1393,17 @@ unsigned long do_mmap(struct file *file, unsigned long addr,
return -EACCES;
/*
+ * Require write access and the immutable
+ * capability for MAP_DIRECT mappings
+ */
+ if (flags & MAP_DIRECT) {
+ if (!(prot & PROT_WRITE))
+ return -EACCES;
+ if (!capable(CAP_LINUX_IMMUTABLE))
+ return -EACCES;
+ }
+
+ /*
* Make sure we don't allow writing to an append-only
* file..
*/
@@ -1411,6 +1422,9 @@ unsigned long do_mmap(struct file *file, unsigned long addr,
/* fall through */
case MAP_PRIVATE:
+ if ((flags & (MAP_PRIVATE|MAP_DIRECT))
+ == (MAP_PRIVATE|MAP_DIRECT))
+ return -EINVAL;
if (!(file->f_mode & FMODE_READ))
return -EACCES;
if (path_noexec(&file->f_path)) {
@@ -1448,6 +1462,9 @@ unsigned long do_mmap(struct file *file, unsigned long addr,
default:
return -EINVAL;
}
+
+ if (flags & MAP_DIRECT)
+ return -EINVAL;
}
/*
@@ -1525,6 +1542,12 @@ SYSCALL_DEFINE6(mmap_pgoff_strict, unsigned long, addr, unsigned long, len,
unsigned long, prot, unsigned long, flags,
unsigned long, fd, unsigned long, pgoff)
{
+ /*
+ * since mmap flag definitions are spread over several files,
+ * sanity check new definitions here.
+ */
+ BUILD_BUG_ON((MAP_DIRECT & ~LEGACY_MAP_SUPPORTED_MASK) != MAP_DIRECT);
+
if (flags & ~(MAP_SUPPORTED_MASK))
return -EOPNOTSUPP;
[toc] | [prev] | [next] | [standalone]
| From | Christoph Hellwig <hch@lst.de> |
|---|---|
| Date | 2017-08-24 18:20 +0200 |
| Subject | Re: [PATCH v6 4/5] fs, xfs: introduce MAP_DIRECT for creating block-map-atomic file ranges |
| Message-ID | <ui31n-pa-5@gated-at.bofh.it> |
| In reply to | #1718726 |
I still can't make any sense of this description. What is an external agent? Userspace obviously can't ever see a change in the extent map, so it can't be meant. It would help a lot if you could come up with a concrete user for this, including example code.
[toc] | [prev] | [next] | [standalone]
| From | Christoph Hellwig <hch@lst.de> |
|---|---|
| Date | 2017-08-24 18:40 +0200 |
| Subject | Re: [PATCH v6 4/5] fs, xfs: introduce MAP_DIRECT for creating block-map-atomic file ranges |
| Message-ID | <ui3kM-wC-53@gated-at.bofh.it> |
| In reply to | #1719379 |
On Thu, Aug 24, 2017 at 09:31:17AM -0700, Dan Williams wrote: > External agent is a DMA device, or a hypervisor like Xen. In the DMA > case perhaps we can use the fcntl lease mechanism, I'll investigate. > In the Xen case it actually would need to use fiemap() to discover the > physical addresses that back the file to setup their M2P tables. > Here's the discussion where we discovered that physical address > dependency: > > https://lists.xen.org/archives/html/xen-devel/2017-04/msg00419.html fiemap does not work to discover physical addresses. If they want to do anything involving physical address they will need a kernel driver.
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-24 22:30 +0200 |
| Subject | Re: [PATCH v6 4/5] fs, xfs: introduce MAP_DIRECT for creating block-map-atomic file ranges |
| Message-ID | <ui6Vk-2MH-23@gated-at.bofh.it> |
| In reply to | #1719420 |
On Thu, Aug 24, 2017 at 9:39 AM, Christoph Hellwig <hch@lst.de> wrote: > On Thu, Aug 24, 2017 at 09:31:17AM -0700, Dan Williams wrote: >> External agent is a DMA device, or a hypervisor like Xen. In the DMA >> case perhaps we can use the fcntl lease mechanism, I'll investigate. >> In the Xen case it actually would need to use fiemap() to discover the >> physical addresses that back the file to setup their M2P tables. >> Here's the discussion where we discovered that physical address >> dependency: >> >> https://lists.xen.org/archives/html/xen-devel/2017-04/msg00419.html > > fiemap does not work to discover physical addresses. If they want > to do anything involving physical address they will need a kernel > driver. True, it's broken with respect to multi-device filesystems and these patches do nothing to fix that problem. Ok, I'm fine to let that use case depend on a kernel driver and just focus on fixing the DMA case.
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-24 18:40 +0200 |
| Subject | Re: [PATCH v6 4/5] fs, xfs: introduce MAP_DIRECT for creating block-map-atomic file ranges |
| Message-ID | <ui3kM-wC-55@gated-at.bofh.it> |
| In reply to | #1719379 |
[ adding Xen ]
On Thu, Aug 24, 2017 at 9:11 AM, Christoph Hellwig <hch@lst.de> wrote:
> I still can't make any sense of this description. What is an external
> agent? Userspace obviously can't ever see a change in the extent
> map, so it can't be meant.
External agent is a DMA device, or a hypervisor like Xen. In the DMA
case perhaps we can use the fcntl lease mechanism, I'll investigate.
In the Xen case it actually would need to use fiemap() to discover the
physical addresses that back the file to setup their M2P tables.
Here's the discussion where we discovered that physical address
dependency:
https://lists.xen.org/archives/html/xen-devel/2017-04/msg00419.html
> It would help a lot if you could come up with a concrete user for this,
> including example code.
Will do.
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-24 02:00 +0200 |
| Subject | [PATCH v6 5/5] fs, fcntl: add F_MAP_DIRECT |
| Message-ID | <uhNJ0-78j-15@gated-at.bofh.it> |
| In reply to | #1718723 |
A common pattern for granting a privilege to an unprivileged process is
to pass it an established file descriptor over a unix domain socket.
This enables fine grained access to the MAP_DIRECT mechanism instead of
requiring the mapping process have CAP_LINUX_IMMUTABLE.
Cc: Jan Kara <jack@suse.cz>
Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Dave Chinner <david@fromorbit.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
fs/fcntl.c | 15 +++++++++++++++
include/linux/fs.h | 5 +++--
include/uapi/linux/fcntl.h | 5 +++++
mm/mmap.c | 3 ++-
4 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 3b01b646e528..f2375c406e6f 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -318,6 +318,18 @@ static long fcntl_rw_hint(struct file *file, unsigned int cmd,
}
}
+static int fcntl_map_direct(struct file *filp)
+{
+ if (!capable(CAP_LINUX_IMMUTABLE))
+ return -EACCES;
+
+ spin_lock(&filp->f_lock);
+ filp->f_map_direct = 1;
+ spin_unlock(&filp->f_lock);
+
+ return 0;
+}
+
static long do_fcntl(int fd, unsigned int cmd, unsigned long arg,
struct file *filp)
{
@@ -425,6 +437,9 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long arg,
case F_SET_FILE_RW_HINT:
err = fcntl_rw_hint(filp, cmd, arg);
break;
+ case F_MAP_DIRECT:
+ err = fcntl_map_direct(filp);
+ break;
default:
break;
}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index db42da9f98c4..ec2e1d6bf22c 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -855,11 +855,12 @@ struct file {
const struct file_operations *f_op;
/*
- * Protects f_ep_links, f_flags.
+ * Protects f_ep_links, f_flags, f_write_hint, and f_map_direct.
* Must not be taken from IRQ context.
*/
spinlock_t f_lock;
- enum rw_hint f_write_hint;
+ enum rw_hint f_write_hint:3;
+ unsigned int f_map_direct:1;
atomic_long_t f_count;
unsigned int f_flags;
fmode_t f_mode;
diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
index ec69d55bcec7..2a57a503174e 100644
--- a/include/uapi/linux/fcntl.h
+++ b/include/uapi/linux/fcntl.h
@@ -53,6 +53,11 @@
#define F_SET_FILE_RW_HINT (F_LINUX_SPECIFIC_BASE + 14)
/*
+ * Enable MAP_DIRECT on the file without CAP_LINUX_IMMUTABLE
+ */
+#define F_MAP_DIRECT (F_LINUX_SPECIFIC_BASE + 15)
+
+/*
* Valid hint values for F_{GET,SET}_RW_HINT. 0 is "not set", or can be
* used to clear any hints previously set.
*/
diff --git a/mm/mmap.c b/mm/mmap.c
index 32417b2a668c..cf5e0cb7d0e3 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1399,7 +1399,8 @@ unsigned long do_mmap(struct file *file, unsigned long addr,
if (flags & MAP_DIRECT) {
if (!(prot & PROT_WRITE))
return -EACCES;
- if (!capable(CAP_LINUX_IMMUTABLE))
+ if (!file->f_map_direct
+ && !capable(CAP_LINUX_IMMUTABLE))
return -EACCES;
}
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-24 02:00 +0200 |
| Subject | [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags |
| Message-ID | <uhNJ0-78j-21@gated-at.bofh.it> |
| In reply to | #1718723 |
The mmap(2) syscall suffers from the ABI anti-pattern of not validating
unknown flags. However, proposals like MAP_SYNC and MAP_DIRECT need a
mechanism to define new behavior that is known to fail on older kernels
without the support. Define a new mmap3 syscall that checks for
unsupported flags at syscall entry and add a 'mmap_supported_mask' to
'struct file_operations' so generic code can validate the ->mmap()
handler knows about the specified flags. This also arranges for the
flags to be passed to the handler so it can do further local validation
if the requested behavior can be fulfilled.
Cc: Jan Kara <jack@suse.cz>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Suggested-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
include/linux/fs.h | 1 +
include/linux/mm.h | 2 +-
include/linux/mman.h | 42 ++++++++++++++++++++++++++++++++
include/linux/syscalls.h | 3 ++
mm/mmap.c | 32 ++++++++++++++++++++++--
7 files changed, 78 insertions(+), 4 deletions(-)
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 448ac2161112..0618b5b38b45 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -391,3 +391,4 @@
382 i386 pkey_free sys_pkey_free
383 i386 statx sys_statx
384 i386 arch_prctl sys_arch_prctl compat_sys_arch_prctl
+385 i386 mmap3 sys_mmap_pgoff_strict
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 5aef183e2f85..e204c736d7e9 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -339,6 +339,7 @@
330 common pkey_alloc sys_pkey_alloc
331 common pkey_free sys_pkey_free
332 common statx sys_statx
+333 common mmap3 sys_mmap_pgoff_strict
#
# x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 33d1ee8f51be..db42da9f98c4 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1674,6 +1674,7 @@ struct file_operations {
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
int (*mmap) (struct file *, struct vm_area_struct *, unsigned long);
+ unsigned long mmap_supported_mask;
int (*open) (struct inode *, struct file *);
int (*flush) (struct file *, fl_owner_t id);
int (*release) (struct inode *, struct file *);
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 46b9ac5e8569..49eef48da4b7 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2090,7 +2090,7 @@ extern unsigned long get_unmapped_area(struct file *, unsigned long, unsigned lo
extern unsigned long mmap_region(struct file *file, unsigned long addr,
unsigned long len, vm_flags_t vm_flags, unsigned long pgoff,
- struct list_head *uf);
+ struct list_head *uf, unsigned long flags);
extern unsigned long do_mmap(struct file *file, unsigned long addr,
unsigned long len, unsigned long prot, unsigned long flags,
vm_flags_t vm_flags, unsigned long pgoff, unsigned long *populate,
diff --git a/include/linux/mman.h b/include/linux/mman.h
index c8367041fafd..64b6cb3dec70 100644
--- a/include/linux/mman.h
+++ b/include/linux/mman.h
@@ -7,6 +7,48 @@
#include <linux/atomic.h>
#include <uapi/linux/mman.h>
+/*
+ * Arrange for undefined architecture specific flags to be rejected by
+ * default.
+ */
+#ifndef MAP_32BIT
+#define MAP_32BIT 0
+#endif
+#ifndef MAP_HUGE_2MB
+#define MAP_HUGE_2MB 0
+#endif
+#ifndef MAP_HUGE_1GB
+#define MAP_HUGE_1GB 0
+#endif
+#ifndef MAP_UNINITIALIZED
+#define MAP_UNINITIALIZED 0
+#endif
+
+/*
+ * The historical set of flags that all mmap implementations implicitly
+ * support when file_operations.mmap_supported_mask is zero. With the
+ * mmap3 syscall the deprecated MAP_DENYWRITE and MAP_EXECUTABLE bit
+ * values are explicitly rejected with EOPNOTSUPP rather than being
+ * silently accepted.
+ */
+#define LEGACY_MAP_SUPPORTED_MASK (MAP_SHARED \
+ | MAP_PRIVATE \
+ | MAP_FIXED \
+ | MAP_ANONYMOUS \
+ | MAP_UNINITIALIZED \
+ | MAP_GROWSDOWN \
+ | MAP_LOCKED \
+ | MAP_NORESERVE \
+ | MAP_POPULATE \
+ | MAP_NONBLOCK \
+ | MAP_STACK \
+ | MAP_HUGETLB \
+ | MAP_32BIT \
+ | MAP_HUGE_2MB \
+ | MAP_HUGE_1GB)
+
+#define MAP_SUPPORTED_MASK (LEGACY_MAP_SUPPORTED_MASK)
+
extern int sysctl_overcommit_memory;
extern int sysctl_overcommit_ratio;
extern unsigned long sysctl_overcommit_kbytes;
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 3cb15ea48aee..c0e0c99cf4ad 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -858,6 +858,9 @@ asmlinkage long sys_perf_event_open(
asmlinkage long sys_mmap_pgoff(unsigned long addr, unsigned long len,
unsigned long prot, unsigned long flags,
unsigned long fd, unsigned long pgoff);
+asmlinkage long sys_mmap_pgoff_strict(unsigned long addr, unsigned long len,
+ unsigned long prot, unsigned long flags,
+ unsigned long fd, unsigned long pgoff);
asmlinkage long sys_old_mmap(struct mmap_arg_struct __user *arg);
asmlinkage long sys_name_to_handle_at(int dfd, const char __user *name,
struct file_handle __user *handle,
diff --git a/mm/mmap.c b/mm/mmap.c
index 744faae86781..386706831d67 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1464,7 +1464,7 @@ unsigned long do_mmap(struct file *file, unsigned long addr,
vm_flags |= VM_NORESERVE;
}
- addr = mmap_region(file, addr, len, vm_flags, pgoff, uf);
+ addr = mmap_region(file, addr, len, vm_flags, pgoff, uf, flags);
if (!IS_ERR_VALUE(addr) &&
((vm_flags & VM_LOCKED) ||
(flags & (MAP_POPULATE | MAP_NONBLOCK)) == MAP_POPULATE))
@@ -1521,6 +1521,32 @@ SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
return retval;
}
+SYSCALL_DEFINE6(mmap_pgoff_strict, unsigned long, addr, unsigned long, len,
+ unsigned long, prot, unsigned long, flags,
+ unsigned long, fd, unsigned long, pgoff)
+{
+ if (flags & ~(MAP_SUPPORTED_MASK))
+ return -EOPNOTSUPP;
+
+ if (!(flags & MAP_ANONYMOUS)) {
+ unsigned long f_supported;
+ struct file *file;
+
+ audit_mmap_fd(fd, flags);
+ file = fget(fd);
+ if (!file)
+ return -EBADF;
+ f_supported = file->f_op->mmap_supported_mask;
+ fput(file);
+ if (!f_supported)
+ f_supported = LEGACY_MAP_SUPPORTED_MASK;
+ if (flags & ~f_supported)
+ return -EOPNOTSUPP;
+ }
+
+ return sys_mmap_pgoff(addr, len, prot, flags, fd, pgoff);
+}
+
#ifdef __ARCH_WANT_SYS_OLD_MMAP
struct mmap_arg_struct {
unsigned long addr;
@@ -1601,7 +1627,7 @@ static inline int accountable_mapping(struct file *file, vm_flags_t vm_flags)
unsigned long mmap_region(struct file *file, unsigned long addr,
unsigned long len, vm_flags_t vm_flags, unsigned long pgoff,
- struct list_head *uf)
+ struct list_head *uf, unsigned long flags)
{
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma, *prev;
@@ -1686,7 +1712,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
* new file must not have been exposed to user-space, yet.
*/
vma->vm_file = get_file(file);
- error = call_mmap(file, vma, 0);
+ error = call_mmap(file, vma, flags);
if (error)
goto unmap_and_free_vma;
[toc] | [prev] | [next] | [standalone]
| From | Jan Kara <jack@suse.cz> |
|---|---|
| Date | 2017-08-24 15:10 +0200 |
| Subject | Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags |
| Message-ID | <ui03w-70q-13@gated-at.bofh.it> |
| In reply to | #1718729 |
On Wed 23-08-17 16:48:51, Dan Williams wrote:
> The mmap(2) syscall suffers from the ABI anti-pattern of not validating
> unknown flags. However, proposals like MAP_SYNC and MAP_DIRECT need a
> mechanism to define new behavior that is known to fail on older kernels
> without the support. Define a new mmap3 syscall that checks for
> unsupported flags at syscall entry and add a 'mmap_supported_mask' to
> 'struct file_operations' so generic code can validate the ->mmap()
> handler knows about the specified flags. This also arranges for the
> flags to be passed to the handler so it can do further local validation
> if the requested behavior can be fulfilled.
>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Suggested-by: Andy Lutomirski <luto@kernel.org>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
OK, are we sold on this approach to introduction of new mmap flags? I'm
asking because working API for mmap flag is basically the only thing that's
missing from my MAP_SYNC patches so I'd like to rebase my patches onto
something that is working...
Honza
> ---
> arch/x86/entry/syscalls/syscall_32.tbl | 1 +
> arch/x86/entry/syscalls/syscall_64.tbl | 1 +
> include/linux/fs.h | 1 +
> include/linux/mm.h | 2 +-
> include/linux/mman.h | 42 ++++++++++++++++++++++++++++++++
> include/linux/syscalls.h | 3 ++
> mm/mmap.c | 32 ++++++++++++++++++++++--
> 7 files changed, 78 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
> index 448ac2161112..0618b5b38b45 100644
> --- a/arch/x86/entry/syscalls/syscall_32.tbl
> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
> @@ -391,3 +391,4 @@
> 382 i386 pkey_free sys_pkey_free
> 383 i386 statx sys_statx
> 384 i386 arch_prctl sys_arch_prctl compat_sys_arch_prctl
> +385 i386 mmap3 sys_mmap_pgoff_strict
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> index 5aef183e2f85..e204c736d7e9 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -339,6 +339,7 @@
> 330 common pkey_alloc sys_pkey_alloc
> 331 common pkey_free sys_pkey_free
> 332 common statx sys_statx
> +333 common mmap3 sys_mmap_pgoff_strict
>
> #
> # x32-specific system call numbers start at 512 to avoid cache impact
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 33d1ee8f51be..db42da9f98c4 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1674,6 +1674,7 @@ struct file_operations {
> long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
> long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
> int (*mmap) (struct file *, struct vm_area_struct *, unsigned long);
> + unsigned long mmap_supported_mask;
> int (*open) (struct inode *, struct file *);
> int (*flush) (struct file *, fl_owner_t id);
> int (*release) (struct inode *, struct file *);
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 46b9ac5e8569..49eef48da4b7 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2090,7 +2090,7 @@ extern unsigned long get_unmapped_area(struct file *, unsigned long, unsigned lo
>
> extern unsigned long mmap_region(struct file *file, unsigned long addr,
> unsigned long len, vm_flags_t vm_flags, unsigned long pgoff,
> - struct list_head *uf);
> + struct list_head *uf, unsigned long flags);
> extern unsigned long do_mmap(struct file *file, unsigned long addr,
> unsigned long len, unsigned long prot, unsigned long flags,
> vm_flags_t vm_flags, unsigned long pgoff, unsigned long *populate,
> diff --git a/include/linux/mman.h b/include/linux/mman.h
> index c8367041fafd..64b6cb3dec70 100644
> --- a/include/linux/mman.h
> +++ b/include/linux/mman.h
> @@ -7,6 +7,48 @@
> #include <linux/atomic.h>
> #include <uapi/linux/mman.h>
>
> +/*
> + * Arrange for undefined architecture specific flags to be rejected by
> + * default.
> + */
> +#ifndef MAP_32BIT
> +#define MAP_32BIT 0
> +#endif
> +#ifndef MAP_HUGE_2MB
> +#define MAP_HUGE_2MB 0
> +#endif
> +#ifndef MAP_HUGE_1GB
> +#define MAP_HUGE_1GB 0
> +#endif
> +#ifndef MAP_UNINITIALIZED
> +#define MAP_UNINITIALIZED 0
> +#endif
> +
> +/*
> + * The historical set of flags that all mmap implementations implicitly
> + * support when file_operations.mmap_supported_mask is zero. With the
> + * mmap3 syscall the deprecated MAP_DENYWRITE and MAP_EXECUTABLE bit
> + * values are explicitly rejected with EOPNOTSUPP rather than being
> + * silently accepted.
> + */
> +#define LEGACY_MAP_SUPPORTED_MASK (MAP_SHARED \
> + | MAP_PRIVATE \
> + | MAP_FIXED \
> + | MAP_ANONYMOUS \
> + | MAP_UNINITIALIZED \
> + | MAP_GROWSDOWN \
> + | MAP_LOCKED \
> + | MAP_NORESERVE \
> + | MAP_POPULATE \
> + | MAP_NONBLOCK \
> + | MAP_STACK \
> + | MAP_HUGETLB \
> + | MAP_32BIT \
> + | MAP_HUGE_2MB \
> + | MAP_HUGE_1GB)
> +
> +#define MAP_SUPPORTED_MASK (LEGACY_MAP_SUPPORTED_MASK)
> +
> extern int sysctl_overcommit_memory;
> extern int sysctl_overcommit_ratio;
> extern unsigned long sysctl_overcommit_kbytes;
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 3cb15ea48aee..c0e0c99cf4ad 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -858,6 +858,9 @@ asmlinkage long sys_perf_event_open(
> asmlinkage long sys_mmap_pgoff(unsigned long addr, unsigned long len,
> unsigned long prot, unsigned long flags,
> unsigned long fd, unsigned long pgoff);
> +asmlinkage long sys_mmap_pgoff_strict(unsigned long addr, unsigned long len,
> + unsigned long prot, unsigned long flags,
> + unsigned long fd, unsigned long pgoff);
> asmlinkage long sys_old_mmap(struct mmap_arg_struct __user *arg);
> asmlinkage long sys_name_to_handle_at(int dfd, const char __user *name,
> struct file_handle __user *handle,
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 744faae86781..386706831d67 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1464,7 +1464,7 @@ unsigned long do_mmap(struct file *file, unsigned long addr,
> vm_flags |= VM_NORESERVE;
> }
>
> - addr = mmap_region(file, addr, len, vm_flags, pgoff, uf);
> + addr = mmap_region(file, addr, len, vm_flags, pgoff, uf, flags);
> if (!IS_ERR_VALUE(addr) &&
> ((vm_flags & VM_LOCKED) ||
> (flags & (MAP_POPULATE | MAP_NONBLOCK)) == MAP_POPULATE))
> @@ -1521,6 +1521,32 @@ SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
> return retval;
> }
>
> +SYSCALL_DEFINE6(mmap_pgoff_strict, unsigned long, addr, unsigned long, len,
> + unsigned long, prot, unsigned long, flags,
> + unsigned long, fd, unsigned long, pgoff)
> +{
> + if (flags & ~(MAP_SUPPORTED_MASK))
> + return -EOPNOTSUPP;
> +
> + if (!(flags & MAP_ANONYMOUS)) {
> + unsigned long f_supported;
> + struct file *file;
> +
> + audit_mmap_fd(fd, flags);
> + file = fget(fd);
> + if (!file)
> + return -EBADF;
> + f_supported = file->f_op->mmap_supported_mask;
> + fput(file);
> + if (!f_supported)
> + f_supported = LEGACY_MAP_SUPPORTED_MASK;
> + if (flags & ~f_supported)
> + return -EOPNOTSUPP;
> + }
> +
> + return sys_mmap_pgoff(addr, len, prot, flags, fd, pgoff);
> +}
> +
> #ifdef __ARCH_WANT_SYS_OLD_MMAP
> struct mmap_arg_struct {
> unsigned long addr;
> @@ -1601,7 +1627,7 @@ static inline int accountable_mapping(struct file *file, vm_flags_t vm_flags)
>
> unsigned long mmap_region(struct file *file, unsigned long addr,
> unsigned long len, vm_flags_t vm_flags, unsigned long pgoff,
> - struct list_head *uf)
> + struct list_head *uf, unsigned long flags)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma, *prev;
> @@ -1686,7 +1712,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
> * new file must not have been exposed to user-space, yet.
> */
> vma->vm_file = get_file(file);
> - error = call_mmap(file, vma, 0);
> + error = call_mmap(file, vma, flags);
> if (error)
> goto unmap_and_free_vma;
>
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
[toc] | [prev] | [next] | [standalone]
| From | Christoph Hellwig <hch@infradead.org> |
|---|---|
| Date | 2017-08-24 19:00 +0200 |
| Subject | Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags |
| Message-ID | <ui3E6-Db-23@gated-at.bofh.it> |
| In reply to | #1718729 |
On Wed, Aug 23, 2017 at 04:48:51PM -0700, Dan Williams wrote: > The mmap(2) syscall suffers from the ABI anti-pattern of not validating > unknown flags. However, proposals like MAP_SYNC and MAP_DIRECT need a > mechanism to define new behavior that is known to fail on older kernels > without the support. Define a new mmap3 syscall that checks for > unsupported flags at syscall entry and add a 'mmap_supported_mask' to > 'struct file_operations' so generic code can validate the ->mmap() > handler knows about the specified flags. This also arranges for the > flags to be passed to the handler so it can do further local validation > if the requested behavior can be fulfilled. What is the reason to not go with __MAP_VALID hack? Adding new syscalls is extremely painful, it will take forever to trickle this through all architectures (especially with the various 32-bit architectures having all kinds of different granularities for the offset) and then the various C libraries, never mind applications.
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-24 19:40 +0200 |
| Subject | Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags |
| Message-ID | <ui4gP-17m-33@gated-at.bofh.it> |
| In reply to | #1719440 |
On Thu, Aug 24, 2017 at 9:55 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Wed, Aug 23, 2017 at 04:48:51PM -0700, Dan Williams wrote:
>> The mmap(2) syscall suffers from the ABI anti-pattern of not validating
>> unknown flags. However, proposals like MAP_SYNC and MAP_DIRECT need a
>> mechanism to define new behavior that is known to fail on older kernels
>> without the support. Define a new mmap3 syscall that checks for
>> unsupported flags at syscall entry and add a 'mmap_supported_mask' to
>> 'struct file_operations' so generic code can validate the ->mmap()
>> handler knows about the specified flags. This also arranges for the
>> flags to be passed to the handler so it can do further local validation
>> if the requested behavior can be fulfilled.
>
> What is the reason to not go with __MAP_VALID hack? Adding new
> syscalls is extremely painful, it will take forever to trickle this
> through all architectures (especially with the various 32-bit
> architectures having all kinds of different granularities for the
> offset) and then the various C libraries, never mind applications.
I'll let Andy and Kirill restate their concerns, but one of the
arguments that swayed me is that any new mmap flag with this hack must
be documented to only work with MAP_SHARED and that MAP_PRIVATE is
silently ignored. I agree with the mess and delays it causes for other
archs and libc, but at the same time this is for new applications and
libraries that know to look for the new flag, so they need to do the
extra work to check for the new syscall.
However, if the fcntl lease approach works for the DMA cases then we
only have the one mmap flag to add for now, so maybe the weird
MAP_{SHARED|PRIVATE} semantics are tolerable.
[toc] | [prev] | [next] | [standalone]
| From | Christoph Hellwig <hch@infradead.org> |
|---|---|
| Date | 2017-08-25 15:10 +0200 |
| Subject | Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags |
| Message-ID | <uimx3-4tc-7@gated-at.bofh.it> |
| In reply to | #1719467 |
On Thu, Aug 24, 2017 at 10:36:02AM -0700, Dan Williams wrote:
> I'll let Andy and Kirill restate their concerns, but one of the
> arguments that swayed me is that any new mmap flag with this hack must
> be documented to only work with MAP_SHARED and that MAP_PRIVATE is
> silently ignored. I agree with the mess and delays it causes for other
> archs and libc, but at the same time this is for new applications and
> libraries that know to look for the new flag, so they need to do the
> extra work to check for the new syscall.
True. That is for the original hack, but I spent some more time
looking at the mmap code, and there is one thing I noticed:
include/uapi/asm-generic/mman-common.h:
#define MAP_SHARED 0x01 /* Share changes */
#define MAP_PRIVATE 0x02 /* Changes are private */
#define MAP_TYPE 0x0f /* Mask for type of mapping */
mm/mmap.c:
if (file) {
struct inode *inode = file_inode(file);
switch (flags & MAP_TYPE) {
case MAP_SHARED:
...
case MAP_PRIVATE:
...
default:
return -EINVAL;
}
and very similar for the anonymous and nommu cases.
So if we pick e.g. 0x4 as the valid bit we don't even need to overload
the MAP_SHARED and MAP_PRIVATE meaning.
>
> However, if the fcntl lease approach works for the DMA cases then we
> only have the one mmap flag to add for now, so maybe the weird
> MAP_{SHARED|PRIVATE} semantics are tolerable.
---end quoted text---
[toc] | [prev] | [next] | [standalone]
| From | "Kirill A. Shutemov" <kirill@shutemov.name> |
|---|---|
| Date | 2017-08-25 18:00 +0200 |
| Subject | Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags |
| Message-ID | <uipbz-5V8-7@gated-at.bofh.it> |
| In reply to | #1720041 |
On Fri, Aug 25, 2017 at 06:00:11AM -0700, Christoph Hellwig wrote:
> On Thu, Aug 24, 2017 at 10:36:02AM -0700, Dan Williams wrote:
> > I'll let Andy and Kirill restate their concerns, but one of the
> > arguments that swayed me is that any new mmap flag with this hack must
> > be documented to only work with MAP_SHARED and that MAP_PRIVATE is
> > silently ignored. I agree with the mess and delays it causes for other
> > archs and libc, but at the same time this is for new applications and
> > libraries that know to look for the new flag, so they need to do the
> > extra work to check for the new syscall.
>
> True. That is for the original hack, but I spent some more time
> looking at the mmap code, and there is one thing I noticed:
>
> include/uapi/asm-generic/mman-common.h:
>
> #define MAP_SHARED 0x01 /* Share changes */
> #define MAP_PRIVATE 0x02 /* Changes are private */
> #define MAP_TYPE 0x0f /* Mask for type of mapping */
>
> mm/mmap.c:
>
> if (file) {
> struct inode *inode = file_inode(file);
>
> switch (flags & MAP_TYPE) {
> case MAP_SHARED:
> ...
> case MAP_PRIVATE:
> ...
> default:
> return -EINVAL;
> }
>
> and very similar for the anonymous and nommu cases.
>
> So if we pick e.g. 0x4 as the valid bit we don't even need to overload
> the MAP_SHARED and MAP_PRIVATE meaning.
Not all archs are ready for this:
arch/parisc/include/uapi/asm/mman.h:#define MAP_TYPE 0x03 /* Mask for type of mapping */
arch/parisc/include/uapi/asm/mman.h:#define MAP_FIXED 0x04 /* Interpret addr exactly */
--
Kirill A. Shutemov
[toc] | [prev] | [next] | [standalone]
| From | Christoph Hellwig <hch@infradead.org> |
|---|---|
| Date | 2017-08-25 18:10 +0200 |
| Subject | Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags |
| Message-ID | <uiplh-6dJ-41@gated-at.bofh.it> |
| In reply to | #1720205 |
On Fri, Aug 25, 2017 at 06:58:03PM +0300, Kirill A. Shutemov wrote: > Not all archs are ready for this: > > arch/parisc/include/uapi/asm/mman.h:#define MAP_TYPE 0x03 /* Mask for type of mapping */ > arch/parisc/include/uapi/asm/mman.h:#define MAP_FIXED 0x04 /* Interpret addr exactly */ I'd be happy to say that we should not care about parisc for persistent memory. We'll just have to find a way to exclude parisc without making life too ugly.
[toc] | [prev] | [next] | [standalone]
| From | "Kirill A. Shutemov" <kirill@shutemov.name> |
|---|---|
| Date | 2017-08-25 18:20 +0200 |
| Subject | Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags |
| Message-ID | <uipuW-6gP-27@gated-at.bofh.it> |
| In reply to | #1720230 |
On Fri, Aug 25, 2017 at 09:02:36AM -0700, Christoph Hellwig wrote: > On Fri, Aug 25, 2017 at 06:58:03PM +0300, Kirill A. Shutemov wrote: > > Not all archs are ready for this: > > > > arch/parisc/include/uapi/asm/mman.h:#define MAP_TYPE 0x03 /* Mask for type of mapping */ > > arch/parisc/include/uapi/asm/mman.h:#define MAP_FIXED 0x04 /* Interpret addr exactly */ > > I'd be happy to say that we should not care about parisc for > persistent memory. We'll just have to find a way to exclude > parisc without making life too ugly. I don't think creapling mmap() interface for one arch is the right way to go. I think the interface should be universal. I may imagine MAP_DIRECT can be useful not only for persistent memory. For tmpfs instead of mlock()? -- Kirill A. Shutemov
[toc] | [prev] | [next] | [standalone]
| From | Helge Deller <deller@gmx.de> |
|---|---|
| Date | 2017-08-25 18:30 +0200 |
| Subject | Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags |
| Message-ID | <uipEB-6ki-1@gated-at.bofh.it> |
| In reply to | #1720241 |
On 25.08.2017 18:16, Kirill A. Shutemov wrote: > On Fri, Aug 25, 2017 at 09:02:36AM -0700, Christoph Hellwig wrote: >> On Fri, Aug 25, 2017 at 06:58:03PM +0300, Kirill A. Shutemov wrote: >>> Not all archs are ready for this: >>> >>> arch/parisc/include/uapi/asm/mman.h:#define MAP_TYPE 0x03 /* Mask for type of mapping */ >>> arch/parisc/include/uapi/asm/mman.h:#define MAP_FIXED 0x04 /* Interpret addr exactly */ >> >> I'd be happy to say that we should not care about parisc for >> persistent memory. We'll just have to find a way to exclude >> parisc without making life too ugly. > > I don't think creapling mmap() interface for one arch is the right way to > go. I think the interface should be universal. > > I may imagine MAP_DIRECT can be useful not only for persistent memory. > For tmpfs instead of mlock()? On parisc we have #define MAP_SHARED 0x01 /* Share changes */ #define MAP_PRIVATE 0x02 /* Changes are private */ #define MAP_TYPE 0x03 /* Mask for type of mapping */ #define MAP_FIXED 0x04 /* Interpret addr exactly */ #define MAP_ANONYMOUS 0x10 /* don't use a file */ So, if you need a MAP_DIRECT, wouldn't e.g. #define MAP_DIRECT 0x08 be possible (for parisc, and others 0x04). And if MAP_TYPE needs to include this flag on parisc: #define MAP_TYPE (0x03 | 0x08) /* Mask for type of mapping */ Helge
[toc] | [prev] | [next] | [standalone]
| From | "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> |
|---|---|
| Date | 2017-08-25 19:00 +0200 |
| Subject | Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags |
| Message-ID | <uiq7D-6u6-3@gated-at.bofh.it> |
| In reply to | #1720244 |
On Fri, Aug 25, 2017 at 04:19:19PM +0000, Helge Deller wrote: > On 25.08.2017 18:16, Kirill A. Shutemov wrote: > > On Fri, Aug 25, 2017 at 09:02:36AM -0700, Christoph Hellwig wrote: > >> On Fri, Aug 25, 2017 at 06:58:03PM +0300, Kirill A. Shutemov wrote: > >>> Not all archs are ready for this: > >>> > >>> arch/parisc/include/uapi/asm/mman.h:#define MAP_TYPE 0x03 /* Mask for type of mapping */ > >>> arch/parisc/include/uapi/asm/mman.h:#define MAP_FIXED 0x04 /* Interpret addr exactly */ > >> > >> I'd be happy to say that we should not care about parisc for > >> persistent memory. We'll just have to find a way to exclude > >> parisc without making life too ugly. > > > > I don't think creapling mmap() interface for one arch is the right way to > > go. I think the interface should be universal. > > > > I may imagine MAP_DIRECT can be useful not only for persistent memory. > > For tmpfs instead of mlock()? > > On parisc we have > #define MAP_SHARED 0x01 /* Share changes */ > #define MAP_PRIVATE 0x02 /* Changes are private */ > #define MAP_TYPE 0x03 /* Mask for type of mapping */ > #define MAP_FIXED 0x04 /* Interpret addr exactly */ > #define MAP_ANONYMOUS 0x10 /* don't use a file */ > > So, if you need a MAP_DIRECT, wouldn't e.g. > #define MAP_DIRECT 0x08 > be possible (for parisc, and others 0x04). > And if MAP_TYPE needs to include this flag on parisc: > #define MAP_TYPE (0x03 | 0x08) /* Mask for type of mapping */ I guess it's better to re-define MAP_TYPE as 0x3 everywhere and make MAP_DIRECT a normal flag. It's not new type of mapping anyway. -- Kirill A. Shutemov
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-25 22:30 +0200 |
| Subject | Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags |
| Message-ID | <uitoS-ek-11@gated-at.bofh.it> |
| In reply to | #1720244 |
On Fri, Aug 25, 2017 at 9:19 AM, Helge Deller <deller@gmx.de> wrote: > On 25.08.2017 18:16, Kirill A. Shutemov wrote: >> On Fri, Aug 25, 2017 at 09:02:36AM -0700, Christoph Hellwig wrote: >>> On Fri, Aug 25, 2017 at 06:58:03PM +0300, Kirill A. Shutemov wrote: >>>> Not all archs are ready for this: >>>> >>>> arch/parisc/include/uapi/asm/mman.h:#define MAP_TYPE 0x03 /* Mask for type of mapping */ >>>> arch/parisc/include/uapi/asm/mman.h:#define MAP_FIXED 0x04 /* Interpret addr exactly */ >>> >>> I'd be happy to say that we should not care about parisc for >>> persistent memory. We'll just have to find a way to exclude >>> parisc without making life too ugly. >> >> I don't think creapling mmap() interface for one arch is the right way to >> go. I think the interface should be universal. >> >> I may imagine MAP_DIRECT can be useful not only for persistent memory. >> For tmpfs instead of mlock()? > > On parisc we have > #define MAP_SHARED 0x01 /* Share changes */ > #define MAP_PRIVATE 0x02 /* Changes are private */ > #define MAP_TYPE 0x03 /* Mask for type of mapping */ > #define MAP_FIXED 0x04 /* Interpret addr exactly */ > #define MAP_ANONYMOUS 0x10 /* don't use a file */ > > So, if you need a MAP_DIRECT, wouldn't e.g. > #define MAP_DIRECT 0x08 > be possible (for parisc, and others 0x04). > And if MAP_TYPE needs to include this flag on parisc: > #define MAP_TYPE (0x03 | 0x08) /* Mask for type of mapping */ The problem here is that to support new the mmap flags the arch needs to find a flag that is guaranteed to fail on older kernels. Defining MAP_DIRECT to 0x8 on parisc doesn't work because it will simply be ignored on older parisc kernels. However, it's already the case that several archs have their own sys_mmap entry points. Those archs that can't follow the common scheme (only parsic it seems) will need to add a new mmap syscall. I think that's a reasonable tradeoff to allow every other architecture to add this support with their existing mmap syscall paths. That means MAP_DIRECT should be defined to MAP_TYPE on parisc until it later defines an opt-in mechanism to a new syscall that honors MAP_DIRECT as a valid flag.
[toc] | [prev] | [next] | [standalone]
| From | Helge Deller <deller@gmx.de> |
|---|---|
| Date | 2017-08-26 09:50 +0200 |
| Subject | Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags |
| Message-ID | <uiE0V-6NN-5@gated-at.bofh.it> |
| In reply to | #1720396 |
* Dan Williams <dan.j.williams@intel.com>:
> On Fri, Aug 25, 2017 at 9:19 AM, Helge Deller <deller@gmx.de> wrote:
> > On 25.08.2017 18:16, Kirill A. Shutemov wrote:
> >> On Fri, Aug 25, 2017 at 09:02:36AM -0700, Christoph Hellwig wrote:
> >>> On Fri, Aug 25, 2017 at 06:58:03PM +0300, Kirill A. Shutemov wrote:
> >>>> Not all archs are ready for this:
> >>>>
> >>>> arch/parisc/include/uapi/asm/mman.h:#define MAP_TYPE 0x03 /* Mask for type of mapping */
> >>>> arch/parisc/include/uapi/asm/mman.h:#define MAP_FIXED 0x04 /* Interpret addr exactly */
> >>>
> >>> I'd be happy to say that we should not care about parisc for
> >>> persistent memory. We'll just have to find a way to exclude
> >>> parisc without making life too ugly.
> >>
> >> I don't think creapling mmap() interface for one arch is the right way to
> >> go. I think the interface should be universal.
> >>
> >> I may imagine MAP_DIRECT can be useful not only for persistent memory.
> >> For tmpfs instead of mlock()?
> >
> > On parisc we have
> > #define MAP_SHARED 0x01 /* Share changes */
> > #define MAP_PRIVATE 0x02 /* Changes are private */
> > #define MAP_TYPE 0x03 /* Mask for type of mapping */
> > #define MAP_FIXED 0x04 /* Interpret addr exactly */
> > #define MAP_ANONYMOUS 0x10 /* don't use a file */
> >
> > So, if you need a MAP_DIRECT, wouldn't e.g.
> > #define MAP_DIRECT 0x08
> > be possible (for parisc, and others 0x04).
> > And if MAP_TYPE needs to include this flag on parisc:
> > #define MAP_TYPE (0x03 | 0x08) /* Mask for type of mapping */
>
> The problem here is that to support new the mmap flags the arch needs
> to find a flag that is guaranteed to fail on older kernels. Defining
> MAP_DIRECT to 0x8 on parisc doesn't work because it will simply be
> ignored on older parisc kernels.
>
> However, it's already the case that several archs have their own
> sys_mmap entry points. Those archs that can't follow the common scheme
> (only parsic it seems) will need to add a new mmap syscall. I think
> that's a reasonable tradeoff to allow every other architecture to add
> this support with their existing mmap syscall paths.
I don't want other architectures to suffer just because of parisc.
But adding a new syscall just for usage on parisc won't work either,
because nobody will add code to call it then.
> That means MAP_DIRECT should be defined to MAP_TYPE on parisc until it
> later defines an opt-in mechanism to a new syscall that honors
> MAP_DIRECT as a valid flag.
I'd instead propose to to introduce an ABI breakage for parisc users
(which aren't many). Most parisc users update their kernel regularily
anyway, because we fixed so many bugs in the latest kernel.
With the following patch pushed down to the stable kernel series,
MAP_DIRECT will fail as expected on those kernels, while we can
keep parisc up with current developments regarding MAP_DIRECT.
diff --git a/arch/parisc/include/uapi/asm/mman.h b/arch/parisc/include/uapi/asm/mman.h
index 9a9c2fe..43b9a1e 100644
--- a/arch/parisc/include/uapi/asm/mman.h
+++ b/arch/parisc/include/uapi/asm/mman.h
@@ -13,6 +13,7 @@
#define MAP_PRIVATE 0x02 /* Changes are private */
#define MAP_TYPE 0x03 /* Mask for type of mapping */
#define MAP_FIXED 0x04 /* Interpret addr exactly */
+#define MAP_DIRECT 0x08 /* Interpret addr exactly */
#define MAP_ANONYMOUS 0x10 /* don't use a file */
#define MAP_DENYWRITE 0x0800 /* ETXTBSY */
diff --git a/arch/parisc/kernel/sys_parisc.c b/arch/parisc/kernel/sys_parisc.c
index 378a754..0499f87 100644
--- a/arch/parisc/kernel/sys_parisc.c
+++ b/arch/parisc/kernel/sys_parisc.c
@@ -270,6 +270,10 @@ asmlinkage unsigned long sys_mmap2(unsigned long addr, unsigned long len,
{
/* Make sure the shift for mmap2 is constant (12), no matter what PAGE_SIZE
we have. */
+#if !defined(CONFIG_HAVE_MAP_DIRECT_SUPPORT)
+ if (flags & MAP_DIRECT)
+ return -EINVAL;
+#endif
return sys_mmap_pgoff(addr, len, prot, flags, fd,
pgoff >> (PAGE_SHIFT - 12));
}
@@ -278,6 +282,10 @@ asmlinkage unsigned long sys_mmap(unsigned long addr, unsigned long len,
unsigned long prot, unsigned long flags, unsigned long fd,
unsigned long offset)
{
+#if !defined(CONFIG_HAVE_MAP_DIRECT_SUPPORT)
+ if (flags & MAP_DIRECT)
+ return -EINVAL;
+#endif
if (!(offset & ~PAGE_MASK)) {
return sys_mmap_pgoff(addr, len, prot, flags, fd,
offset >> PAGE_SHIFT);
Helge
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-26 17:20 +0200 |
| Subject | Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags |
| Message-ID | <uiL2p-2ME-3@gated-at.bofh.it> |
| In reply to | #1720569 |
On Sat, Aug 26, 2017 at 12:40 AM, Helge Deller <deller@gmx.de> wrote: > * Dan Williams <dan.j.williams@intel.com>: >> On Fri, Aug 25, 2017 at 9:19 AM, Helge Deller <deller@gmx.de> wrote: >> > On 25.08.2017 18:16, Kirill A. Shutemov wrote: >> >> On Fri, Aug 25, 2017 at 09:02:36AM -0700, Christoph Hellwig wrote: >> >>> On Fri, Aug 25, 2017 at 06:58:03PM +0300, Kirill A. Shutemov wrote: >> >>>> Not all archs are ready for this: >> >>>> >> >>>> arch/parisc/include/uapi/asm/mman.h:#define MAP_TYPE 0x03 /* Mask for type of mapping */ >> >>>> arch/parisc/include/uapi/asm/mman.h:#define MAP_FIXED 0x04 /* Interpret addr exactly */ >> >>> >> >>> I'd be happy to say that we should not care about parisc for >> >>> persistent memory. We'll just have to find a way to exclude >> >>> parisc without making life too ugly. >> >> >> >> I don't think creapling mmap() interface for one arch is the right way to >> >> go. I think the interface should be universal. >> >> >> >> I may imagine MAP_DIRECT can be useful not only for persistent memory. >> >> For tmpfs instead of mlock()? >> > >> > On parisc we have >> > #define MAP_SHARED 0x01 /* Share changes */ >> > #define MAP_PRIVATE 0x02 /* Changes are private */ >> > #define MAP_TYPE 0x03 /* Mask for type of mapping */ >> > #define MAP_FIXED 0x04 /* Interpret addr exactly */ >> > #define MAP_ANONYMOUS 0x10 /* don't use a file */ >> > >> > So, if you need a MAP_DIRECT, wouldn't e.g. >> > #define MAP_DIRECT 0x08 >> > be possible (for parisc, and others 0x04). >> > And if MAP_TYPE needs to include this flag on parisc: >> > #define MAP_TYPE (0x03 | 0x08) /* Mask for type of mapping */ >> >> The problem here is that to support new the mmap flags the arch needs >> to find a flag that is guaranteed to fail on older kernels. Defining >> MAP_DIRECT to 0x8 on parisc doesn't work because it will simply be >> ignored on older parisc kernels. >> >> However, it's already the case that several archs have their own >> sys_mmap entry points. Those archs that can't follow the common scheme >> (only parsic it seems) will need to add a new mmap syscall. I think >> that's a reasonable tradeoff to allow every other architecture to add >> this support with their existing mmap syscall paths. > > I don't want other architectures to suffer just because of parisc. > But adding a new syscall just for usage on parisc won't work either, > because nobody will add code to call it then. I don't understand this comment, if / when parisc gets around to adding pmem and dax support why wouldn't libc grow support for the new parisc mmap variant? Also, it's not just MAP_DIRECT you would also need space for a MAP_SYNC flag. >> That means MAP_DIRECT should be defined to MAP_TYPE on parisc until it >> later defines an opt-in mechanism to a new syscall that honors >> MAP_DIRECT as a valid flag. > > I'd instead propose to to introduce an ABI breakage for parisc users > (which aren't many). Most parisc users update their kernel regularily > anyway, because we fixed so many bugs in the latest kernel. > > With the following patch pushed down to the stable kernel series, > MAP_DIRECT will fail as expected on those kernels, while we can > keep parisc up with current developments regarding MAP_DIRECT. The whole point is to avoid an ABI regression and the chance for false positive results. We're immediately stuck if some application was expecting 0x8 to be ignored, or conversely an application that absolutely needs to rely on MAP_SYNC/MAP_DIRECT semantics assumes the wrong result on a parisc kernel where they are ignored. I have not seen any patches for parisc pmem+dax enabling so it seems too early to worry about these "last mile" enabling features of MAP_DIRECT and MAP_SYNC. In particular parisc doesn't appear to have ARCH_ENABLE_MEMORY_HOTPLUG, so as far as I can see it can't yet support the ZONE_DEVICE scheme that is a pre-requisite for MAP_DIRECT.
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web