[0/2] ceph: adapt ceph to the fscache rewrite

This is a follow-on set for David Howells' recent patchset to rewrite
the fscache and cachefiles infrastructure. This re-enables fscache read
support in the ceph driver, and also adds support for writing to the
cache as well.

What's the best way to handle these, going forward? David, would it be
easier for you to carry these in your tree along with the rest of your
series?

Jeff Layton (2):
  ceph: conversion to new fscache API
  ceph: add fscache writeback support

 fs/ceph/Kconfig |   2 +-
 fs/ceph/addr.c  |  99 +++++++++++++++++++------
 fs/ceph/cache.c | 188 ++++++++++++++++++++----------------------------
 fs/ceph/cache.h |  98 +++++++++++++++++--------
 fs/ceph/caps.c  |   3 +-
 fs/ceph/file.c  |  13 +++-
 fs/ceph/inode.c |  22 ++++--
 fs/ceph/super.c |  10 +--
 fs/ceph/super.h |   2 +-
 9 files changed, 255 insertions(+), 182 deletions(-)

Jeff Layton <jlayton@kernel.org> wrote:

> +void ceph_fscache_unregister_inode_cookie(struct ceph_inode_info* ci)
>  {
> -	return fscache_register_netfs(&ceph_cache_netfs);
> +	struct fscache_cookie* cookie = xchg(&ci->fscache, NULL);
> +
> +	fscache_relinquish_cookie(cookie, false);
>  }

xchg() should be excessive there.  This is only called from
ceph_evict_inode().  Also, if you're going to reset the pointer, it might be
worth poisoning it rather than nulling it.

David

On Mon, 2021-11-29 at 16:46 +0000, David Howells wrote:
> Jeff Layton <jlayton@kernel.org> wrote:
> 
> > +void ceph_fscache_unregister_inode_cookie(struct ceph_inode_info* ci)
> >  {
> > -	return fscache_register_netfs(&ceph_cache_netfs);
> > +	struct fscache_cookie* cookie = xchg(&ci->fscache, NULL);
> > +
> > +	fscache_relinquish_cookie(cookie, false);
> >  }
> 
> xchg() should be excessive there.  This is only called from
> ceph_evict_inode().  Also, if you're going to reset the pointer, it might be
> worth poisoning it rather than nulling it.
> 

Ok, makes sense. I'll make that change soon.

On Mon, 2021-12-06 at 09:57 +0000, David Howells wrote:
> Jeff Layton <jlayton@kernel.org> wrote:
> 
> >  		if (!(gfp & __GFP_DIRECT_RECLAIM) || !(gfp & __GFP_FS))
> 
> There's a function for the first part of this:
> 
> 		if (!gfpflags_allow_blocking(gfp) || !(gfp & __GFP_FS))
> 
> > +	fsc->fscache = fscache_acquire_volume(name, NULL, 0);
> >  
> >  	if (fsc->fscache) {
> >  		ent->fscache = fsc->fscache;
> >  		list_add_tail(&ent->list, &ceph_fscache_list);
> 
> It shouldn't really be necessary to have ceph_fscache_list since
> fscache_acquire_volume() will do it's own duplicate check.  I wonder if I
> should make fscache_acquire_volume() return -EEXIST or -EBUSY rather than NULL
> in such a case and not print an error, but rather leave that to the filesystem
> to display.
> 
> That would allow you to get rid of the ceph_fscache_entry struct also, I
> think.
> 

Returning an error there sounds like a better thing to do.

I'll make the other changes you suggested now. Let me know if you change
the fscache_acquire_volume return.

> > +#define FSCACHE_USE_NEW_IO_API
> 
> That doesn't exist anymore.
> 
> > +		/*
> > +		 * If we're truncating up, then we should be able to just update
> > +		 * the existing cookie.
> > +		 */
> > +		if (size > isize)
> > +			ceph_fscache_update(inode);
> 
> Might look better to say "expanding" rather than "truncating up".
> 
> David
>