Re: [PATCH] 9p: Convert to new fscache API

[David Howells <dhowells@redhat.com>]

Dominique Martinet <asmadeus@codewreck.org> wrote:

> What's the current schedule/plan for the fscache branch merging? Will
> you be trying this merge window next month?

That's the aim.  We have afs, ceph and nfs are about ready; I've had a go at
doing the 9p conversion, which I'll have to leave to you now, I think, and am
having a poke at cifs.

> >  (*) I have made an assumption that 9p_client_read() and write can handle I/Os
> >      larger than a page.  If this is not the case, v9fs_req_ops will need
> >      clamp_length() implementing.
> 
> There's a max driven by the client's msize

The netfs read helpers provide you with a couple of options here:

 (1) You can use ->clamp_length() to do multiple slices of at least 1 byte
     each.  The assumption being that these represent parallel operations.  A
     new subreq will be generated for each slice.

 (2) You can go with large slices that are larger than msize, and just read
     part of it with each read.  After reading, the netfs helper will keep
     calling you again to read some more of it, provided you didn't return an
     error and you at least read something.

> (client->msize - P9_IOHDRSZ ; unfortunately msize is just guaranted to be >=
> 4k so that means the actual IO size would be smaller in that case even if
> that's not intended to be common)

Does that mean you might be limited to reads of less than PAGE_SIZE on some
systems (ppc64 for example)?  This isn't a problem for the read helper, but
might be an issue for writing from THPs.

> >  (*) The cache needs to be invalidated if a 3rd-party change happens, but I
> >      haven't done that.
> 
> There's no concurrent access logic in 9p as far as I'm aware (like NFS
> does if the mtime changes for example), so I assume we can keep ignoring
> this.

By that, I presume you mean concurrent accesses are just not permitted?

> >  (*) If 9p supports DIO writes, it should invalidate a cache object with
> >      FSCACHE_INVAL_DIO_WRITE when one happens - thereby stopping caching for
> >      that file until all file handles on it are closed.
> 
> Not 100% sure actually there is some code about it but comment says it's
> disabled when cache is active; I'll check just found another problem
> with some queued patch that need fixing first...

Ok.

> > I forgot something: netfs_subreq_terminated() needs to be called when
> > the read is complete.  If p9_client_read() is synchronous, then that
> > would be here,
> 
> (it is synchronous; I'll add that suggestion)

Thanks.

David