diff mbox series

[v6,4/7] fuse: negotiate per-file DAX in FUSE_INIT

Message ID 20211011030052.98923-5-jefflexu@linux.alibaba.com (mailing list archive)
State New, archived
Headers show
Series fuse,virtiofs: support per-file DAX | expand

Commit Message

Jingbo Xu Oct. 11, 2021, 3 a.m. UTC
Among the FUSE_INIT phase, client shall advertise per-file DAX if it's
mounted with "-o dax=inode". Then server is aware that client is in
per-file DAX mode, and will construct per-inode DAX attribute
accordingly.

Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
---
 fs/fuse/inode.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Vivek Goyal Oct. 18, 2021, 2:30 p.m. UTC | #1
On Mon, Oct 11, 2021 at 11:00:49AM +0800, Jeffle Xu wrote:
> Among the FUSE_INIT phase, client shall advertise per-file DAX if it's
> mounted with "-o dax=inode". Then server is aware that client is in
> per-file DAX mode, and will construct per-inode DAX attribute
> accordingly.
> 
> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> ---
>  fs/fuse/inode.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index b4b41683e97e..f4ad99e2415b 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1203,6 +1203,8 @@ void fuse_send_init(struct fuse_mount *fm)
>  #ifdef CONFIG_FUSE_DAX
>  	if (fm->fc->dax)
>  		ia->in.flags |= FUSE_MAP_ALIGNMENT;
> +	if (fm->fc->dax_mode == FUSE_DAX_INODE)
> +		ia->in.flags |= FUSE_PERFILE_DAX;

Are you not keeping track of server's response whether server supports
per inode dax or not. Client might be new and server might be old and
server might not support per inode dax. In that case, we probably 
should error out if user mounted with "-o dax=inode".

Vivek

>  #endif
>  	if (fm->fc->auto_submounts)
>  		ia->in.flags |= FUSE_SUBMOUNTS;
> -- 
> 2.27.0
>
Jingbo Xu Oct. 20, 2021, 3:10 a.m. UTC | #2
On 10/18/21 10:30 PM, Vivek Goyal wrote:
> On Mon, Oct 11, 2021 at 11:00:49AM +0800, Jeffle Xu wrote:
>> Among the FUSE_INIT phase, client shall advertise per-file DAX if it's
>> mounted with "-o dax=inode". Then server is aware that client is in
>> per-file DAX mode, and will construct per-inode DAX attribute
>> accordingly.
>>
>> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
>> ---
>>  fs/fuse/inode.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
>> index b4b41683e97e..f4ad99e2415b 100644
>> --- a/fs/fuse/inode.c
>> +++ b/fs/fuse/inode.c
>> @@ -1203,6 +1203,8 @@ void fuse_send_init(struct fuse_mount *fm)
>>  #ifdef CONFIG_FUSE_DAX
>>  	if (fm->fc->dax)
>>  		ia->in.flags |= FUSE_MAP_ALIGNMENT;
>> +	if (fm->fc->dax_mode == FUSE_DAX_INODE)
>> +		ia->in.flags |= FUSE_PERFILE_DAX;
> 
> Are you not keeping track of server's response whether server supports
> per inode dax or not. Client might be new and server might be old and
> server might not support per inode dax. In that case, we probably 
> should error out if user mounted with "-o dax=inode".
> 

Yes, if guest virtiofs is mounted with '-o dax=inode' while virtiofsd is
old and doesn't support per inode dax, then guest virtiofs will never
receive FUSE_ATTR_DAX and actually behaves as '-o dax=never'. So the
whole system works in this case, though the behavior may be beyond the
expectation of users ....

If the behavior really matters, I could change the behavior and fail
directly if virtiofsd doesn't advertise supporting per inode DAX.
Vivek Goyal Oct. 20, 2021, 3:44 p.m. UTC | #3
On Wed, Oct 20, 2021 at 11:10:30AM +0800, JeffleXu wrote:
> 
> 
> On 10/18/21 10:30 PM, Vivek Goyal wrote:
> > On Mon, Oct 11, 2021 at 11:00:49AM +0800, Jeffle Xu wrote:
> >> Among the FUSE_INIT phase, client shall advertise per-file DAX if it's
> >> mounted with "-o dax=inode". Then server is aware that client is in
> >> per-file DAX mode, and will construct per-inode DAX attribute
> >> accordingly.
> >>
> >> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> >> ---
> >>  fs/fuse/inode.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> >> index b4b41683e97e..f4ad99e2415b 100644
> >> --- a/fs/fuse/inode.c
> >> +++ b/fs/fuse/inode.c
> >> @@ -1203,6 +1203,8 @@ void fuse_send_init(struct fuse_mount *fm)
> >>  #ifdef CONFIG_FUSE_DAX
> >>  	if (fm->fc->dax)
> >>  		ia->in.flags |= FUSE_MAP_ALIGNMENT;
> >> +	if (fm->fc->dax_mode == FUSE_DAX_INODE)
> >> +		ia->in.flags |= FUSE_PERFILE_DAX;
> > 
> > Are you not keeping track of server's response whether server supports
> > per inode dax or not. Client might be new and server might be old and
> > server might not support per inode dax. In that case, we probably 
> > should error out if user mounted with "-o dax=inode".
> > 
> 
> Yes, if guest virtiofs is mounted with '-o dax=inode' while virtiofsd is
> old and doesn't support per inode dax, then guest virtiofs will never
> receive FUSE_ATTR_DAX and actually behaves as '-o dax=never'. So the
> whole system works in this case, though the behavior may be beyond the
> expectation of users ....
> 
> If the behavior really matters, I could change the behavior and fail
> directly if virtiofsd doesn't advertise supporting per inode DAX.

I think it probably is better to error out if client asked for per-inode
DAX and server does not support it. 

Vivek
> 
> -- 
> Thanks,
> Jeffle
>
diff mbox series

Patch

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index b4b41683e97e..f4ad99e2415b 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1203,6 +1203,8 @@  void fuse_send_init(struct fuse_mount *fm)
 #ifdef CONFIG_FUSE_DAX
 	if (fm->fc->dax)
 		ia->in.flags |= FUSE_MAP_ALIGNMENT;
+	if (fm->fc->dax_mode == FUSE_DAX_INODE)
+		ia->in.flags |= FUSE_PERFILE_DAX;
 #endif
 	if (fm->fc->auto_submounts)
 		ia->in.flags |= FUSE_SUBMOUNTS;