Linux-EROFS Archive mirror
 help / color / mirror / Atom feed
From: Gao Xiang <hsiangkao@linux.alibaba.com>
To: Jingbo Xu <jefflexu@linux.alibaba.com>,
	David Howells <dhowells@redhat.com>, Gao Xiang <xiang@kernel.org>
Cc: Dominique Martinet <asmadeus@codewreck.org>,
	linux-mm@kvack.org, Marc Dionne <marc.dionne@auristor.com>,
	linux-afs@lists.infradead.org, Paulo Alcantara <pc@manguebit.com>,
	linux-cifs@vger.kernel.org, Matthew Wilcox <willy@infradead.org>,
	Steve French <smfrench@gmail.com>,
	linux-cachefs@redhat.com, Ilya Dryomov <idryomov@gmail.com>,
	Shyam Prasad N <sprasad@microsoft.com>,
	Tom Talpey <tom@talpey.com>, Yue Hu <huyue2@coolpad.com>,
	ceph-devel@vger.kernel.org,
	Eric Van Hensbergen <ericvh@kernel.org>,
	Christian Brauner <christian@brauner.io>,
	linux-nfs@vger.kernel.org, netdev@vger.kernel.org,
	v9fs@lists.linux.dev, Jeff Layton <jlayton@kernel.org>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-erofs@lists.ozlabs.org
Subject: Re: [PATCH] Fix EROFS Kconfig
Date: Sat, 23 Dec 2023 21:32:07 +0800	[thread overview]
Message-ID: <fac01751-73dc-4d93-b9c0-b637fece8334@linux.alibaba.com> (raw)
In-Reply-To: <d50555e9-3b8e-41d4-bec6-317aaaec5ff0@linux.alibaba.com>

Hi David and Jingbo,

