2021-09-21 11:17:12

by Dominique Martinet

[permalink] [raw]
Subject: Re: [PATCH 4/8] 9p: (untested) Convert to using the netfs helper lib to do reads and caching

David Howells wrote on Tue, Sep 14, 2021 at 02:55:26PM +0100:
> 9p: (untested) Convert to using the netfs helper lib to do reads and caching

Finally tested to some extent: let's remove that (untested) tag.


> Convert the 9p filesystem to use the netfs helper lib to handle readpage,
> readahead and write_begin, converting those into a common issue_op for the
> filesystem itself to handle. The netfs helper lib also handles reading
> from fscache if a cache is available, and interleaving reads from both
> sources.
>
> This change also switches from the old fscache I/O API to the new one,
> meaning that fscache no longer keeps track of netfs pages and instead does
> async DIO between the backing files and the 9p file pagecache. As a part
> of this change, the handling of PG_fscache changes. It now just means that
> the cache has a write I/O operation in progress on a page (PG_locked
> is used for a read I/O op).
>
> Note that this is a cut-down version of the fscache rewrite and does not
> change any of the cookie and cache coherency handling.
>
> Signed-off-by: David Howells <[email protected]>
> cc: Dominique Martinet <[email protected]>

can add either my sob or a reviewed-by tag from me instead.
I'm honestly not familiar enough with some of the changes (parts
checking PAGE_SIZE or similar) but I didn't spot any obvious error
except the few ifdefs I commented on below, and will keep running a few
more tests until next merge window.

> cc: [email protected]
> cc: [email protected]
> ---
>
> fs/9p/Kconfig | 1
> fs/9p/cache.c | 137 -------------------------------------------
> fs/9p/cache.h | 99 +------------------------------
> fs/9p/v9fs.h | 9 +++
> fs/9p/vfs_addr.c | 174 ++++++++++++++++++++++++------------------------------
> fs/9p/vfs_file.c | 21 +++++--
> 6 files changed, 108 insertions(+), 333 deletions(-)
>
> diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c
> index cce9ace651a2..a7e080916826 100644
> --- a/fs/9p/vfs_addr.c
> +++ b/fs/9p/vfs_addr.c
> @@ -124,7 +117,14 @@ static int v9fs_release_page(struct page *page, gfp_t gfp)
> {
> if (PagePrivate(page))
> return 0;
> - return v9fs_fscache_release_page(page, gfp);
> +#ifdef CONFIG_AFS_FSCACHE

s/AFS/9P/

> + if (PageFsCache(page)) {
> + if (!(gfp & __GFP_DIRECT_RECLAIM) || !(gfp & __GFP_FS))
> + return 0;
> + wait_on_page_fscache(page);
> + }
> +#endif
> + return 1;
> }
>
> /**
> diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
> index aab5e6538660..4b617d10cf28 100644
> --- a/fs/9p/vfs_file.c
> +++ b/fs/9p/vfs_file.c
> @@ -542,14 +542,27 @@ v9fs_vm_page_mkwrite(struct vm_fault *vmf)
> p9_debug(P9_DEBUG_VFS, "page %p fid %lx\n",
> page, (unsigned long)filp->private_data);
>
> + v9inode = V9FS_I(inode);
> +
> + /* Wait for the page to be written to the cache before we allow it to
> + * be modified. We then assume the entire page will need writing back.
> + */
> +#ifdef CONFIG_V9FS_FSCACHE

s/V9FS/9P/


--
Dominique