| Message ID | 20230519074047.1739879-14-dhowells@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On 5/19/23 15:40, David Howells wrote: > Provide a splice_read stub for Ceph. This does the inode shutdown check > before proceeding and jumps to direct_splice_read() if O_DIRECT is set, the > file has inline data or is a synchronous file. > > We try and get FILE_RD and either FILE_CACHE and/or FILE_LAZYIO caps and > hold them across filemap_splice_read(). If we fail to get FILE_CACHE or > FILE_LAZYIO capabilities, we use direct_splice_read() instead. > > Signed-off-by: David Howells <dhowells@redhat.com> > cc: Christoph Hellwig <hch@lst.de> > cc: Al Viro <viro@zeniv.linux.org.uk> > cc: Jens Axboe <axboe@kernel.dk> > cc: Xiubo Li <xiubli@redhat.com> > cc: Ilya Dryomov <idryomov@gmail.com> > cc: Jeff Layton <jlayton@kernel.org> > cc: ceph-devel@vger.kernel.org > cc: linux-fsdevel@vger.kernel.org > cc: linux-block@vger.kernel.org > cc: linux-mm@kvack.org > --- > fs/ceph/file.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 65 insertions(+), 1 deletion(-) > > diff --git a/fs/ceph/file.c b/fs/ceph/file.c > index f4d8bf7dec88..382dd5901748 100644 > --- a/fs/ceph/file.c > +++ b/fs/ceph/file.c > @@ -1745,6 +1745,70 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to) > return ret; > } > > +/* > + * Wrap filemap_splice_read with checks for cap bits on the inode. > + * Atomically grab references, so that those bits are not released > + * back to the MDS mid-read. > + */ > +static ssize_t ceph_splice_read(struct file *in, loff_t *ppos, > + struct pipe_inode_info *pipe, > + size_t len, unsigned int flags) > +{ > + struct ceph_file_info *fi = in->private_data; > + struct inode *inode = file_inode(in); > + struct ceph_inode_info *ci = ceph_inode(inode); > + ssize_t ret; > + int want = 0, got = 0; > + CEPH_DEFINE_RW_CONTEXT(rw_ctx, 0); > + > + dout("splice_read %p %llx.%llx %llu~%zu trying to get caps on %p\n", > + inode, ceph_vinop(inode), *ppos, len, inode); > + > + if (ceph_inode_is_shutdown(inode)) > + return -ESTALE; > + > + if ((in->f_flags & O_DIRECT) || > + ceph_has_inline_data(ci) || > + (fi->flags & CEPH_F_SYNC)) > + return direct_splice_read(in, ppos, pipe, len, flags); > + > + ceph_start_io_read(inode); > + > + want = CEPH_CAP_FILE_CACHE; > + if (fi->fmode & CEPH_FILE_MODE_LAZY) > + want |= CEPH_CAP_FILE_LAZYIO; > + > + ret = ceph_get_caps(in, CEPH_CAP_FILE_RD, want, -1, &got); > + if (ret < 0) { > + ceph_end_io_read(inode); > + return ret; > + } > + > + if ((got & (CEPH_CAP_FILE_CACHE | CEPH_CAP_FILE_LAZYIO)) == 0) { > + dout("splice_read/sync %p %llx.%llx %llu~%zu got cap refs on %s\n", > + inode, ceph_vinop(inode), *ppos, len, > + ceph_cap_string(got)); > + > + ceph_end_io_read(inode); > + return direct_splice_read(in, ppos, pipe, len, flags); Shouldn't we release cap ref before returning here ? Thanks - Xiubo > + } > + > + dout("splice_read %p %llx.%llx %llu~%zu got cap refs on %s\n", > + inode, ceph_vinop(inode), *ppos, len, ceph_cap_string(got)); > + > + rw_ctx.caps = got; > + ceph_add_rw_context(fi, &rw_ctx); > + ret = filemap_splice_read(in, ppos, pipe, len, flags); > + ceph_del_rw_context(fi, &rw_ctx); > + > + dout("splice_read %p %llx.%llx dropping cap refs on %s = %zd\n", > + inode, ceph_vinop(inode), ceph_cap_string(got), ret); > + ceph_put_cap_refs(ci, got); > + > + ceph_end_io_read(inode); > + return ret; > +} > + > /* > * Take cap references to avoid releasing caps to MDS mid-write. > * > @@ -2593,7 +2657,7 @@ const struct file_operations ceph_file_fops = { > .lock = ceph_lock, > .setlease = simple_nosetlease, > .flock = ceph_flock, > - .splice_read = generic_file_splice_read, > + .splice_read = ceph_splice_read, > .splice_write = iter_file_splice_write, > .unlocked_ioctl = ceph_ioctl, > .compat_ioctl = compat_ptr_ioctl, >
Xiubo Li <xiubli@redhat.com> wrote: > > + ret = ceph_get_caps(in, CEPH_CAP_FILE_RD, want, -1, &got); > > + if (ret < 0) { > > + ceph_end_io_read(inode); > > + return ret; > > + } > > + > > + if ((got & (CEPH_CAP_FILE_CACHE | CEPH_CAP_FILE_LAZYIO)) == 0) { > > + dout("splice_read/sync %p %llx.%llx %llu~%zu got cap refs on %s\n", > > + inode, ceph_vinop(inode), *ppos, len, > > + ceph_cap_string(got)); > > + > > + ceph_end_io_read(inode); > > + return direct_splice_read(in, ppos, pipe, len, flags); > > Shouldn't we release cap ref before returning here ? Ummm... Even if we got no caps? David
On 5/19/23 17:24, David Howells wrote: > Xiubo Li <xiubli@redhat.com> wrote: > >>> + ret = ceph_get_caps(in, CEPH_CAP_FILE_RD, want, -1, &got); >>> + if (ret < 0) { >>> + ceph_end_io_read(inode); >>> + return ret; >>> + } >>> + >>> + if ((got & (CEPH_CAP_FILE_CACHE | CEPH_CAP_FILE_LAZYIO)) == 0) { >>> + dout("splice_read/sync %p %llx.%llx %llu~%zu got cap refs on %s\n", >>> + inode, ceph_vinop(inode), *ppos, len, >>> + ceph_cap_string(got)); >>> + >>> + ceph_end_io_read(inode); >>> + return direct_splice_read(in, ppos, pipe, len, flags); >> Shouldn't we release cap ref before returning here ? > Ummm... Even if we got no caps? No, at least we have got the 'need' caps: CEPH_CAP_FILE_RD once here. I saw you have updated this and will check it. Thanks - Xiubo > > David >
diff --git a/fs/ceph/file.c b/fs/ceph/file.c index f4d8bf7dec88..382dd5901748 100644 --- a/fs/ceph/file.c +++ b/fs/ceph/file.c @@ -1745,6 +1745,70 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to) return ret; } +/* + * Wrap filemap_splice_read with checks for cap bits on the inode. + * Atomically grab references, so that those bits are not released + * back to the MDS mid-read. + */ +static ssize_t ceph_splice_read(struct file *in, loff_t *ppos, + struct pipe_inode_info *pipe, + size_t len, unsigned int flags) +{ + struct ceph_file_info *fi = in->private_data; + struct inode *inode = file_inode(in); + struct ceph_inode_info *ci = ceph_inode(inode); + ssize_t ret; + int want = 0, got = 0; + CEPH_DEFINE_RW_CONTEXT(rw_ctx, 0); + + dout("splice_read %p %llx.%llx %llu~%zu trying to get caps on %p\n", + inode, ceph_vinop(inode), *ppos, len, inode); + + if (ceph_inode_is_shutdown(inode)) + return -ESTALE; + + if ((in->f_flags & O_DIRECT) || + ceph_has_inline_data(ci) || + (fi->flags & CEPH_F_SYNC)) + return direct_splice_read(in, ppos, pipe, len, flags); + + ceph_start_io_read(inode); + + want = CEPH_CAP_FILE_CACHE; + if (fi->fmode & CEPH_FILE_MODE_LAZY) + want |= CEPH_CAP_FILE_LAZYIO; + + ret = ceph_get_caps(in, CEPH_CAP_FILE_RD, want, -1, &got); + if (ret < 0) { + ceph_end_io_read(inode); + return ret; + } + + if ((got & (CEPH_CAP_FILE_CACHE | CEPH_CAP_FILE_LAZYIO)) == 0) { + dout("splice_read/sync %p %llx.%llx %llu~%zu got cap refs on %s\n", + inode, ceph_vinop(inode), *ppos, len, + ceph_cap_string(got)); + + ceph_end_io_read(inode); + return direct_splice_read(in, ppos, pipe, len, flags); + } + + dout("splice_read %p %llx.%llx %llu~%zu got cap refs on %s\n", + inode, ceph_vinop(inode), *ppos, len, ceph_cap_string(got)); + + rw_ctx.caps = got; + ceph_add_rw_context(fi, &rw_ctx); + ret = filemap_splice_read(in, ppos, pipe, len, flags); + ceph_del_rw_context(fi, &rw_ctx); + + dout("splice_read %p %llx.%llx dropping cap refs on %s = %zd\n", + inode, ceph_vinop(inode), ceph_cap_string(got), ret); + ceph_put_cap_refs(ci, got); + + ceph_end_io_read(inode); + return ret; +} + /* * Take cap references to avoid releasing caps to MDS mid-write. * @@ -2593,7 +2657,7 @@ const struct file_operations ceph_file_fops = { .lock = ceph_lock, .setlease = simple_nosetlease, .flock = ceph_flock, - .splice_read = generic_file_splice_read, + .splice_read = ceph_splice_read, .splice_write = iter_file_splice_write, .unlocked_ioctl = ceph_ioctl, .compat_ioctl = compat_ptr_ioctl,
Provide a splice_read stub for Ceph. This does the inode shutdown check before proceeding and jumps to direct_splice_read() if O_DIRECT is set, the file has inline data or is a synchronous file. We try and get FILE_RD and either FILE_CACHE and/or FILE_LAZYIO caps and hold them across filemap_splice_read(). If we fail to get FILE_CACHE or FILE_LAZYIO capabilities, we use direct_splice_read() instead. Signed-off-by: David Howells <dhowells@redhat.com> cc: Christoph Hellwig <hch@lst.de> cc: Al Viro <viro@zeniv.linux.org.uk> cc: Jens Axboe <axboe@kernel.dk> cc: Xiubo Li <xiubli@redhat.com> cc: Ilya Dryomov <idryomov@gmail.com> cc: Jeff Layton <jlayton@kernel.org> cc: ceph-devel@vger.kernel.org cc: linux-fsdevel@vger.kernel.org cc: linux-block@vger.kernel.org cc: linux-mm@kvack.org --- fs/ceph/file.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-)