On 2023/12/23 11:55, Jingbo Xu wrote:
> Hi,
> 
> On 12/22/23 9:02 PM, David Howells wrote:
>> This needs an additional change (see attached).
>>
>> diff --git a/fs/erofs/Kconfig b/fs/erofs/Kconfig
>> index 1d318f85232d..1949763e66aa 100644
>> --- a/fs/erofs/Kconfig
>> +++ b/fs/erofs/Kconfig
>> @@ -114,7 +114,8 @@ config EROFS_FS_ZIP_DEFLATE
>>   
>>   config EROFS_FS_ONDEMAND
>>   	bool "EROFS fscache-based on-demand read support"
>> -	depends on CACHEFILES_ONDEMAND && (EROFS_FS=m && FSCACHE || EROFS_FS=y && FSCACHE=y)
>> +	depends on CACHEFILES_ONDEMAND && FSCACHE && \
>> +		(EROFS_FS=m && NETFS_SUPPORT || EROFS_FS=y && NETFS_SUPPORT=y)
>>   	default n
>>   	help
>>   	  This permits EROFS to use fscache-backed data blobs with on-demand
>>
> 
> Thanks for the special reminder.  I noticed that it has been included in
> this commit[*] in the dev tree.
> 
> [*]
> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=netfs-lib&id=7472173cc3baf4a0bd8c803e56c37efdb8388f1c
> 
> 
> Besides I noticed an issue when trying to configure EROFS_FS_ONDEMAND.
> The above kconfig indicates that EROFS_FS_ONDEMAND depends on
> NETFS_SUPPORT, while NETFS_SUPPORT has no prompt in menuconfig and can
> only be selected by, e.g. fs/ceph/Kconfig:
> 
> 	config CEPH_FS
>          select NETFS_SUPPORT
> 
> IOW EROFS_FS_ONDEMAND will not be prompted and has no way being
> configured if NETFS_SUPPORT itself is not selected by any other filesystem.
> 
> 
> I tried to fix this in following way:
> 
> diff --git a/fs/erofs/Kconfig b/fs/erofs/Kconfig
> index 1949763e66aa..5b7b71e537f1 100644
> --- a/fs/erofs/Kconfig
> +++ b/fs/erofs/Kconfig
> @@ -5,6 +5,7 @@ config EROFS_FS
>          depends on BLOCK
>          select FS_IOMAP
>          select LIBCRC32C
> +       select NETFS_SUPPORT if EROFS_FS_ONDEMAND
>          help
>            EROFS (Enhanced Read-Only File System) is a lightweight read-only
>            file system with modern designs (e.g. no buffer heads, inline
> @@ -114,8 +115,10 @@ config EROFS_FS_ZIP_DEFLATE
> 
>   config EROFS_FS_ONDEMAND
>          bool "EROFS fscache-based on-demand read support"
> -       depends on CACHEFILES_ONDEMAND && FSCACHE && \
> -               (EROFS_FS=m && NETFS_SUPPORT || EROFS_FS=y &&
> NETFS_SUPPORT=y)
> +       depends on EROFS_FS
> +       select FSCACHE
>          default n
>          help
>            This permits EROFS to use fscache-backed data blobs with on-demand
> 
> 
> But still the dependency for CACHEFILES_ONDEMAND and CACHEFILES can not
> be resolved.  Though CACHEFILES is not a must during the linking stage
> as EROFS only calls fscache APIs directly, CACHEFILES is indeed needed
> to ensure that the EROFS on-demand functionality works at runtime.
> 
> If we let EROFS_FS_ONDEMAND select CACHEFILES_ONDEMAND, then only
> CACHEFILES_ONDEMAND will be selected while CACHEFILES can be still N.
> Maybe EROFS_FS_ONDEMAND needs to selct both CACHEFILES_ONDEMAND and
> CACHEFILES?

I think the main point here is that we don't have an explicit
menuconfig item for either netfs or fscache directly.

In principle, EROFS ondemand feature only needs fscache "volume
and cookie" management framework as well as cachefiles since
they're all needed to manage EROFS cached blobs, but I'm fine
if that needs NETFS_SUPPORT is also enabled.

If netfs doesn't have a plan for a new explicit menuconfig
item for users to use, I think we have to enable as below:

diff --git a/fs/erofs/Kconfig b/fs/erofs/Kconfig
index 1d318f85232d..fffd3919343e 100644
--- a/fs/erofs/Kconfig
+++ b/fs/erofs/Kconfig
@@ -114,8 +114,11 @@ config EROFS_FS_ZIP_DEFLATE

  config EROFS_FS_ONDEMAND
  	bool "EROFS fscache-based on-demand read support"
-	depends on CACHEFILES_ONDEMAND && (EROFS_FS=m && FSCACHE || EROFS_FS=y && FSCACHE=y)
-	default n
+	depends on EROFS_FS
+	select NETFS_SUPPORT
+	select FSCACHE
+	select CACHEFILES
+	select CACHEFILES_ONDEMAND
  	help
  	  This permits EROFS to use fscache-backed data blobs with on-demand
  	  read support.
--
2.39.3

But cachefiles won't be complied as modules anymore. Does it
sounds good?

Thanks,
Gao Xiang

      reply	other threads:[~2023-12-23 13:32 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20231221132400.1601991-5-dhowells@redhat.com>
     [not found] ` <20231221132400.1601991-1-dhowells@redhat.com>
2023-12-22 13:02   ` [PATCH] Fix EROFS Kconfig David Howells
2023-12-23  3:55     ` Jingbo Xu
2023-12-23 13:32       ` Gao Xiang [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fac01751-73dc-4d93-b9c0-b637fece8334@linux.alibaba.com \
    --to=hsiangkao@linux.alibaba.com \
    --cc=asmadeus@codewreck.org \
    --cc=ceph-devel@vger.kernel.org \
    --cc=christian@brauner.io \
    --cc=dhowells@redhat.com \
    --cc=ericvh@kernel.org \
    --cc=huyue2@coolpad.com \
    --cc=idryomov@gmail.com \
    --cc=jefflexu@linux.alibaba.com \
    --cc=jlayton@kernel.org \
    --cc=linux-afs@lists.infradead.org \
    --cc=linux-cachefs@redhat.com \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-erofs@lists.ozlabs.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=marc.dionne@auristor.com \
    --cc=netdev@vger.kernel.org \
    --cc=pc@manguebit.com \
    --cc=smfrench@gmail.com \
    --cc=sprasad@microsoft.com \
    --cc=tom@talpey.com \
    --cc=v9fs@lists.linux.dev \
    --cc=willy@infradead.org \
    --cc=xiang@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).