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 | 11 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 2 of 2 — ← Prev page 1 [2]
| From | Helge Deller <deller@gmx.de> |
|---|---|
| Date | 2017-08-26 22:00 +0200 |
| Subject | Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags |
| Message-ID | <uiPpo-5s1-5@gated-at.bofh.it> |
| In reply to | #1720674 |
On 26.08.2017 17:15, Dan Williams wrote: > 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. I see, but then it's probably best to not to define any MAP_DIRECT or MAP_SYNC at all in the headers of those arches which don't support pmem+dax (parisc, m68k, alpha, and probably quite some others). That way applications can detect at configure time if the platform supports that, and can leave out the functionality completely. Helge
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-27 00:50 +0200 |
| Subject | Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags |
| Message-ID | <uiS3U-7cv-11@gated-at.bofh.it> |
| In reply to | #1720700 |
On Sat, Aug 26, 2017 at 12:50 PM, Helge Deller <deller@gmx.de> wrote: > On 26.08.2017 17:15, Dan Williams wrote: [..] >> 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. > > I see, but then it's probably best to not to define any MAP_DIRECT or > MAP_SYNC at all in the headers of those arches which don't support > pmem+dax (parisc, m68k, alpha, and probably quite some others). > That way applications can detect at configure time if the platform > supports that, and can leave out the functionality completely. Yes, that's a good idea we can handle this similar to CONFIG_MMAP_ALLOW_UNINITIALIZED. These patches will also modify 'struct file_operations' so that do_mmap() can validate whether a flag is supported on per architecture basis. Also the plan is to plumb the flags passed to the syscall all the way down to the individual mmap implementations. The ext4 and xfs ->mmap() operations will be able to return -EOPNOTSUP based on runtime variables.
[toc] | [prev] | [next] | [standalone]
| From | "Kirill A. Shutemov" <kirill@shutemov.name> |
|---|---|
| Date | 2017-08-27 02:00 +0200 |
| Subject | Re: [PATCH v6 3/5] mm: introduce mmap3 for safely defining new mmap flags |
| Message-ID | <uiT9E-7SW-11@gated-at.bofh.it> |
| In reply to | #1720716 |
On Sat, Aug 26, 2017 at 03:46:22PM -0700, Dan Williams wrote: > On Sat, Aug 26, 2017 at 12:50 PM, Helge Deller <deller@gmx.de> wrote: > > On 26.08.2017 17:15, Dan Williams wrote: > [..] > >> 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. > > > > I see, but then it's probably best to not to define any MAP_DIRECT or > > MAP_SYNC at all in the headers of those arches which don't support > > pmem+dax (parisc, m68k, alpha, and probably quite some others). > > That way applications can detect at configure time if the platform > > supports that, and can leave out the functionality completely. > > Yes, that's a good idea we can handle this similar to > CONFIG_MMAP_ALLOW_UNINITIALIZED. These patches will also modify > 'struct file_operations' so that do_mmap() can validate whether a flag > is supported on per architecture basis. Also the plan is to plumb the > flags passed to the syscall all the way down to the individual mmap > implementations. The ext4 and xfs ->mmap() operations will be able to > return -EOPNOTSUP based on runtime variables. BTW, we may be able to reuse the bit used for MAP_UNINITIALIZED -- it's only used on !MMU machines. -- Kirill A. Shutemov
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-24 02:00 +0200 |
| Subject | [PATCH v6 2/5] fs, xfs: introduce S_IOMAP_SEALED |
| Message-ID | <uhNJ0-78j-23@gated-at.bofh.it> |
| In reply to | #1718723 |
When a filesystem sees this flag set it will not allow changes to the
file-offset to physical-block-offset relationship of any extent in the
file. The extent of the extents covered by the global S_IOMAP_SEALED is
filesystem specific. In other words it is similar to the inode-wide
XFS_DIFLAG2_REFLINK flag where we make the distinction apply globally to
the inode even though we could theoretically limit that effect to a
sub-range of the file.
The interface that sets this flag (mmap(..., MAP_DIRECT, ...)) will be
careful to document that it is implementation specific whether the
'sealed' restrictions apply to a sub-range or the whole file.
Applications should be prepared for unrelated ranges in the file to be
effected.
The term 'sealed' is used instead of 'immutable' to better indicate that
this is a file property that is temporary and can be undone.
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/attr.c | 10 ++++++++++
fs/open.c | 6 ++++++
fs/read_write.c | 3 +++
fs/xfs/libxfs/xfs_bmap.c | 5 +++++
fs/xfs/xfs_bmap_util.c | 3 +++
fs/xfs/xfs_ioctl.c | 6 ++++++
include/linux/fs.h | 2 ++
mm/filemap.c | 5 +++++
8 files changed, 40 insertions(+)
diff --git a/fs/attr.c b/fs/attr.c
index 135304146120..d940386e0ca9 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -112,6 +112,16 @@ EXPORT_SYMBOL(setattr_prepare);
*/
int inode_newsize_ok(const struct inode *inode, loff_t offset)
{
+ if (IS_IOMAP_SEALED(inode)) {
+ /*
+ * Any size change is disallowed. Size increases may
+ * dirty metadata that an application is not prepared to
+ * sync, and a size decrease may expose free blocks to
+ * in-flight DMA.
+ */
+ return -ETXTBSY;
+ }
+
if (inode->i_size < offset) {
unsigned long limit;
diff --git a/fs/open.c b/fs/open.c
index 35bb784763a4..92d89ec2d6b3 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -292,6 +292,12 @@ int vfs_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
return -ETXTBSY;
/*
+ * We cannot allow any allocation changes on an iomap sealed file
+ */
+ if (IS_IOMAP_SEALED(inode))
+ return -ETXTBSY;
+
+ /*
* Revalidate the write permissions, in case security policy has
* changed since the files were opened.
*/
diff --git a/fs/read_write.c b/fs/read_write.c
index 0cc7033aa413..55700ca85f7e 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1706,6 +1706,9 @@ int vfs_clone_file_prep_inodes(struct inode *inode_in, loff_t pos_in,
if (IS_SWAPFILE(inode_in) || IS_SWAPFILE(inode_out))
return -ETXTBSY;
+ if (IS_IOMAP_SEALED(inode_in) || IS_IOMAP_SEALED(inode_out))
+ return -ETXTBSY;
+
/* Don't reflink dirs, pipes, sockets... */
if (S_ISDIR(inode_in->i_mode) || S_ISDIR(inode_out->i_mode))
return -EISDIR;
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index c09c16b1ad3b..241f3a272f49 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4481,6 +4481,11 @@ xfs_bmapi_write(
if (XFS_FORCED_SHUTDOWN(mp))
return -EIO;
+ /* fail any attempts to mutate data extents */
+ if (IS_IOMAP_SEALED(VFS_I(ip))
+ && !(flags & (XFS_BMAPI_METADATA | XFS_BMAPI_ATTRFORK)))
+ return -ETXTBSY;
+
ifp = XFS_IFORK_PTR(ip, whichfork);
XFS_STATS_INC(mp, xs_blk_mapw);
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 93e955262d07..ef4c4e8b0f58 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -1294,6 +1294,9 @@ xfs_free_file_space(
trace_xfs_free_file_space(ip);
+ if (IS_IOMAP_SEALED(VFS_I(ip)))
+ return -ETXTBSY;
+
error = xfs_qm_dqattach(ip, 0);
if (error)
return error;
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 9c0c7a920304..845587e6928b 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1730,6 +1730,12 @@ xfs_ioc_swapext(
goto out_put_tmp_file;
}
+ if (IS_IOMAP_SEALED(file_inode(f.file)) ||
+ IS_IOMAP_SEALED(file_inode(tmp.file))) {
+ error = -EINVAL;
+ goto out_put_tmp_file;
+ }
+
/*
* We need to ensure that the fds passed in point to XFS inodes
* before we cast and access them as XFS structures as we have no
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 47249bbe973c..33d1ee8f51be 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1830,6 +1830,7 @@ struct super_operations {
#else
#define S_DAX 0 /* Make all the DAX code disappear */
#endif
+#define S_IOMAP_SEALED 16384 /* logical-to-physical extent map is fixed */
/*
* Note that nosuid etc flags are inode-specific: setting some file-system
@@ -1868,6 +1869,7 @@ struct super_operations {
#define IS_AUTOMOUNT(inode) ((inode)->i_flags & S_AUTOMOUNT)
#define IS_NOSEC(inode) ((inode)->i_flags & S_NOSEC)
#define IS_DAX(inode) ((inode)->i_flags & S_DAX)
+#define IS_IOMAP_SEALED(inode) ((inode)->i_flags & S_IOMAP_SEALED)
#define IS_WHITEOUT(inode) (S_ISCHR(inode->i_mode) && \
(inode)->i_rdev == WHITEOUT_DEV)
diff --git a/mm/filemap.c b/mm/filemap.c
index 2457e34d10e0..4cbcf9d589fa 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2810,6 +2810,11 @@ inline ssize_t generic_write_checks(struct kiocb *iocb, struct iov_iter *from)
if (unlikely(pos >= inode->i_sb->s_maxbytes))
return -EFBIG;
+ /* Are we about to mutate the block map on a sealed file? */
+ if (IS_IOMAP_SEALED(inode)
+ && (pos + iov_iter_count(from) > i_size_read(inode)))
+ return -ETXTBSY;
+
iov_iter_truncate(from, inode->i_sb->s_maxbytes - pos);
return iov_iter_count(from);
}
[toc] | [prev] | [next] | [standalone]
| From | Christoph Hellwig <hch@lst.de> |
|---|---|
| Date | 2017-08-24 18:20 +0200 |
| Subject | Re: [PATCH v6 2/5] fs, xfs: introduce S_IOMAP_SEALED |
| Message-ID | <ui31n-pa-3@gated-at.bofh.it> |
| In reply to | #1718730 |
I'm still very unhappy about the get/set flag state. What is the reason you can't use/extend leases? (take a look at the fcntl man page and look for Leases). A variant of the concept is what the pNFS block server uses.
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-25 08:10 +0200 |
| Subject | Re: [PATCH v6 2/5] fs, xfs: introduce S_IOMAP_SEALED |
| Message-ID | <uifYB-nL-1@gated-at.bofh.it> |
| In reply to | #1719378 |
On Thu, Aug 24, 2017 at 9:13 AM, Christoph Hellwig <hch@lst.de> wrote: > I'm still very unhappy about the get/set flag state. What is the > reason you can't use/extend leases? (take a look at the fcntl > man page and look for Leases). A variant of the concept is what > the pNFS block server uses. So I think leases could potentially be extended to replace the inode flag. A MAP_DIRECT operation would take out a lease that is broken by break_layouts(). However, like the pNFS case the lease break would need to held off while any DMA might be in-flight. We can use an elevated page count as that indication as ZONE_DEVICE pages only ever have an elevated page count in response to get_user_pages(). However, I think the only practical difference is turning an immediate ETXTBSY response that S_IOMAP_SEALED provides into an indefinite blocking wait for break_layouts() to complete. Can pNFS run break_layouts() in bounded time? As far I can see a lease and S_IOMAP_SEALED have the same DMA cancelling problem, so a lease is not better in that regard. Absent an overlaying protocol like pNFS, I think S_IOMAP_SEALED is cleaner because it fails incompatible operations outright rather than stalls them in break_layouts(). Were their other benefits to a lease over an inode flag that you had in mind for this case where the protocol is userspace defined? Maybe I'm thinking too small on the ways a lease might be extended.
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-25 21:50 +0200 |
| Subject | Re: [PATCH v6 2/5] fs, xfs: introduce S_IOMAP_SEALED |
| Message-ID | <uisM9-8cd-13@gated-at.bofh.it> |
| In reply to | #1719765 |
On Thu, Aug 24, 2017 at 11:00 PM, Dan Williams <dan.j.williams@intel.com> wrote: > On Thu, Aug 24, 2017 at 9:13 AM, Christoph Hellwig <hch@lst.de> wrote: >> I'm still very unhappy about the get/set flag state. What is the >> reason you can't use/extend leases? (take a look at the fcntl >> man page and look for Leases). A variant of the concept is what >> the pNFS block server uses. > > So I think leases could potentially be extended to replace the inode > flag. A MAP_DIRECT operation would take out a lease that is broken by > break_layouts(). However, like the pNFS case the lease break would > need to held off while any DMA might be in-flight. We can use an > elevated page count as that indication as ZONE_DEVICE pages only ever > have an elevated page count in response to get_user_pages(). > > However, I think the only practical difference is turning an immediate > ETXTBSY response that S_IOMAP_SEALED provides into an indefinite > blocking wait for break_layouts() to complete. Can pNFS run > break_layouts() in bounded time? > > As far I can see a lease and S_IOMAP_SEALED have the same DMA > cancelling problem, so a lease is not better in that regard. Absent an > overlaying protocol like pNFS, I think S_IOMAP_SEALED is cleaner > because it fails incompatible operations outright rather than stalls > them in break_layouts(). Were their other benefits to a lease over an > inode flag that you had in mind for this case where the protocol is > userspace defined? Maybe I'm thinking too small on the ways a lease > might be extended. At a minimum I can at least use a new lease type as an indication of when to bail out an block-map operation with ETXTBSY, and reuse the lease security model. That way we at least start to converge the in-kernel lease machinery for pinning blocks with this userspace mechanism.
[toc] | [prev] | [next] | [standalone]
| From | Christoph Hellwig <hch@lst.de> |
|---|---|
| Date | 2017-08-24 18:10 +0200 |
| Message-ID | <ui2RH-kG-5@gated-at.bofh.it> |
| In reply to | #1718723 |
This seems to be missing patches 1 and 3.
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-24 18:30 +0200 |
| Message-ID | <ui3b4-tc-23@gated-at.bofh.it> |
| In reply to | #1719365 |
On Thu, Aug 24, 2017 at 9:08 AM, Christoph Hellwig <hch@lst.de> wrote: > This seems to be missing patches 1 and 3. Sorry, I didn't cc you directly on those. They're on the list: https://patchwork.kernel.org/patch/9918657/ https://patchwork.kernel.org/patch/9918663/
[toc] | [prev] | [next] | [standalone]
| From | Christoph Hellwig <hch@infradead.org> |
|---|---|
| Date | 2017-08-24 19:00 +0200 |
| Subject | Re: [PATCH v6 1/5] vfs: add flags parameter to ->mmap() in 'struct file_operations' |
| Message-ID | <ui3E6-Db-17@gated-at.bofh.it> |
| In reply to | #1718723 |
On Wed, Aug 23, 2017 at 04:48:40PM -0700, Dan Williams wrote: > We are running running short of vma->vm_flags. We can avoid needing a > new VM_* flag in some cases if the original @flags submitted to mmap(2) > is made available to the ->mmap() 'struct file_operations' > implementation. For example, the proposed addition of MAP_DIRECT can be > implemented without taking up a new vm_flags bit. Another motivation to > avoid vm_flags is that they appear in /proc/$pid/smaps, and we have seen > software that tries to dangerously (TOCTOU) read smaps to infer the > behavior of a virtual address range. > > This conversion was performed by the following semantic patch. There > were a few manual edits for oddities like proc_reg_mmap. > > Thanks to Julia for helping me with coccinelle iteration to cover cases > where the mmap routine is defined in a separate file from the 'struct > file_operations' instance that consumes it. How are we going to check that an instance actually supports any of those flags?
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-24 19:50 +0200 |
| Subject | Re: [PATCH v6 1/5] vfs: add flags parameter to ->mmap() in 'struct file_operations' |
| Message-ID | <ui4qu-1bx-3@gated-at.bofh.it> |
| In reply to | #1719437 |
On Thu, Aug 24, 2017 at 9:58 AM, Christoph Hellwig <hch@infradead.org> wrote: > On Wed, Aug 23, 2017 at 04:48:40PM -0700, Dan Williams wrote: >> We are running running short of vma->vm_flags. We can avoid needing a >> new VM_* flag in some cases if the original @flags submitted to mmap(2) >> is made available to the ->mmap() 'struct file_operations' >> implementation. For example, the proposed addition of MAP_DIRECT can be >> implemented without taking up a new vm_flags bit. Another motivation to >> avoid vm_flags is that they appear in /proc/$pid/smaps, and we have seen >> software that tries to dangerously (TOCTOU) read smaps to infer the >> behavior of a virtual address range. >> >> This conversion was performed by the following semantic patch. There >> were a few manual edits for oddities like proc_reg_mmap. >> >> Thanks to Julia for helping me with coccinelle iteration to cover cases >> where the mmap routine is defined in a separate file from the 'struct >> file_operations' instance that consumes it. > > How are we going to check that an instance actually supports any > of those flags? In patch 3 I validate the flags by introducing an "mmap_supported_mask" field to 'struct file_operations'. It will be zero by default for almost all implementations and zero means "support the legacy mmap flags".
[toc] | [prev] | [standalone]
Page 2 of 2 — ← Prev page 1 [2]
Back to top | Article view | linux.kernel
csiph-web