Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1703601 > unrolled thread
| Started by | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| First post | 2017-08-04 04:40 +0200 |
| Last post | 2017-08-04 22:40 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v2 3/5] fs, xfs: introduce FALLOC_FL_UNSEAL_BLOCK_MAP Dan Williams <dan.j.williams@intel.com> - 2017-08-04 04:40 +0200
Re: [PATCH v2 3/5] fs, xfs: introduce FALLOC_FL_UNSEAL_BLOCK_MAP "Darrick J. Wong" <darrick.wong@oracle.com> - 2017-08-04 22:10 +0200
Re: [PATCH v2 3/5] fs, xfs: introduce FALLOC_FL_UNSEAL_BLOCK_MAP Dan Williams <dan.j.williams@intel.com> - 2017-08-04 22:40 +0200
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-04 04:40 +0200 |
| Subject | [PATCH v2 3/5] fs, xfs: introduce FALLOC_FL_UNSEAL_BLOCK_MAP |
| Message-ID | <uaAGR-Pk-1@gated-at.bofh.it> |
Provide an explicit fallocate operation type for clearing the
S_IOMAP_IMMUTABLE flag. Like the enable case it requires CAP_IMMUTABLE
and it can only be performed while no process has the file mapped.
Cc: Jan Kara <jack@suse.cz>
Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
Suggested-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
fs/open.c | 17 +++++++++++------
fs/xfs/xfs_bmap_util.c | 42 ++++++++++++++++++++++++++++++++++++++++++
fs/xfs/xfs_bmap_util.h | 3 +++
fs/xfs/xfs_file.c | 4 +++-
include/linux/falloc.h | 3 ++-
include/uapi/linux/falloc.h | 1 +
6 files changed, 62 insertions(+), 8 deletions(-)
diff --git a/fs/open.c b/fs/open.c
index e3aae59785ae..ccfd8d3becc8 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -274,13 +274,17 @@ int vfs_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
return -EINVAL;
/*
- * Seal block map operation should only be used exclusively, and
- * with the IMMUTABLE capability.
+ * Seal/unseal block map operations should only be used
+ * exclusively, and with the IMMUTABLE capability.
*/
- if (mode & FALLOC_FL_SEAL_BLOCK_MAP) {
+ if (mode & (FALLOC_FL_SEAL_BLOCK_MAP | FALLOC_FL_UNSEAL_BLOCK_MAP)) {
if (!capable(CAP_LINUX_IMMUTABLE))
return -EPERM;
- if (mode & ~FALLOC_FL_SEAL_BLOCK_MAP)
+ if (mode == (FALLOC_FL_SEAL_BLOCK_MAP
+ | FALLOC_FL_UNSEAL_BLOCK_MAP))
+ return -EINVAL;
+ if (mode & ~(FALLOC_FL_SEAL_BLOCK_MAP
+ | FALLOC_FL_UNSEAL_BLOCK_MAP))
return -EINVAL;
}
@@ -303,9 +307,10 @@ 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 immutable file
+ * We cannot allow any allocation changes on an iomap immutable
+ * file, but we can allow clearing the immutable state.
*/
- if (IS_IOMAP_IMMUTABLE(inode))
+ if (IS_IOMAP_IMMUTABLE(inode) && !(mode & FALLOC_FL_UNSEAL_BLOCK_MAP))
return -ETXTBSY;
/*
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 46d8eb9e19fc..70ac2d33ab27 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -1494,6 +1494,48 @@ xfs_seal_file_space(
return error;
}
+int
+xfs_unseal_file_space(
+ struct xfs_inode *ip,
+ xfs_off_t offset,
+ xfs_off_t len)
+{
+ struct inode *inode = VFS_I(ip);
+ struct address_space *mapping = inode->i_mapping;
+ int error;
+
+ ASSERT(xfs_isilocked(ip, XFS_MMAPLOCK_EXCL));
+
+ if (offset)
+ return -EINVAL;
+
+ xfs_ilock(ip, XFS_ILOCK_EXCL);
+ /*
+ * It does not make sense to unseal less than the full range of
+ * the file.
+ */
+ error = -EINVAL;
+ if (len < i_size_read(inode))
+ goto out_unlock;
+
+ /*
+ * Provide safety against one thread changing the policy of not
+ * requiring fsync/msync (for block allocations) behind another
+ * thread's back.
+ */
+ error = -EBUSY;
+ if (mapping_mapped(mapping))
+ goto out_unlock;
+
+ inode->i_flags &= ~S_IOMAP_IMMUTABLE;
+ error = 0;
+
+out_unlock:
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
+
+ return error;
+}
+
/*
* @next_fsb will keep track of the extent currently undergoing shift.
* @stop_fsb will keep track of the extent at which we have to stop.
diff --git a/fs/xfs/xfs_bmap_util.h b/fs/xfs/xfs_bmap_util.h
index 5115a32a2483..b64653a75942 100644
--- a/fs/xfs/xfs_bmap_util.h
+++ b/fs/xfs/xfs_bmap_util.h
@@ -62,6 +62,9 @@ int xfs_insert_file_space(struct xfs_inode *, xfs_off_t offset,
xfs_off_t len);
int xfs_seal_file_space(struct xfs_inode *, xfs_off_t offset,
xfs_off_t len);
+int xfs_unseal_file_space(struct xfs_inode *, xfs_off_t offset,
+ xfs_off_t len);
+
/* EOF block manipulation functions */
bool xfs_can_free_eofblocks(struct xfs_inode *ip, bool force);
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index e21121530a90..833f77700be2 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -740,7 +740,7 @@ xfs_file_write_iter(
(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE | \
FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_ZERO_RANGE | \
FALLOC_FL_INSERT_RANGE | FALLOC_FL_UNSHARE_RANGE | \
- FALLOC_FL_SEAL_BLOCK_MAP)
+ FALLOC_FL_SEAL_BLOCK_MAP | FALLOC_FL_UNSEAL_BLOCK_MAP)
STATIC long
xfs_file_fallocate(
@@ -840,6 +840,8 @@ xfs_file_fallocate(
XFS_BMAPI_PREALLOC);
} else if (mode & FALLOC_FL_SEAL_BLOCK_MAP) {
error = xfs_seal_file_space(ip, offset, len);
+ } else if (mode & FALLOC_FL_UNSEAL_BLOCK_MAP) {
+ error = xfs_unseal_file_space(ip, offset, len);
} else
error = xfs_alloc_file_space(ip, offset, len,
XFS_BMAPI_PREALLOC);
diff --git a/include/linux/falloc.h b/include/linux/falloc.h
index 48546c6fbec7..b22c1368ed1e 100644
--- a/include/linux/falloc.h
+++ b/include/linux/falloc.h
@@ -27,6 +27,7 @@ struct space_resv {
FALLOC_FL_ZERO_RANGE | \
FALLOC_FL_INSERT_RANGE | \
FALLOC_FL_UNSHARE_RANGE | \
- FALLOC_FL_SEAL_BLOCK_MAP)
+ FALLOC_FL_SEAL_BLOCK_MAP | \
+ FALLOC_FL_UNSEAL_BLOCK_MAP)
#endif /* _FALLOC_H_ */
diff --git a/include/uapi/linux/falloc.h b/include/uapi/linux/falloc.h
index 39076975bf6f..a4949e1a2dae 100644
--- a/include/uapi/linux/falloc.h
+++ b/include/uapi/linux/falloc.h
@@ -95,4 +95,5 @@
* with the punch, zero, collapse, or insert range modes.
*/
#define FALLOC_FL_SEAL_BLOCK_MAP 0x080
+#define FALLOC_FL_UNSEAL_BLOCK_MAP 0x100
#endif /* _UAPI_FALLOC_H_ */
[toc] | [next] | [standalone]
| From | "Darrick J. Wong" <darrick.wong@oracle.com> |
|---|---|
| Date | 2017-08-04 22:10 +0200 |
| Message-ID | <uaR50-3eu-17@gated-at.bofh.it> |
| In reply to | #1703601 |
On Thu, Aug 03, 2017 at 07:28:23PM -0700, Dan Williams wrote:
> Provide an explicit fallocate operation type for clearing the
> S_IOMAP_IMMUTABLE flag. Like the enable case it requires CAP_IMMUTABLE
> and it can only be performed while no process has the file mapped.
>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Jeff Moyer <jmoyer@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
> Suggested-by: Dave Chinner <david@fromorbit.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> fs/open.c | 17 +++++++++++------
> fs/xfs/xfs_bmap_util.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> fs/xfs/xfs_bmap_util.h | 3 +++
> fs/xfs/xfs_file.c | 4 +++-
> include/linux/falloc.h | 3 ++-
> include/uapi/linux/falloc.h | 1 +
> 6 files changed, 62 insertions(+), 8 deletions(-)
>
> diff --git a/fs/open.c b/fs/open.c
> index e3aae59785ae..ccfd8d3becc8 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -274,13 +274,17 @@ int vfs_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
> return -EINVAL;
>
> /*
> - * Seal block map operation should only be used exclusively, and
> - * with the IMMUTABLE capability.
> + * Seal/unseal block map operations should only be used
> + * exclusively, and with the IMMUTABLE capability.
> */
> - if (mode & FALLOC_FL_SEAL_BLOCK_MAP) {
> + if (mode & (FALLOC_FL_SEAL_BLOCK_MAP | FALLOC_FL_UNSEAL_BLOCK_MAP)) {
> if (!capable(CAP_LINUX_IMMUTABLE))
> return -EPERM;
> - if (mode & ~FALLOC_FL_SEAL_BLOCK_MAP)
> + if (mode == (FALLOC_FL_SEAL_BLOCK_MAP
> + | FALLOC_FL_UNSEAL_BLOCK_MAP))
> + return -EINVAL;
> + if (mode & ~(FALLOC_FL_SEAL_BLOCK_MAP
> + | FALLOC_FL_UNSEAL_BLOCK_MAP))
> return -EINVAL;
> }
>
> @@ -303,9 +307,10 @@ 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 immutable file
> + * We cannot allow any allocation changes on an iomap immutable
> + * file, but we can allow clearing the immutable state.
> */
> - if (IS_IOMAP_IMMUTABLE(inode))
> + if (IS_IOMAP_IMMUTABLE(inode) && !(mode & FALLOC_FL_UNSEAL_BLOCK_MAP))
> return -ETXTBSY;
>
> /*
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 46d8eb9e19fc..70ac2d33ab27 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -1494,6 +1494,48 @@ xfs_seal_file_space(
> return error;
> }
>
> +int
> +xfs_unseal_file_space(
> + struct xfs_inode *ip,
> + xfs_off_t offset,
> + xfs_off_t len)
> +{
> + struct inode *inode = VFS_I(ip);
> + struct address_space *mapping = inode->i_mapping;
> + int error;
> +
> + ASSERT(xfs_isilocked(ip, XFS_MMAPLOCK_EXCL));
Same assert-on-the-iolock comment as the previous patch.
> +
> + if (offset)
> + return -EINVAL;
> +
> + xfs_ilock(ip, XFS_ILOCK_EXCL);
> + /*
> + * It does not make sense to unseal less than the full range of
> + * the file.
> + */
> + error = -EINVAL;
> + if (len < i_size_read(inode))
> + goto out_unlock;
Hmm, should we be picky and require len == i_size_read() here?
> + /*
> + * Provide safety against one thread changing the policy of not
> + * requiring fsync/msync (for block allocations) behind another
> + * thread's back.
> + */
> + error = -EBUSY;
> + if (mapping_mapped(mapping))
> + goto out_unlock;
> +
> + inode->i_flags &= ~S_IOMAP_IMMUTABLE;
It occurred to me, should we jump out early from the seal/unseal
operations if the flag state matches whatever the user is asking for?
This is perhaps not necessary for unseal since we don't do a lot of
work.
--D
> + error = 0;
> +
> +out_unlock:
> + xfs_iunlock(ip, XFS_ILOCK_EXCL);
> +
> + return error;
> +}
> +
> /*
> * @next_fsb will keep track of the extent currently undergoing shift.
> * @stop_fsb will keep track of the extent at which we have to stop.
> diff --git a/fs/xfs/xfs_bmap_util.h b/fs/xfs/xfs_bmap_util.h
> index 5115a32a2483..b64653a75942 100644
> --- a/fs/xfs/xfs_bmap_util.h
> +++ b/fs/xfs/xfs_bmap_util.h
> @@ -62,6 +62,9 @@ int xfs_insert_file_space(struct xfs_inode *, xfs_off_t offset,
> xfs_off_t len);
> int xfs_seal_file_space(struct xfs_inode *, xfs_off_t offset,
> xfs_off_t len);
> +int xfs_unseal_file_space(struct xfs_inode *, xfs_off_t offset,
> + xfs_off_t len);
> +
>
> /* EOF block manipulation functions */
> bool xfs_can_free_eofblocks(struct xfs_inode *ip, bool force);
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index e21121530a90..833f77700be2 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -740,7 +740,7 @@ xfs_file_write_iter(
> (FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE | \
> FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_ZERO_RANGE | \
> FALLOC_FL_INSERT_RANGE | FALLOC_FL_UNSHARE_RANGE | \
> - FALLOC_FL_SEAL_BLOCK_MAP)
> + FALLOC_FL_SEAL_BLOCK_MAP | FALLOC_FL_UNSEAL_BLOCK_MAP)
>
> STATIC long
> xfs_file_fallocate(
> @@ -840,6 +840,8 @@ xfs_file_fallocate(
> XFS_BMAPI_PREALLOC);
> } else if (mode & FALLOC_FL_SEAL_BLOCK_MAP) {
> error = xfs_seal_file_space(ip, offset, len);
> + } else if (mode & FALLOC_FL_UNSEAL_BLOCK_MAP) {
> + error = xfs_unseal_file_space(ip, offset, len);
> } else
> error = xfs_alloc_file_space(ip, offset, len,
> XFS_BMAPI_PREALLOC);
> diff --git a/include/linux/falloc.h b/include/linux/falloc.h
> index 48546c6fbec7..b22c1368ed1e 100644
> --- a/include/linux/falloc.h
> +++ b/include/linux/falloc.h
> @@ -27,6 +27,7 @@ struct space_resv {
> FALLOC_FL_ZERO_RANGE | \
> FALLOC_FL_INSERT_RANGE | \
> FALLOC_FL_UNSHARE_RANGE | \
> - FALLOC_FL_SEAL_BLOCK_MAP)
> + FALLOC_FL_SEAL_BLOCK_MAP | \
> + FALLOC_FL_UNSEAL_BLOCK_MAP)
>
> #endif /* _FALLOC_H_ */
> diff --git a/include/uapi/linux/falloc.h b/include/uapi/linux/falloc.h
> index 39076975bf6f..a4949e1a2dae 100644
> --- a/include/uapi/linux/falloc.h
> +++ b/include/uapi/linux/falloc.h
> @@ -95,4 +95,5 @@
> * with the punch, zero, collapse, or insert range modes.
> */
> #define FALLOC_FL_SEAL_BLOCK_MAP 0x080
> +#define FALLOC_FL_UNSEAL_BLOCK_MAP 0x100
> #endif /* _UAPI_FALLOC_H_ */
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-04 22:40 +0200 |
| Message-ID | <uaRy1-3oq-1@gated-at.bofh.it> |
| In reply to | #1704191 |
On Fri, Aug 4, 2017 at 1:04 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> On Thu, Aug 03, 2017 at 07:28:23PM -0700, Dan Williams wrote:
>> Provide an explicit fallocate operation type for clearing the
>> S_IOMAP_IMMUTABLE flag. Like the enable case it requires CAP_IMMUTABLE
>> and it can only be performed while no process has the file mapped.
>>
>> Cc: Jan Kara <jack@suse.cz>
>> Cc: Jeff Moyer <jmoyer@redhat.com>
>> Cc: Christoph Hellwig <hch@lst.de>
>> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
>> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
>> Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
>> Suggested-by: Dave Chinner <david@fromorbit.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> ---
>> fs/open.c | 17 +++++++++++------
>> fs/xfs/xfs_bmap_util.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>> fs/xfs/xfs_bmap_util.h | 3 +++
>> fs/xfs/xfs_file.c | 4 +++-
>> include/linux/falloc.h | 3 ++-
>> include/uapi/linux/falloc.h | 1 +
>> 6 files changed, 62 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/open.c b/fs/open.c
>> index e3aae59785ae..ccfd8d3becc8 100644
>> --- a/fs/open.c
>> +++ b/fs/open.c
>> @@ -274,13 +274,17 @@ int vfs_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
>> return -EINVAL;
>>
>> /*
>> - * Seal block map operation should only be used exclusively, and
>> - * with the IMMUTABLE capability.
>> + * Seal/unseal block map operations should only be used
>> + * exclusively, and with the IMMUTABLE capability.
>> */
>> - if (mode & FALLOC_FL_SEAL_BLOCK_MAP) {
>> + if (mode & (FALLOC_FL_SEAL_BLOCK_MAP | FALLOC_FL_UNSEAL_BLOCK_MAP)) {
>> if (!capable(CAP_LINUX_IMMUTABLE))
>> return -EPERM;
>> - if (mode & ~FALLOC_FL_SEAL_BLOCK_MAP)
>> + if (mode == (FALLOC_FL_SEAL_BLOCK_MAP
>> + | FALLOC_FL_UNSEAL_BLOCK_MAP))
>> + return -EINVAL;
>> + if (mode & ~(FALLOC_FL_SEAL_BLOCK_MAP
>> + | FALLOC_FL_UNSEAL_BLOCK_MAP))
>> return -EINVAL;
>> }
>>
>> @@ -303,9 +307,10 @@ 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 immutable file
>> + * We cannot allow any allocation changes on an iomap immutable
>> + * file, but we can allow clearing the immutable state.
>> */
>> - if (IS_IOMAP_IMMUTABLE(inode))
>> + if (IS_IOMAP_IMMUTABLE(inode) && !(mode & FALLOC_FL_UNSEAL_BLOCK_MAP))
>> return -ETXTBSY;
>>
>> /*
>> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
>> index 46d8eb9e19fc..70ac2d33ab27 100644
>> --- a/fs/xfs/xfs_bmap_util.c
>> +++ b/fs/xfs/xfs_bmap_util.c
>> @@ -1494,6 +1494,48 @@ xfs_seal_file_space(
>> return error;
>> }
>>
>> +int
>> +xfs_unseal_file_space(
>> + struct xfs_inode *ip,
>> + xfs_off_t offset,
>> + xfs_off_t len)
>> +{
>> + struct inode *inode = VFS_I(ip);
>> + struct address_space *mapping = inode->i_mapping;
>> + int error;
>> +
>> + ASSERT(xfs_isilocked(ip, XFS_MMAPLOCK_EXCL));
>
> Same assert-on-the-iolock comment as the previous patch.
Ok.
>
>> +
>> + if (offset)
>> + return -EINVAL;
>> +
>> + xfs_ilock(ip, XFS_ILOCK_EXCL);
>> + /*
>> + * It does not make sense to unseal less than the full range of
>> + * the file.
>> + */
>> + error = -EINVAL;
>> + if (len < i_size_read(inode))
>> + goto out_unlock;
>
> Hmm, should we be picky and require len == i_size_read() here?
Yes, I think so, otherwise we may have raced someone who increased the
file size with unwritten extents.
>
>> + /*
>> + * Provide safety against one thread changing the policy of not
>> + * requiring fsync/msync (for block allocations) behind another
>> + * thread's back.
>> + */
>> + error = -EBUSY;
>> + if (mapping_mapped(mapping))
>> + goto out_unlock;
>> +
>> + inode->i_flags &= ~S_IOMAP_IMMUTABLE;
>
> It occurred to me, should we jump out early from the seal/unseal
> operations if the flag state matches whatever the user is asking for?
> This is perhaps not necessary for unseal since we don't do a lot of
> work.
>
Yes, I think I had that semantic in v1, but lost in the cleanups. Will
bring it back.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web