On Wed, Aug 21, 2024 at 08:30:29AM +0200, Christoph Hellwig wrote:
> The fallocate system call takes a mode argument, but that argument
> contains a wild mix of exclusive modes and an optional flags.
>
> Replace FALLOC_FL_SUPPORTED_MASK with FALLOC_FL_MODE_MASK, which excludes
> the optional flag bit, so that we can use switch statement on the value
> to easily enumerate the cases while getting the check for duplicate modes
> for free.
>
> To make this (and in the future the file system implementations) more
> readable also add a symbolic name for the 0 mode used to allocate blocks.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/open.c | 53 ++++++++++++++++++-------------------
> include/linux/falloc.h | 18 ++++++++-----
> include/uapi/linux/falloc.h | 1 +
> 3 files changed, 39 insertions(+), 33 deletions(-)
>
> diff --git a/fs/open.c b/fs/open.c
> index 22adbef7ecc2a6..c598d2071c45cc 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -252,42 +252,41 @@ int vfs_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
> if (offset < 0 || len <= 0)
> return -EINVAL;
>
> - /* Return error if mode is not supported */
> - if (mode & ~FALLOC_FL_SUPPORTED_MASK)
> + if (mode & ~(FALLOC_FL_MODE_MASK | FALLOC_FL_KEEP_SIZE))
> return -EOPNOTSUPP;
>
> - /* Punch hole and zero range are mutually exclusive */
> - if ((mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE)) ==
> - (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE))
> - return -EOPNOTSUPP;
> -
> - /* Punch hole must have keep size set */
> - if ((mode & FALLOC_FL_PUNCH_HOLE) &&
> - !(mode & FALLOC_FL_KEEP_SIZE))
> + /*
> + * Modes are exclusive, even if that is not obvious from the encoding
> + * as bit masks and the mix with the flag in the same namespace.
> + *
> + * To make things even more complicated, FALLOC_FL_ALLOCATE_RANGE is
> + * encoded as no bit set.
> + */
> + switch (mode & FALLOC_FL_MODE_MASK) {
> + case FALLOC_FL_ALLOCATE_RANGE:
> + case FALLOC_FL_UNSHARE_RANGE:
> + case FALLOC_FL_ZERO_RANGE:
> + break;
> + case FALLOC_FL_PUNCH_HOLE:
> + if (!(mode & FALLOC_FL_KEEP_SIZE))
> + return -EOPNOTSUPP;
> + break;
> + case FALLOC_FL_COLLAPSE_RANGE:
> + case FALLOC_FL_INSERT_RANGE:
> + if (mode & FALLOC_FL_KEEP_SIZE)
> + return -EOPNOTSUPP;
> + break;
> + default:
> return -EOPNOTSUPP;
> -
> - /* Collapse range should only be used exclusively. */
> - if ((mode & FALLOC_FL_COLLAPSE_RANGE) &&
> - (mode & ~FALLOC_FL_COLLAPSE_RANGE))
> - return -EINVAL;
> -
> - /* Insert range should only be used exclusively. */
> - if ((mode & FALLOC_FL_INSERT_RANGE) &&
> - (mode & ~FALLOC_FL_INSERT_RANGE))
> - return -EINVAL;
> -
> - /* Unshare range should only be used with allocate mode. */
> - if ((mode & FALLOC_FL_UNSHARE_RANGE) &&
> - (mode & ~(FALLOC_FL_UNSHARE_RANGE | FALLOC_FL_KEEP_SIZE)))
> - return -EINVAL;
> + }
>
> if (!(file->f_mode & FMODE_WRITE))
> return -EBADF;
>
> /*
> - * We can only allow pure fallocate on append only files
> + * We can only allow pure space allocation on append only files.
> */
> - if ((mode & ~FALLOC_FL_KEEP_SIZE) && IS_APPEND(inode))
> + if (mode != FALLOC_FL_ALLOCATE_RANGE && IS_APPEND(inode))
> return -EPERM;
Unless I'm misreading, this changes semantics by enforcing that we
cannot use KEEP_SIZE on append only files. That means one can no longer
do a post-eof prealloc without actually changing the file size, which on
a quick test seems to work today.
That aside, this all looks like a nice cleanup to me.
Brian
>
> if (IS_IMMUTABLE(inode))
> diff --git a/include/linux/falloc.h b/include/linux/falloc.h
> index f3f0b97b167579..3f49f3df6af5fb 100644
> --- a/include/linux/falloc.h
> +++ b/include/linux/falloc.h
> @@ -25,12 +25,18 @@ struct space_resv {
> #define FS_IOC_UNRESVSP64 _IOW('X', 43, struct space_resv)
> #define FS_IOC_ZERO_RANGE _IOW('X', 57, struct space_resv)
>
> -#define FALLOC_FL_SUPPORTED_MASK (FALLOC_FL_KEEP_SIZE | \
> - FALLOC_FL_PUNCH_HOLE | \
> - FALLOC_FL_COLLAPSE_RANGE | \
> - FALLOC_FL_ZERO_RANGE | \
> - FALLOC_FL_INSERT_RANGE | \
> - FALLOC_FL_UNSHARE_RANGE)
> +/*
> + * Mask of all supported fallocate modes. Only one can be set at a time.
> + *
> + * In addition to the mode bit, the mode argument can also encode flags.
> + * FALLOC_FL_KEEP_SIZE is the only supported flag so far.
> + */
> +#define FALLOC_FL_MODE_MASK (FALLOC_FL_ALLOCATE_RANGE | \
> + FALLOC_FL_PUNCH_HOLE | \
> + FALLOC_FL_COLLAPSE_RANGE | \
> + FALLOC_FL_ZERO_RANGE | \
> + FALLOC_FL_INSERT_RANGE | \
> + FALLOC_FL_UNSHARE_RANGE)
>
> /* on ia32 l_start is on a 32-bit boundary */
> #if defined(CONFIG_X86_64)
> diff --git a/include/uapi/linux/falloc.h b/include/uapi/linux/falloc.h
> index 51398fa57f6cdf..5810371ed72bbd 100644
> --- a/include/uapi/linux/falloc.h
> +++ b/include/uapi/linux/falloc.h
> @@ -2,6 +2,7 @@
> #ifndef _UAPI_FALLOC_H_
> #define _UAPI_FALLOC_H_
>
> +#define FALLOC_FL_ALLOCATE_RANGE 0x00 /* allocate range */
> #define FALLOC_FL_KEEP_SIZE 0x01 /* default is extend size */
> #define FALLOC_FL_PUNCH_HOLE 0x02 /* de-allocates range */
> #define FALLOC_FL_NO_HIDE_STALE 0x04 /* reserved codepoint */
> --
> 2.43.0
>
>
