| Message ID | 164692907694.2099075.10081819855690054094.stgit@warthog.procyon.org.uk |
|---|---|
| State | New |
| Headers | show |
| Series | netfs: Prep for write helpers | expand |
On Thu, 2022-03-10 at 16:17 +0000, David Howells wrote: > Move the caps check from ceph_readahead() to ceph_init_request(), > conditional on the origin being NETFS_READAHEAD so that in a future patch, > ceph can point its ->readahead() vector directly at netfs_readahead(). > > Changes > ======= > ver #3) > - Split from the patch to add a netfs inode context[1]. > - Need to store the caps got in rreq->netfs_priv for later freeing. > > Signed-off-by: David Howells <dhowells@redhat.com> > cc: ceph-devel@vger.kernel.org > cc: linux-cachefs@redhat.com > Link: https://lore.kernel.org/r/8af0d47f17d89c06bbf602496dd845f2b0bf25b3.camel@kernel.org/ [1] > --- > > fs/ceph/addr.c | 69 +++++++++++++++++++++++++++++++++----------------------- > 1 file changed, 41 insertions(+), 28 deletions(-) > > diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c > index 9189257476f8..6d056db41f50 100644 > --- a/fs/ceph/addr.c > +++ b/fs/ceph/addr.c > @@ -354,6 +354,45 @@ static void ceph_netfs_issue_read(struct netfs_io_subrequest *subreq) > dout("%s: result %d\n", __func__, err); > } > > +static int ceph_init_request(struct netfs_io_request *rreq, struct file *file) > +{ > + struct inode *inode = rreq->inode; > + int got = 0, want = CEPH_CAP_FILE_CACHE; > + int ret = 0; > + > + if (file) { > + struct ceph_rw_context *rw_ctx; > + struct ceph_file_info *fi = file->private_data; > + > + rw_ctx = ceph_find_rw_context(fi); > + if (rw_ctx) > + return 0; > + } > + > + if (rreq->origin != NETFS_READAHEAD) > + return 0; > + ^^^ I think you should move this check above the if (file) block above it. We don't need to anything at all if we're not in readahead. > + /* > + * readahead callers do not necessarily hold Fcb caps > + * (e.g. fadvise, madvise). > + */ > + ret = ceph_try_get_caps(inode, CEPH_CAP_FILE_RD, want, true, &got); > + if (ret < 0) { > + dout("start_read %p, error getting cap\n", inode); > + return ret; > + } > + > + if (!(got & want)) { > + dout("start_read %p, no cache cap\n", inode); > + return -EACCES; > + } > + if (ret == 0) > + return -EACCES; > + > + rreq->netfs_priv = (void *)(uintptr_t)got; > + return 0; > +} > + > static void ceph_readahead_cleanup(struct address_space *mapping, void *priv) > { > struct inode *inode = mapping->host; > @@ -365,7 +404,7 @@ static void ceph_readahead_cleanup(struct address_space *mapping, void *priv) > } > > static const struct netfs_request_ops ceph_netfs_read_ops = { > - .is_cache_enabled = ceph_is_cache_enabled, > + .init_request = ceph_init_request, > .begin_cache_operation = ceph_begin_cache_operation, > .issue_read = ceph_netfs_issue_read, > .expand_readahead = ceph_netfs_expand_readahead, > @@ -393,33 +432,7 @@ static int ceph_readpage(struct file *file, struct page *subpage) > > static void ceph_readahead(struct readahead_control *ractl) > { > - struct inode *inode = file_inode(ractl->file); > - struct ceph_file_info *fi = ractl->file->private_data; > - struct ceph_rw_context *rw_ctx; > - int got = 0; > - int ret = 0; > - > - if (ceph_inode(inode)->i_inline_version != CEPH_INLINE_NONE) > - return; > - > - rw_ctx = ceph_find_rw_context(fi); > - if (!rw_ctx) { > - /* > - * readahead callers do not necessarily hold Fcb caps > - * (e.g. fadvise, madvise). > - */ > - int want = CEPH_CAP_FILE_CACHE; > - > - ret = ceph_try_get_caps(inode, CEPH_CAP_FILE_RD, want, true, &got); > - if (ret < 0) > - dout("start_read %p, error getting cap\n", inode); > - else if (!(got & want)) > - dout("start_read %p, no cache cap\n", inode); > - > - if (ret <= 0) > - return; > - } > - netfs_readahead(ractl, &ceph_netfs_read_ops, (void *)(uintptr_t)got); > + netfs_readahead(ractl, &ceph_netfs_read_ops, NULL); > } > > #ifdef CONFIG_CEPH_FSCACHE > >
Jeff Layton <jlayton@redhat.com> wrote: > > +static int ceph_init_request(struct netfs_io_request *rreq, struct file *file) > > +{ > > + struct inode *inode = rreq->inode; > > + int got = 0, want = CEPH_CAP_FILE_CACHE; > > + int ret = 0; > > + > > + if (file) { > > + struct ceph_rw_context *rw_ctx; > > + struct ceph_file_info *fi = file->private_data; > > + > > + rw_ctx = ceph_find_rw_context(fi); > > + if (rw_ctx) > > + return 0; > > + } > > + > > + if (rreq->origin != NETFS_READAHEAD) > > + return 0; > > + > > ^^^ > I think you should move this check above the if (file) block above it. > We don't need to anything at all if we're not in readahead. How about the attached, then? David --- commit 7082946186fc26016b15bc9039bd6d92ae732ef3 Author: David Howells <dhowells@redhat.com> Date: Wed Mar 9 21:45:22 2022 +0000 ceph: Make ceph_init_request() check caps on readahead Move the caps check from ceph_readahead() to ceph_init_request(), conditional on the origin being NETFS_READAHEAD so that in a future patch, ceph can point its ->readahead() vector directly at netfs_readahead(). Changes ======= ver #4) - Move the check for NETFS_READAHEAD up in ceph_init_request()[2]. ver #3) - Split from the patch to add a netfs inode context[1]. - Need to store the caps got in rreq->netfs_priv for later freeing. Signed-off-by: David Howells <dhowells@redhat.com> cc: ceph-devel@vger.kernel.org cc: linux-cachefs@redhat.com Link: https://lore.kernel.org/r/8af0d47f17d89c06bbf602496dd845f2b0bf25b3.camel@kernel.org/ [1] Link: https://lore.kernel.org/r/dd054c962818716e718bd9b446ee5322ca097675.camel@redhat.com/ [2] diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c index 9189257476f8..4aeccafa5dda 100644 --- a/fs/ceph/addr.c +++ b/fs/ceph/addr.c @@ -354,6 +354,45 @@ static void ceph_netfs_issue_read(struct netfs_io_subrequest *subreq) dout("%s: result %d\n", __func__, err); } +static int ceph_init_request(struct netfs_io_request *rreq, struct file *file) +{ + struct inode *inode = rreq->inode; + int got = 0, want = CEPH_CAP_FILE_CACHE; + int ret = 0; + + if (rreq->origin != NETFS_READAHEAD) + return 0; + + if (file) { + struct ceph_rw_context *rw_ctx; + struct ceph_file_info *fi = file->private_data; + + rw_ctx = ceph_find_rw_context(fi); + if (rw_ctx) + return 0; + } + + /* + * readahead callers do not necessarily hold Fcb caps + * (e.g. fadvise, madvise). + */ + ret = ceph_try_get_caps(inode, CEPH_CAP_FILE_RD, want, true, &got); + if (ret < 0) { + dout("start_read %p, error getting cap\n", inode); + return ret; + } + + if (!(got & want)) { + dout("start_read %p, no cache cap\n", inode); + return -EACCES; + } + if (ret == 0) + return -EACCES; + + rreq->netfs_priv = (void *)(uintptr_t)got; + return 0; +} + static void ceph_readahead_cleanup(struct address_space *mapping, void *priv) { struct inode *inode = mapping->host; @@ -365,7 +404,7 @@ static void ceph_readahead_cleanup(struct address_space *mapping, void *priv) } static const struct netfs_request_ops ceph_netfs_read_ops = { - .is_cache_enabled = ceph_is_cache_enabled, + .init_request = ceph_init_request, .begin_cache_operation = ceph_begin_cache_operation, .issue_read = ceph_netfs_issue_read, .expand_readahead = ceph_netfs_expand_readahead, @@ -393,33 +432,7 @@ static int ceph_readpage(struct file *file, struct page *subpage) static void ceph_readahead(struct readahead_control *ractl) { - struct inode *inode = file_inode(ractl->file); - struct ceph_file_info *fi = ractl->file->private_data; - struct ceph_rw_context *rw_ctx; - int got = 0; - int ret = 0; - - if (ceph_inode(inode)->i_inline_version != CEPH_INLINE_NONE) - return; - - rw_ctx = ceph_find_rw_context(fi); - if (!rw_ctx) { - /* - * readahead callers do not necessarily hold Fcb caps - * (e.g. fadvise, madvise). - */ - int want = CEPH_CAP_FILE_CACHE; - - ret = ceph_try_get_caps(inode, CEPH_CAP_FILE_RD, want, true, &got); - if (ret < 0) - dout("start_read %p, error getting cap\n", inode); - else if (!(got & want)) - dout("start_read %p, no cache cap\n", inode); - - if (ret <= 0) - return; - } - netfs_readahead(ractl, &ceph_netfs_read_ops, (void *)(uintptr_t)got); + netfs_readahead(ractl, &ceph_netfs_read_ops, NULL); } #ifdef CONFIG_CEPH_FSCACHE
On Fri, 2022-03-11 at 13:49 +0000, David Howells wrote: > Jeff Layton <jlayton@redhat.com> wrote: > > > > +static int ceph_init_request(struct netfs_io_request *rreq, struct file *file) > > > +{ > > > + struct inode *inode = rreq->inode; > > > + int got = 0, want = CEPH_CAP_FILE_CACHE; > > > + int ret = 0; > > > + > > > + if (file) { > > > + struct ceph_rw_context *rw_ctx; > > > + struct ceph_file_info *fi = file->private_data; > > > + > > > + rw_ctx = ceph_find_rw_context(fi); > > > + if (rw_ctx) > > > + return 0; > > > + } > > > + > > > + if (rreq->origin != NETFS_READAHEAD) > > > + return 0; > > > + > > > > ^^^ > > I think you should move this check above the if (file) block above it. > > We don't need to anything at all if we're not in readahead. > > How about the attached, then? > > David > --- > commit 7082946186fc26016b15bc9039bd6d92ae732ef3 > Author: David Howells <dhowells@redhat.com> > Date: Wed Mar 9 21:45:22 2022 +0000 > > ceph: Make ceph_init_request() check caps on readahead > > Move the caps check from ceph_readahead() to ceph_init_request(), > conditional on the origin being NETFS_READAHEAD so that in a future patch, > ceph can point its ->readahead() vector directly at netfs_readahead(). > > Changes > ======= > ver #4) > - Move the check for NETFS_READAHEAD up in ceph_init_request()[2]. > > ver #3) > - Split from the patch to add a netfs inode context[1]. > - Need to store the caps got in rreq->netfs_priv for later freeing. > > Signed-off-by: David Howells <dhowells@redhat.com> > cc: ceph-devel@vger.kernel.org > cc: linux-cachefs@redhat.com > Link: https://lore.kernel.org/r/8af0d47f17d89c06bbf602496dd845f2b0bf25b3.camel@kernel.org/ [1] > Link: https://lore.kernel.org/r/dd054c962818716e718bd9b446ee5322ca097675.camel@redhat.com/ [2] > > diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c > index 9189257476f8..4aeccafa5dda 100644 > --- a/fs/ceph/addr.c > +++ b/fs/ceph/addr.c > @@ -354,6 +354,45 @@ static void ceph_netfs_issue_read(struct netfs_io_subrequest *subreq) > dout("%s: result %d\n", __func__, err); > } > > +static int ceph_init_request(struct netfs_io_request *rreq, struct file *file) > +{ > + struct inode *inode = rreq->inode; > + int got = 0, want = CEPH_CAP_FILE_CACHE; > + int ret = 0; > + > + if (rreq->origin != NETFS_READAHEAD) > + return 0; > + > + if (file) { > + struct ceph_rw_context *rw_ctx; > + struct ceph_file_info *fi = file->private_data; > + > + rw_ctx = ceph_find_rw_context(fi); > + if (rw_ctx) > + return 0; > + } > + > + /* > + * readahead callers do not necessarily hold Fcb caps > + * (e.g. fadvise, madvise). > + */ > + ret = ceph_try_get_caps(inode, CEPH_CAP_FILE_RD, want, true, &got); > + if (ret < 0) { > + dout("start_read %p, error getting cap\n", inode); > + return ret; > + } > + > + if (!(got & want)) { > + dout("start_read %p, no cache cap\n", inode); > + return -EACCES; > + } > + if (ret == 0) > + return -EACCES; > + > + rreq->netfs_priv = (void *)(uintptr_t)got; > + return 0; > +} > + > static void ceph_readahead_cleanup(struct address_space *mapping, void *priv) > { > struct inode *inode = mapping->host; > @@ -365,7 +404,7 @@ static void ceph_readahead_cleanup(struct address_space *mapping, void *priv) > } > > static const struct netfs_request_ops ceph_netfs_read_ops = { > - .is_cache_enabled = ceph_is_cache_enabled, > + .init_request = ceph_init_request, > .begin_cache_operation = ceph_begin_cache_operation, > .issue_read = ceph_netfs_issue_read, > .expand_readahead = ceph_netfs_expand_readahead, > @@ -393,33 +432,7 @@ static int ceph_readpage(struct file *file, struct page *subpage) > > static void ceph_readahead(struct readahead_control *ractl) > { > - struct inode *inode = file_inode(ractl->file); > - struct ceph_file_info *fi = ractl->file->private_data; > - struct ceph_rw_context *rw_ctx; > - int got = 0; > - int ret = 0; > - > - if (ceph_inode(inode)->i_inline_version != CEPH_INLINE_NONE) > - return; > - > - rw_ctx = ceph_find_rw_context(fi); > - if (!rw_ctx) { > - /* > - * readahead callers do not necessarily hold Fcb caps > - * (e.g. fadvise, madvise). > - */ > - int want = CEPH_CAP_FILE_CACHE; > - > - ret = ceph_try_get_caps(inode, CEPH_CAP_FILE_RD, want, true, &got); > - if (ret < 0) > - dout("start_read %p, error getting cap\n", inode); > - else if (!(got & want)) > - dout("start_read %p, no cache cap\n", inode); > - > - if (ret <= 0) > - return; > - } > - netfs_readahead(ractl, &ceph_netfs_read_ops, (void *)(uintptr_t)got); > + netfs_readahead(ractl, &ceph_netfs_read_ops, NULL); > } > > #ifdef CONFIG_CEPH_FSCACHE > LGTM, Reviewed-by: Jeff Layton <jlayton@kernel.org>
diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c index 9189257476f8..6d056db41f50 100644 --- a/fs/ceph/addr.c +++ b/fs/ceph/addr.c @@ -354,6 +354,45 @@ static void ceph_netfs_issue_read(struct netfs_io_subrequest *subreq) dout("%s: result %d\n", __func__, err); } +static int ceph_init_request(struct netfs_io_request *rreq, struct file *file) +{ + struct inode *inode = rreq->inode; + int got = 0, want = CEPH_CAP_FILE_CACHE; + int ret = 0; + + if (file) { + struct ceph_rw_context *rw_ctx; + struct ceph_file_info *fi = file->private_data; + + rw_ctx = ceph_find_rw_context(fi); + if (rw_ctx) + return 0; + } + + if (rreq->origin != NETFS_READAHEAD) + return 0; + + /* + * readahead callers do not necessarily hold Fcb caps + * (e.g. fadvise, madvise). + */ + ret = ceph_try_get_caps(inode, CEPH_CAP_FILE_RD, want, true, &got); + if (ret < 0) { + dout("start_read %p, error getting cap\n", inode); + return ret; + } + + if (!(got & want)) { + dout("start_read %p, no cache cap\n", inode); + return -EACCES; + } + if (ret == 0) + return -EACCES; + + rreq->netfs_priv = (void *)(uintptr_t)got; + return 0; +} + static void ceph_readahead_cleanup(struct address_space *mapping, void *priv) { struct inode *inode = mapping->host; @@ -365,7 +404,7 @@ static void ceph_readahead_cleanup(struct address_space *mapping, void *priv) } static const struct netfs_request_ops ceph_netfs_read_ops = { - .is_cache_enabled = ceph_is_cache_enabled, + .init_request = ceph_init_request, .begin_cache_operation = ceph_begin_cache_operation, .issue_read = ceph_netfs_issue_read, .expand_readahead = ceph_netfs_expand_readahead, @@ -393,33 +432,7 @@ static int ceph_readpage(struct file *file, struct page *subpage) static void ceph_readahead(struct readahead_control *ractl) { - struct inode *inode = file_inode(ractl->file); - struct ceph_file_info *fi = ractl->file->private_data; - struct ceph_rw_context *rw_ctx; - int got = 0; - int ret = 0; - - if (ceph_inode(inode)->i_inline_version != CEPH_INLINE_NONE) - return; - - rw_ctx = ceph_find_rw_context(fi); - if (!rw_ctx) { - /* - * readahead callers do not necessarily hold Fcb caps - * (e.g. fadvise, madvise). - */ - int want = CEPH_CAP_FILE_CACHE; - - ret = ceph_try_get_caps(inode, CEPH_CAP_FILE_RD, want, true, &got); - if (ret < 0) - dout("start_read %p, error getting cap\n", inode); - else if (!(got & want)) - dout("start_read %p, no cache cap\n", inode); - - if (ret <= 0) - return; - } - netfs_readahead(ractl, &ceph_netfs_read_ops, (void *)(uintptr_t)got); + netfs_readahead(ractl, &ceph_netfs_read_ops, NULL); } #ifdef CONFIG_CEPH_FSCACHE
Move the caps check from ceph_readahead() to ceph_init_request(), conditional on the origin being NETFS_READAHEAD so that in a future patch, ceph can point its ->readahead() vector directly at netfs_readahead(). Changes ======= ver #3) - Split from the patch to add a netfs inode context[1]. - Need to store the caps got in rreq->netfs_priv for later freeing. Signed-off-by: David Howells <dhowells@redhat.com> cc: ceph-devel@vger.kernel.org cc: linux-cachefs@redhat.com Link: https://lore.kernel.org/r/8af0d47f17d89c06bbf602496dd845f2b0bf25b3.camel@kernel.org/ [1] --- fs/ceph/addr.c | 69 +++++++++++++++++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 28 deletions(-)