All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] fuse: add a new fuse init flag to relax restrictions in no cache mode
@ 2023-06-29  8:17 Hao Xu
  2023-06-29 17:13 ` [fuse-devel] " Bernd Schubert
  0 siblings, 1 reply; 4+ messages in thread
From: Hao Xu @ 2023-06-29  8:17 UTC (permalink / raw)
  To: fuse-devel; +Cc: miklos, bernd.schubert, linux-fsdevel, Wanpeng Li, cgxu519

From: Hao Xu <howeyxu@tencent.com>

FOPEN_DIRECT_IO is usually set by fuse daemon to indicate need of strong
coherency, e.g. network filesystems. Thus shared mmap is disabled since
it leverages page cache and may write to it, which may cause
inconsistence. But FOPEN_DIRECT_IO can be used not for coherency but to
reduce memory footprint as well, e.g. reduce guest memory usage with
virtiofs. Therefore, add a new fuse init flag FUSE_DIRECT_IO_RELAX to
relax restrictions in that mode, currently, it allows shared mmap.
One thing to note is to make sure it doesn't break coherency in your
use case.

Signed-off-by: Hao Xu <howeyxu@tencent.com>
---

v1 -> v2:
    make the new flag a fuse init one rather than a open flag since it's
    not common that different files in a filesystem has different
    strategy of shared mmap.

 fs/fuse/file.c            | 8 ++++++--
 fs/fuse/fuse_i.h          | 3 +++
 fs/fuse/inode.c           | 5 ++++-
 include/uapi/linux/fuse.h | 1 +
 4 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index bc4115288eec..871b66b54322 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -2478,14 +2478,18 @@ static const struct vm_operations_struct fuse_file_vm_ops = {
 static int fuse_file_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct fuse_file *ff = file->private_data;
+	struct fuse_conn *fc = ff->fm->fc;
 
 	/* DAX mmap is superior to direct_io mmap */
 	if (FUSE_IS_DAX(file_inode(file)))
 		return fuse_dax_mmap(file, vma);
 
 	if (ff->open_flags & FOPEN_DIRECT_IO) {
-		/* Can't provide the coherency needed for MAP_SHARED */
-		if (vma->vm_flags & VM_MAYSHARE)
+		/* Can't provide the coherency needed for MAP_SHARED
+		 * if FUSE_DIRECT_IO_RELAX isn't set.
+		 */
+		if (!(ff->open_flags & fc->direct_io_relax) &&
+		    vma->vm_flags & VM_MAYSHARE)
 			return -ENODEV;
 
 		invalidate_inode_pages2(file->f_mapping);
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 9b7fc7d3c7f1..d830c2360aef 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -792,6 +792,9 @@ struct fuse_conn {
 	/* Is tmpfile not implemented by fs? */
 	unsigned int no_tmpfile:1;
 
+	/* relax restrictions in FOPEN_DIRECT_IO mode */
+	unsigned int direct_io_relax:1;
+
 	/** The number of requests waiting for completion */
 	atomic_t num_waiting;
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index d66070af145d..049f9ee547d5 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1209,6 +1209,9 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
 				fc->init_security = 1;
 			if (flags & FUSE_CREATE_SUPP_GROUP)
 				fc->create_supp_group = 1;
+
+			if (flags & FUSE_DIRECT_IO_RELAX)
+				fc->direct_io_relax = 1;
 		} else {
 			ra_pages = fc->max_read / PAGE_SIZE;
 			fc->no_lock = 1;
@@ -1254,7 +1257,7 @@ void fuse_send_init(struct fuse_mount *fm)
 		FUSE_ABORT_ERROR | FUSE_MAX_PAGES | FUSE_CACHE_SYMLINKS |
 		FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA |
 		FUSE_HANDLE_KILLPRIV_V2 | FUSE_SETXATTR_EXT | FUSE_INIT_EXT |
-		FUSE_SECURITY_CTX | FUSE_CREATE_SUPP_GROUP;
+		FUSE_SECURITY_CTX | FUSE_CREATE_SUPP_GROUP | FUSE_DIRECT_IO_RELAX;
 #ifdef CONFIG_FUSE_DAX
 	if (fm->fc->dax)
 		flags |= FUSE_MAP_ALIGNMENT;
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 1b9d0dfae72d..2da2acec6bf4 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -406,6 +406,7 @@ struct fuse_file_lock {
 #define FUSE_SECURITY_CTX	(1ULL << 32)
 #define FUSE_HAS_INODE_DAX	(1ULL << 33)
 #define FUSE_CREATE_SUPP_GROUP	(1ULL << 34)
+#define FUSE_DIRECT_IO_RELAX	(1ULL << 35)
 
 /**
  * CUSE INIT request/reply flags
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [fuse-devel] [PATCH v2] fuse: add a new fuse init flag to relax restrictions in no cache mode
  2023-06-29  8:17 [PATCH v2] fuse: add a new fuse init flag to relax restrictions in no cache mode Hao Xu
@ 2023-06-29 17:13 ` Bernd Schubert
  2023-06-30  2:52   ` Hao Xu
  0 siblings, 1 reply; 4+ messages in thread
From: Bernd Schubert @ 2023-06-29 17:13 UTC (permalink / raw)
  To: Hao Xu, fuse-devel; +Cc: linux-fsdevel, Wanpeng Li, cgxu519, miklos



On 6/29/23 10:17, Hao Xu wrote:
> From: Hao Xu <howeyxu@tencent.com>
> 
> FOPEN_DIRECT_IO is usually set by fuse daemon to indicate need of strong
> coherency, e.g. network filesystems. Thus shared mmap is disabled since
> it leverages page cache and may write to it, which may cause
> inconsistence. But FOPEN_DIRECT_IO can be used not for coherency but to
> reduce memory footprint as well, e.g. reduce guest memory usage with
> virtiofs. Therefore, add a new fuse init flag FUSE_DIRECT_IO_RELAX to
> relax restrictions in that mode, currently, it allows shared mmap.
> One thing to note is to make sure it doesn't break coherency in your
> use case.
> 
> Signed-off-by: Hao Xu <howeyxu@tencent.com>
> ---
> 
> v1 -> v2:
>      make the new flag a fuse init one rather than a open flag since it's
>      not common that different files in a filesystem has different
>      strategy of shared mmap.
> 
>   fs/fuse/file.c            | 8 ++++++--
>   fs/fuse/fuse_i.h          | 3 +++
>   fs/fuse/inode.c           | 5 ++++-
>   include/uapi/linux/fuse.h | 1 +
>   4 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index bc4115288eec..871b66b54322 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -2478,14 +2478,18 @@ static const struct vm_operations_struct fuse_file_vm_ops = {
>   static int fuse_file_mmap(struct file *file, struct vm_area_struct *vma)
>   {
>   	struct fuse_file *ff = file->private_data;
> +	struct fuse_conn *fc = ff->fm->fc;
>   
>   	/* DAX mmap is superior to direct_io mmap */
>   	if (FUSE_IS_DAX(file_inode(file)))
>   		return fuse_dax_mmap(file, vma);
>   
>   	if (ff->open_flags & FOPEN_DIRECT_IO) {
> -		/* Can't provide the coherency needed for MAP_SHARED */
> -		if (vma->vm_flags & VM_MAYSHARE)
> +		/* Can't provide the coherency needed for MAP_SHARED
> +		 * if FUSE_DIRECT_IO_RELAX isn't set.
> +		 */
> +		if (!(ff->open_flags & fc->direct_io_relax) &&
> +		    vma->vm_flags & VM_MAYSHARE)
>   			return -ENODEV;

I'm confused here, the idea was that open_flags do not need additional 
flags? Why is this not just

if (ff->open_flags & FOPEN_DIRECT_IO) {
		/* Can't provide the coherency needed for MAP_SHARED */
		if (vma->vm_flags & VM_MAYSHARE && !fc->direct_io_relax)
			return -ENODEV;


>   
>   		invalidate_inode_pages2(file->f_mapping);
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 9b7fc7d3c7f1..d830c2360aef 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -792,6 +792,9 @@ struct fuse_conn {
>   	/* Is tmpfile not implemented by fs? */
>   	unsigned int no_tmpfile:1;
>   
> +	/* relax restrictions in FOPEN_DIRECT_IO mode */
> +	unsigned int direct_io_relax:1;
> +
>   	/** The number of requests waiting for completion */
>   	atomic_t num_waiting;
>   
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index d66070af145d..049f9ee547d5 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1209,6 +1209,9 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
>   				fc->init_security = 1;
>   			if (flags & FUSE_CREATE_SUPP_GROUP)
>   				fc->create_supp_group = 1;
> +
> +			if (flags & FUSE_DIRECT_IO_RELAX)
> +				fc->direct_io_relax = 1;
>   		} else {
>   			ra_pages = fc->max_read / PAGE_SIZE;
>   			fc->no_lock = 1;
> @@ -1254,7 +1257,7 @@ void fuse_send_init(struct fuse_mount *fm)
>   		FUSE_ABORT_ERROR | FUSE_MAX_PAGES | FUSE_CACHE_SYMLINKS |
>   		FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA |
>   		FUSE_HANDLE_KILLPRIV_V2 | FUSE_SETXATTR_EXT | FUSE_INIT_EXT |
> -		FUSE_SECURITY_CTX | FUSE_CREATE_SUPP_GROUP;
> +		FUSE_SECURITY_CTX | FUSE_CREATE_SUPP_GROUP | FUSE_DIRECT_IO_RELAX;
>   #ifdef CONFIG_FUSE_DAX
>   	if (fm->fc->dax)
>   		flags |= FUSE_MAP_ALIGNMENT;
> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index 1b9d0dfae72d..2da2acec6bf4 100644
> --- a/include/uapi/linux/fuse.h
> +++ b/include/uapi/linux/fuse.h
> @@ -406,6 +406,7 @@ struct fuse_file_lock {
>   #define FUSE_SECURITY_CTX	(1ULL << 32)
>   #define FUSE_HAS_INODE_DAX	(1ULL << 33)
>   #define FUSE_CREATE_SUPP_GROUP	(1ULL << 34)
> +#define FUSE_DIRECT_IO_RELAX	(1ULL << 35)
>   
>   /**
>    * CUSE INIT request/reply flags


Thanks,
Bernd

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [fuse-devel] [PATCH v2] fuse: add a new fuse init flag to relax restrictions in no cache mode
  2023-06-29 17:13 ` [fuse-devel] " Bernd Schubert
