| Message ID | 6f2742-6f1f-cae9-7c5b-ed20fc53215@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
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@google.com> > --- > 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@suse.cz> 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
On Thu, 10 Aug 2023 23:27:07 -0700, 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). > > [...] I've dropped the previous version and applied this one. Thank! --- Applied to the vfs.tmpfs branch of the vfs/vfs.git tree. Patches in the vfs.tmpfs branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs.tmpfs [4/5] tmpfs: trivial support for direct IO https://git.kernel.org/vfs/vfs/c/6b55d273ec5b
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,
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@google.com> --- 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. mm/shmem.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-)