linux-kernel - Re: [PATCH 06/67] nfs, cifs, ceph, 9p: Disable use of fscache prior to its rewrite

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <67f55d920f40bf6c49643af08fe8a5cfc97a9542.camel@kernel.org>
Date:   Tue, 19 Oct 2021 13:50:24 -0400
From:   Jeff Layton <jlayton@...nel.org>
To:     David Howells <dhowells@...hat.com>, linux-cachefs@...hat.com
Cc:     Dave Wysochanski <dwysocha@...hat.com>,
        Trond Myklebust <trond.myklebust@...merspace.com>,
        Anna Schumaker <anna.schumaker@...app.com>,
        linux-nfs@...r.kernel.org, Ilya Dryomov <idryomov@...il.com>,
        ceph-devel@...r.kernel.org, Steve French <sfrench@...ba.org>,
        linux-cifs@...r.kernel.org, Eric Van Hensbergen <ericvh@...il.com>,
        Latchesar Ionkov <lucho@...kov.net>,
        Dominique Martinet <asmadeus@...ewreck.org>,
        v9fs-developer@...ts.sourceforge.net,
        Trond Myklebust <trondmy@...merspace.com>,
        Matthew Wilcox <willy@...radead.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Omar Sandoval <osandov@...ndov.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-afs@...ts.infradead.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 06/67] nfs, cifs, ceph, 9p: Disable use of fscache prior
 to its rewrite

On Mon, 2021-10-18 at 15:51 +0100, David Howells wrote:
> Temporarily disable the use of fscache by the various Linux network
> filesystems, apart from afs, so that the fscache core can be rewritten.
> 
> Signed-off-by: David Howells <dhowells@...hat.com>
> cc: Dave Wysochanski <dwysocha@...hat.com>
> cc: Trond Myklebust <trond.myklebust@...merspace.com>
> cc: Anna Schumaker <anna.schumaker@...app.com>
> cc: linux-nfs@...r.kernel.org
> cc: Jeff Layton <jlayton@...nel.org>
> cc: Ilya Dryomov <idryomov@...il.com>
> cc: ceph-devel@...r.kernel.org
> cc: Steve French <sfrench@...ba.org>
> cc: linux-cifs@...r.kernel.org
> cc: Eric Van Hensbergen <ericvh@...il.com>
> cc: Latchesar Ionkov <lucho@...kov.net>
> cc: Dominique Martinet <asmadeus@...ewreck.org>
> cc: v9fs-developer@...ts.sourceforge.net
> ---
> 
>  fs/9p/Kconfig      |    2 +-
>  fs/ceph/Kconfig    |    2 +-
>  fs/cifs/Kconfig    |    2 +-
>  fs/fscache/Kconfig |    4 ++++
>  fs/nfs/Kconfig     |    2 +-
>  5 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/9p/Kconfig b/fs/9p/Kconfig
> index d7bc93447c85..b11c15c30bac 100644
> --- a/fs/9p/Kconfig
> +++ b/fs/9p/Kconfig
> @@ -14,7 +14,7 @@ config 9P_FS
>  if 9P_FS
>  config 9P_FSCACHE
>  	bool "Enable 9P client caching support"
> -	depends on 9P_FS=m && FSCACHE || 9P_FS=y && FSCACHE=y
> +	depends on 9P_FS=m && FSCACHE_OLD || 9P_FS=y && FSCACHE_OLD=y
>  	help
>  	  Choose Y here to enable persistent, read-only local
>  	  caching support for 9p clients using FS-Cache
> diff --git a/fs/ceph/Kconfig b/fs/ceph/Kconfig
> index 94df854147d3..77ad452337ee 100644
> --- a/fs/ceph/Kconfig
> +++ b/fs/ceph/Kconfig
> @@ -21,7 +21,7 @@ config CEPH_FS
>  if CEPH_FS
>  config CEPH_FSCACHE
>  	bool "Enable Ceph client caching support"
> -	depends on CEPH_FS=m && FSCACHE || CEPH_FS=y && FSCACHE=y
> +	depends on CEPH_FS=m && FSCACHE_OLD || CEPH_FS=y && FSCACHE_OLD=y
>  	help
>  	  Choose Y here to enable persistent, read-only local
>  	  caching support for Ceph clients using FS-Cache
> diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig
> index 3b7e3b9e4fd2..c5477abbcff0 100644
> --- a/fs/cifs/Kconfig
> +++ b/fs/cifs/Kconfig
> @@ -188,7 +188,7 @@ config CIFS_SMB_DIRECT
>  
>  config CIFS_FSCACHE
>  	bool "Provide CIFS client caching support"
> -	depends on CIFS=m && FSCACHE || CIFS=y && FSCACHE=y
> +	depends on CIFS=m && FSCACHE_OLD || CIFS=y && FSCACHE_OLD=y
>  	help
>  	  Makes CIFS FS-Cache capable. Say Y here if you want your CIFS data
>  	  to be cached locally on disk through the general filesystem cache
> diff --git a/fs/fscache/Kconfig b/fs/fscache/Kconfig
> index b313a978ae0a..7850de3bdee0 100644
> --- a/fs/fscache/Kconfig
> +++ b/fs/fscache/Kconfig
> @@ -38,3 +38,7 @@ config FSCACHE_DEBUG
>  	  enabled by setting bits in /sys/modules/fscache/parameter/debug.
>  
>  	  See Documentation/filesystems/caching/fscache.rst for more information.
> +
> +config FSCACHE_OLD
> +	bool
> +	default n
> diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
> index 14a72224b657..a8b73c90aa00 100644
> --- a/fs/nfs/Kconfig
> +++ b/fs/nfs/Kconfig
> @@ -170,7 +170,7 @@ config ROOT_NFS
>  
>  config NFS_FSCACHE
>  	bool "Provide NFS client caching support"
> -	depends on NFS_FS=m && FSCACHE || NFS_FS=y && FSCACHE=y
> +	depends on NFS_FS=m && FSCACHE_OLD || NFS_FS=y && FSCACHE_OLD=y
>  	help
>  	  Say Y here if you want NFS data to be cached locally on disc through
>  	  the general filesystem cache manager
> 
> 

The typical way to do this would be to rebrand the existing FSCACHE
Kconfig symbols into FSCACHE_OLD and then build the new fscache
structure such that it exists in parallel with the old. You'd then just
drop the old infrastructure once all of the fs's are converted to the
new. You could even make them conflict with one another in Kconfig too,
so that only one could be built in during the transition period if
supporting both at runtime is too difficult.

This approach of disabling everything is much more of an all-or-nothing
affair. It may mean less "churn" overall, but it seems less "nice"
because you have an interval of commits where fscache is non-functional.

I'm not necessarily opposed to this approach, but I'd like to better
understand why doing it this way was preferred.
-- 
Jeff Layton <jlayton@...nel.org>