Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 21 Sep 2021 20:15:18 +0900
From:   Dominique Martinet <asmadeus@codewreck.org>
To:     David Howells <dhowells@redhat.com>
Cc:     Trond Myklebust <trondmy@hammerspace.com>,
        Anna Schumaker <anna.schumaker@netapp.com>,
        Steve French <sfrench@samba.org>,
        v9fs-developer@lists.sourceforge.net, linux-cachefs@redhat.com,
        Jeff Layton <jlayton@redhat.com>,
        Matthew Wilcox <willy@infradead.org>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        linux-afs@lists.infradead.org, linux-nfs@vger.kernel.org,
        linux-cifs@vger.kernel.org, ceph-devel@vger.kernel.org,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/8] 9p: (untested) Convert to using the netfs helper lib
 to do reads and caching
Message-ID: <YUm+xucHxED+1MJp@codewreck.org>
References: <163162767601.438332.9017034724960075707.stgit@warthog.procyon.org.uk>
 <163162772646.438332.16323773205855053535.stgit@warthog.procyon.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <163162772646.438332.16323773205855053535.stgit@warthog.procyon.org.uk>
Precedence: bulk

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 <dhowells@redhat.com>
> cc: Dominique Martinet <asmadeus@codewreck.org>

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: v9fs-developer@lists.sourceforge.net
> cc: linux-cachefs@redhat.com
> ---
> 
>  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