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@xxxxxxxxxx>