@ 2023-06-30  2:52   ` Hao Xu
  0 siblings, 0 replies; 4+ messages in thread
From: Hao Xu @ 2023-06-30  2:52 UTC (permalink / raw)
  To: Bernd Schubert, fuse-devel; +Cc: linux-fsdevel, Wanpeng Li, cgxu519, miklos

Hi Bernd,

On 6/30/23 01:13, Bernd Schubert wrote:
> 
> 
> On 6/29/23 10:17, Hao Xu wrote:
>> From: Hao Xu <howeyxu@tencent.com>
>>
>> FOPEN_DIRECT_IO is usually set by fuse daemon to indicate need of strong
>> coherency, e.g. network filesystems. Thus shared mmap is disabled since
>> it leverages page cache and may write to it, which may cause
>> inconsistence. But FOPEN_DIRECT_IO can be used not for coherency but to
>> reduce memory footprint as well, e.g. reduce guest memory usage with
>> virtiofs. Therefore, add a new fuse init flag FUSE_DIRECT_IO_RELAX to
>> relax restrictions in that mode, currently, it allows shared mmap.
>> One thing to note is to make sure it doesn't break coherency in your
>> use case.
>>
>> Signed-off-by: Hao Xu <howeyxu@tencent.com>
>> ---
>>
>> v1 -> v2:
>>      make the new flag a fuse init one rather than a open flag since it's
>>      not common that different files in a filesystem has different
>>      strategy of shared mmap.
>>
>>   fs/fuse/file.c            | 8 ++++++--
>>   fs/fuse/fuse_i.h          | 3 +++
>>   fs/fuse/inode.c           | 5 ++++-
>>   include/uapi/linux/fuse.h | 1 +
>>   4 files changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
>> index bc4115288eec..871b66b54322 100644
>> --- a/fs/fuse/file.c
>> +++ b/fs/fuse/file.c
>> @@ -2478,14 +2478,18 @@ static const struct vm_operations_struct 
>> fuse_file_vm_ops = {
>>   static int fuse_file_mmap(struct file *file, struct vm_area_struct 
>> *vma)
>>   {
>>       struct fuse_file *ff = file->private_data;
>> +    struct fuse_conn *fc = ff->fm->fc;
>>       /* DAX mmap is superior to direct_io mmap */
>>       if (FUSE_IS_DAX(file_inode(file)))
>>           return fuse_dax_mmap(file, vma);
>>       if (ff->open_flags & FOPEN_DIRECT_IO) {
>> -        /* Can't provide the coherency needed for MAP_SHARED */
>> -        if (vma->vm_flags & VM_MAYSHARE)
>> +        /* Can't provide the coherency needed for MAP_SHARED
>> +         * if FUSE_DIRECT_IO_RELAX isn't set.
>> +         */
>> +        if (!(ff->open_flags & fc->direct_io_relax) &&
>> +            vma->vm_flags & VM_MAYSHARE)
>>               return -ENODEV;
> 
> I'm confused here, the idea was that open_flags do not need additional 
> flags? Why is this not just
> 

sorry for this, seems I sent a WIP version by accident.., I'll fix it soon.

Thanks,
Hao


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] fuse: add a new fuse init flag to relax restrictions in no cache mode
@ 2023-06-30  1:10 kernel test robot
  0 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2023-06-30  1:10 UTC (permalink / raw)
  To: oe-kbuild; +Cc: lkp, Dan Carpenter

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20230629081733.11309-1-hao.xu@linux.dev>
References: <20230629081733.11309-1-hao.xu@linux.dev>
TO: Hao Xu <hao.xu@linux.dev>
TO: fuse-devel@lists.sourceforge.net
CC: miklos@szeredi.hu
CC: bernd.schubert@fastmail.fm
CC: linux-fsdevel@vger.kernel.org
CC: Wanpeng Li <wanpengli@tencent.com>
CC: cgxu519@mykernel.net

Hi Hao,

kernel test robot noticed the following build warnings:

[auto build test WARNING on v6.4]
[also build test WARNING on linus/master]
[cannot apply to mszeredi-fuse/for-next next-20230629]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Hao-Xu/fuse-add-a-new-fuse-init-flag-to-relax-restrictions-in-no-cache-mode/20230629-162139
base:   v6.4
patch link:    https://lore.kernel.org/r/20230629081733.11309-1-hao.xu%40linux.dev
patch subject: [PATCH v2] fuse: add a new fuse init flag to relax restrictions in no cache mode
:::::: branch date: 17 hours ago
:::::: commit date: 17 hours ago
config: i386-randconfig-m021-20230629 (https://download.01.org/0day-ci/archive/20230630/202306300807.6nFdb8tj-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230630/202306300807.6nFdb8tj-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202306300807.6nFdb8tj-lkp@intel.com/

smatch warnings:
fs/fuse/file.c:2516 fuse_file_mmap() warn: maybe use && instead of &

vim +2516 fs/fuse/file.c

3be5a52b30aa5c Miklos Szeredi  2008-04-30  2502  
3be5a52b30aa5c Miklos Szeredi  2008-04-30  2503  static int fuse_file_mmap(struct file *file, struct vm_area_struct *vma)
3be5a52b30aa5c Miklos Szeredi  2008-04-30  2504  {
55752a3aba1387 Miklos Szeredi  2019-01-24  2505  	struct fuse_file *ff = file->private_data;
fad410b49bfb66 Hao Xu          2023-06-29  2506  	struct fuse_conn *fc = ff->fm->fc;
b6aeadeda22a9a Miklos Szeredi  2005-09-09  2507  
2a9a609a0c4a3b Stefan Hajnoczi 2020-08-19  2508  	/* DAX mmap is superior to direct_io mmap */
2a9a609a0c4a3b Stefan Hajnoczi 2020-08-19  2509  	if (FUSE_IS_DAX(file_inode(file)))
2a9a609a0c4a3b Stefan Hajnoczi 2020-08-19  2510  		return fuse_dax_mmap(file, vma);
2a9a609a0c4a3b Stefan Hajnoczi 2020-08-19  2511  
55752a3aba1387 Miklos Szeredi  2019-01-24  2512  	if (ff->open_flags & FOPEN_DIRECT_IO) {
fad410b49bfb66 Hao Xu          2023-06-29  2513  		/* Can't provide the coherency needed for MAP_SHARED
fad410b49bfb66 Hao Xu          2023-06-29  2514  		 * if FUSE_DIRECT_IO_RELAX isn't set.
fad410b49bfb66 Hao Xu          2023-06-29  2515  		 */
fad410b49bfb66 Hao Xu          2023-06-29 @2516  		if (!(ff->open_flags & fc->direct_io_relax) &&
fad410b49bfb66 Hao Xu          2023-06-29  2517  		    vma->vm_flags & VM_MAYSHARE)
fc280c9692031e Miklos Szeredi  2009-04-02  2518  			return -ENODEV;
fc280c9692031e Miklos Szeredi  2009-04-02  2519  
3121bfe7631126 Miklos Szeredi  2009-04-09  2520  		invalidate_inode_pages2(file->f_mapping);
3121bfe7631126 Miklos Szeredi  2009-04-09  2521  
fc280c9692031e Miklos Szeredi  2009-04-02  2522  		return generic_file_mmap(file, vma);
fc280c9692031e Miklos Szeredi  2009-04-02  2523  	}
fc280c9692031e Miklos Szeredi  2009-04-02  2524  
55752a3aba1387 Miklos Szeredi  2019-01-24  2525  	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE))
55752a3aba1387 Miklos Szeredi  2019-01-24  2526  		fuse_link_write_file(file);
55752a3aba1387 Miklos Szeredi  2019-01-24  2527  
55752a3aba1387 Miklos Szeredi  2019-01-24  2528  	file_accessed(file);
55752a3aba1387 Miklos Szeredi  2019-01-24  2529  	vma->vm_ops = &fuse_file_vm_ops;
55752a3aba1387 Miklos Szeredi  2019-01-24  2530  	return 0;
55752a3aba1387 Miklos Szeredi  2019-01-24  2531  }
55752a3aba1387 Miklos Szeredi  2019-01-24  2532  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-06-30  2:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-29  8:17 [PATCH v2] fuse: add a new fuse init flag to relax restrictions in no cache mode Hao Xu
2023-06-29 17:13 ` [fuse-devel] " Bernd Schubert
2023-06-30  2:52   ` Hao Xu
2023-06-30  1:10 kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.