Re: [PATCH v2 6/6] ceph: convert ceph_readpages to ceph_readahead

From: Jeff Layton <jlayton@kernel.org>
To: Matthew Wilcox <willy@infradead.org>
Cc: dhowells@redhat.com, idryomov@gmail.com, xiubli@redhat.com,
	ceph-devel@vger.kernel.org, linux-cachefs@redhat.com,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v2 6/6] ceph: convert ceph_readpages to ceph_readahead
Date: Wed, 17 Feb 2021 10:46:35 -0500	[thread overview]
Message-ID: <c15e08fdf282b7775e0c3cc7f9139659836a065f.camel@kernel.org> (raw)
In-Reply-To: <20210217151548.GL2858050@casper.infradead.org>

On Wed, 2021-02-17 at 15:15 +0000, Matthew Wilcox wrote:
> On Wed, Feb 17, 2021 at 07:58:45AM -0500, Jeff Layton wrote:
> > +static void ceph_readahead_cleanup(struct address_space *mapping, void *priv)
> >  {
> > +	struct inode *inode = mapping->host;
> >  	struct ceph_inode_info *ci = ceph_inode(inode);
> > +	int got = (int)(uintptr_t)priv;
> >  
> > 
> > 
> > 
> >  	if (got)
> >  		ceph_put_cap_refs(ci, got);
> >  }
> > +const struct netfs_read_request_ops ceph_readahead_netfs_ops = {
> > +	.init_rreq		= ceph_init_rreq,
> > +	.is_cache_enabled	= ceph_is_cache_enabled,
> > +	.begin_cache_operation	= ceph_begin_cache_operation,
> > +	.issue_op		= ceph_netfs_issue_op,
> > +	.expand_readahead	= ceph_netfs_expand_readahead,
> > +	.clamp_length		= ceph_netfs_clamp_length,
> > +	.cleanup		= ceph_readahead_cleanup,
> > +};
> 
> It looks to me like this netfs_read_request_ops is the same as the
> ceph_readpage_netfs_ops except for the addition of ceph_readahead_cleanup.
> If so, since readpage passes NULL as 'priv', the two read_request_ops
> can be the same ... right?
> 

Yeah. I can also do the same for the write_begin one. The only
difference there is check_write_begin op, and it's only called in the
write_begin helper.

> also, you don't need that '(int)' cast -- can be just:
> 
> 	int got = (uintptr_t)priv;

Got it, fixed. I'll do some testing with this and re-post in a few days
if it all looks good.

Thanks!
-- 
Jeff Layton <jlayton@kernel.org>