| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1e96f2c36f7c49a7d2d57dc1eb4135bd27a06122.camel@kernel.org>
Date: Fri, 11 Mar 2022 08:54:59 -0500
From: Jeff Layton <jlayton@...nel.org>
To: David Howells <dhowells@...hat.com>
Cc: linux-cachefs@...hat.com, ceph-devel@...r.kernel.org,
Anna Schumaker <anna.schumaker@...app.com>,
Steve French <sfrench@...ba.org>,
Dominique Martinet <asmadeus@...ewreck.org>,
David Wysochanski <dwysocha@...hat.com>,
Ilya Dryomov <idryomov@...il.com>,
Jeffle Xu <jefflexu@...ux.alibaba.com>,
Linus Torvalds <torvalds@...ux-foundation.org>,
linux-afs@...ts.infradead.org, linux-nfs@...r.kernel.org,
linux-cifs@...r.kernel.org, v9fs-developer@...ts.sourceforge.net,
linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 12/20] ceph: Make ceph_init_request() check caps on
readahead
On Fri, 2022-03-11 at 13:49 +0000, David Howells wrote:
> Jeff Layton <jlayton@...hat.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@...hat.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@...hat.com>
> cc: ceph-devel@...r.kernel.org
> cc: linux-cachefs@...hat.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@...nel.org>
Powered by blists - more mailing lists