On Thu 10-08-23 23:27:07, Hugh Dickins wrote:
> Depending upon your philosophical viewpoint, either tmpfs always does
> direct IO, or it cannot ever do direct IO; but whichever, if tmpfs is to
> stand in for a more sophisticated filesystem, it can be helpful for tmpfs
> to support O_DIRECT. So, give tmpfs a shmem_file_open() method, to set
> the FMODE_CAN_ODIRECT flag: then unchanged shmem_file_read_iter() and new
> shmem_file_write_iter() do the work (without any shmem_direct_IO() stub).
>
> Perhaps later, once the direct_IO method has been eliminated from all
> filesystems, generic_file_write_iter() will be such that tmpfs can again
> use it, even for O_DIRECT.
>
> xfstests auto generic which were not run on tmpfs before but now pass:
> 036 091 113 125 130 133 135 198 207 208 209 210 211 212 214 226 239 263
> 323 355 391 406 412 422 427 446 451 465 551 586 591 609 615 647 708 729
> with no new failures.
>
> LTP dio tests which were not run on tmpfs before but now pass:
> dio01 through dio30, except for dio04 and dio10, which fail because
> tmpfs dio read and write allow odd count: tmpfs could be made stricter,
> but would that be an improvement?
>
> Signed-off-by: Hugh Dickins <hughd@xxxxxxxxxx>
> ---
> Thanks for your earlier review, Jan: I've certainly not copied that
> into this entirely different version. I prefer the v1, but fine if
> people prefer this v2.
Yeah, this solution is also fine with me so feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
I agree the previous version has less code duplication but once .direct_IO
is gone shmem_file_write_iter() will be actually how some generic helper
will look like so we can deduplicate the code then.
Honza
> mm/shmem.c | 32 ++++++++++++++++++++++++++++++--
> 1 file changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index ca43fb256b8e..b782edeb69aa 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2388,6 +2388,12 @@ static int shmem_mmap(struct file *file, struct vm_area_struct *vma)
> return 0;
> }
>
> +static int shmem_file_open(struct inode *inode, struct file *file)
> +{
> + file->f_mode |= FMODE_CAN_ODIRECT;
> + return generic_file_open(inode, file);
> +}
> +
> #ifdef CONFIG_TMPFS_XATTR
> static int shmem_initxattrs(struct inode *, const struct xattr *, void *);
>
> @@ -2839,6 +2845,28 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> return retval ? retval : error;
> }
>
> +static ssize_t shmem_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> +{
> + struct file *file = iocb->ki_filp;
> + struct inode *inode = file->f_mapping->host;
> + ssize_t ret;
> +
> + inode_lock(inode);
> + ret = generic_write_checks(iocb, from);
> + if (ret <= 0)
> + goto unlock;
> + ret = file_remove_privs(file);
> + if (ret)
> + goto unlock;
> + ret = file_update_time(file);
> + if (ret)
> + goto unlock;
> + ret = generic_perform_write(iocb, from);
> +unlock:
> + inode_unlock(inode);
> + return ret;
> +}
> +
> static bool zero_pipe_buf_get(struct pipe_inode_info *pipe,
> struct pipe_buffer *buf)
> {
> @@ -4434,12 +4462,12 @@ EXPORT_SYMBOL(shmem_aops);
>
> static const struct file_operations shmem_file_operations = {
> .mmap = shmem_mmap,
> - .open = generic_file_open,
> + .open = shmem_file_open,
> .get_unmapped_area = shmem_get_unmapped_area,
> #ifdef CONFIG_TMPFS
> .llseek = shmem_file_llseek,
> .read_iter = shmem_file_read_iter,
> - .write_iter = generic_file_write_iter,
> + .write_iter = shmem_file_write_iter,
> .fsync = noop_fsync,
> .splice_read = shmem_file_splice_read,
> .splice_write = iter_file_splice_write,
> --
> 2.35.3
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
