[Virtio-fs] [PATCH] virtiofsd: Reset O_DIRECT flag during file open

[Virtio-fs] [PATCH] virtiofsd: Reset O_DIRECT flag during file open

[Vivek Goyal]

If an application wants to do direct IO and opens a file with O_DIRECT
in guest, that does not necessarily mean that we need to bypass page
cache on host as well. So reset this flag on host.

If somebody needs to bypass page cache on host as well (and it is safe to
do so), we can add a knob in daemon later to control this behavior.

I check virtio-9p and they do reset O_DIRECT flag.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 contrib/virtiofsd/passthrough_ll.c |    7 +++++++
 1 file changed, 7 insertions(+)

Index: qemu/contrib/virtiofsd/passthrough_ll.c
===================================================================
--- qemu.orig/contrib/virtiofsd/passthrough_ll.c	2019-08-20 09:33:07.900975145 -0400
+++ qemu/contrib/virtiofsd/passthrough_ll.c	2019-08-20 09:35:45.829414412 -0400
@@ -1967,6 +1967,13 @@ static void lo_open(fuse_req_t req, fuse
 	if (lo->writeback && (fi->flags & O_APPEND))
 		fi->flags &= ~O_APPEND;
 
+	/*
+	 * O_DIRECT in guest should not necessarily mean bypassing page
+	 * cache on host as well. If somebody needs that behavior, it
+	 * probably should be a configuration knob in daemon.
+	 */
+	fi->flags &= ~O_DIRECT;
+
 	sprintf(buf, "%i", lo_fd(req, ino));
 	fd = openat(lo->proc_self_fd, buf, fi->flags & ~O_NOFOLLOW);
 	if (fd == -1)

Re: [Virtio-fs] [PATCH] virtiofsd: Reset O_DIRECT flag during file open

[piaojun]

Hi Vivek & Aneesh,

On 2019/8/20 21:45, Vivek Goyal wrote:
> If an application wants to do direct IO and opens a file with O_DIRECT
> in guest, that does not necessarily mean that we need to bypass page
> cache on host as well. So reset this flag on host.

I have a different understanding about this. The I/O mode should be kept
same both in Guest and Host. When doing direct io in Guest, the user may
want the data go to disk rather than kept in Host page cache which means
volatile. So it seems more reasonable that Guest and Host have the same
file I/0 semantics.

> 
> If somebody needs to bypass page cache on host as well (and it is safe to
> do so), we can add a knob in daemon later to control this behavior.

This discussion has some relationship with my RFC PATCH "virtiofsd: do
not fall back to buffer io when cache=auto/always", and we could involve
the issue mentioned in that email if you do not mind.

> 
> I check virtio-9p and they do reset O_DIRECT flag.

I just checked the virtio-9p code and found some strange comments which
seemed to say the server could not support direct I/O temporary. With
all these questions, I will CC the author of the code, and discuss them
together.

static int get_dotl_openflags(V9fsState *s, int oflags)
{
    int flags;
    /*
     * Filter the client open flags
     */
    flags = dotl_to_open_flags(oflags);
    flags &= ~(O_NOCTTY | O_ASYNC | O_CREAT);
    /*
     * Ignore direct disk access hint until the server supports it.
     */
    flags &= ~O_DIRECT;
    return flags;
}

> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>  contrib/virtiofsd/passthrough_ll.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> Index: qemu/contrib/virtiofsd/passthrough_ll.c
> ===================================================================
> --- qemu.orig/contrib/virtiofsd/passthrough_ll.c	2019-08-20 09:33:07.900975145 -0400
> +++ qemu/contrib/virtiofsd/passthrough_ll.c	2019-08-20 09:35:45.829414412 -0400
> @@ -1967,6 +1967,13 @@ static void lo_open(fuse_req_t req, fuse
>  	if (lo->writeback && (fi->flags & O_APPEND))
>  		fi->flags &= ~O_APPEND;
>  
> +	/*
> +	 * O_DIRECT in guest should not necessarily mean bypassing page
> +	 * cache on host as well. If somebody needs that behavior, it
> +	 * probably should be a configuration knob in daemon.
> +	 */
> +	fi->flags &= ~O_DIRECT;
> +

We need do the same thing for lo_create().

Thanks,
Jun

>  	sprintf(buf, "%i", lo_fd(req, ino));
>  	fd = openat(lo->proc_self_fd, buf, fi->flags & ~O_NOFOLLOW);
>  	if (fd == -1)
> 
> _______________________________________________
> Virtio-fs mailing list
> Virtio-fs@redhat.com
> https://www.redhat.com/mailman/listinfo/virtio-fs
> 

Re: [Virtio-fs] [PATCH] virtiofsd: Reset O_DIRECT flag during file open

[Vivek Goyal]

On Tue, Aug 20, 2019 at 10:33:29PM +0800, piaojun wrote:
> Hi Vivek & Aneesh,
> 
> On 2019/8/20 21:45, Vivek Goyal wrote:
> > If an application wants to do direct IO and opens a file with O_DIRECT
> > in guest, that does not necessarily mean that we need to bypass page
> > cache on host as well. So reset this flag on host.
> 
> I have a different understanding about this. The I/O mode should be kept
> same both in Guest and Host. When doing direct io in Guest, the user may
> want the data go to disk rather than kept in Host page cache which means
> volatile. So it seems more reasonable that Guest and Host have the same
> file I/0 semantics.

I look at it other way. If we open a file O_DIRECT, it should bypass
page cache on kernel it is running on. It does not guarantee anything
on underlying storage stack. For example, virtual block device or physical
disks could still be caching data with O_DIRECT.

To me same analogy should apply for caching on host. Atleast by default
O_DIRECT should only mean bypassing page cache in guest. If there is
an application which requires bypassing page cache on host as well, then
it should be a server option to control this behavior. But we can do when
there is a real user.

> 
> > 
> > If somebody needs to bypass page cache on host as well (and it is safe to
> > do so), we can add a knob in daemon later to control this behavior.
> 
> This discussion has some relationship with my RFC PATCH "virtiofsd: do
> not fall back to buffer io when cache=auto/always", and we could involve
> the issue mentioned in that email if you do not mind.
> 

I responded to your patch separately and that seems completely unrelated
to this patch.

[..]
> > +	/*
> > +	 * O_DIRECT in guest should not necessarily mean bypassing page
> > +	 * cache on host as well. If somebody needs that behavior, it
> > +	 * probably should be a configuration knob in daemon.
> > +	 */
> > +	fi->flags &= ~O_DIRECT;
> > +
> 
> We need do the same thing for lo_create().

That's a good point. I have fixed it and will post V2.

Vivek

Re: [Virtio-fs] [PATCH] virtiofsd: Reset O_DIRECT flag during file open

[piaojun]

On 2019/8/21 2:36, Vivek Goyal wrote:
> On Tue, Aug 20, 2019 at 10:33:29PM +0800, piaojun wrote:
>> Hi Vivek & Aneesh,
>>
>> On 2019/8/20 21:45, Vivek Goyal wrote:
>>> If an application wants to do direct IO and opens a file with O_DIRECT
>>> in guest, that does not necessarily mean that we need to bypass page
>>> cache on host as well. So reset this flag on host.
>>
>> I have a different understanding about this. The I/O mode should be kept
>> same both in Guest and Host. When doing direct io in Guest, the user may
>> want the data go to disk rather than kept in Host page cache which means
>> volatile. So it seems more reasonable that Guest and Host have the same
>> file I/0 semantics.
> 
> I look at it other way. If we open a file O_DIRECT, it should bypass
> page cache on kernel it is running on. It does not guarantee anything
> on underlying storage stack. For example, virtual block device or physical
> disks could still be caching data with O_DIRECT.
> 
> To me same analogy should apply for caching on host. Atleast by default
> O_DIRECT should only mean bypassing page cache in guest. If there is
> an application which requires bypassing page cache on host as well, then
> it should be a server option to control this behavior. But we can do when
> there is a real user.

This strategy also looks good to me if the user could control caches
both in Guest and Host separatly. So the knob you mentioned will be OK.

> 
>>
>>>
>>> If somebody needs to bypass page cache on host as well (and it is safe to
>>> do so), we can add a knob in daemon later to control this behavior.
>>
>> This discussion has some relationship with my RFC PATCH "virtiofsd: do
>> not fall back to buffer io when cache=auto/always", and we could involve
>> the issue mentioned in that email if you do not mind.
>>
> 
> I responded to your patch separately and that seems completely unrelated
> to this patch.

OK, I will reply in my patch.

Jun

Re: [Virtio-fs] [PATCH] virtiofsd: Reset O_DIRECT flag during file open

[Vivek Goyal]

On Wed, Aug 21, 2019 at 10:36:08AM +0800, piaojun wrote:
> 
> 
> On 2019/8/21 2:36, Vivek Goyal wrote:
> > On Tue, Aug 20, 2019 at 10:33:29PM +0800, piaojun wrote:
> >> Hi Vivek & Aneesh,
> >>
> >> On 2019/8/20 21:45, Vivek Goyal wrote:
> >>> If an application wants to do direct IO and opens a file with O_DIRECT
> >>> in guest, that does not necessarily mean that we need to bypass page
> >>> cache on host as well. So reset this flag on host.
> >>
> >> I have a different understanding about this. The I/O mode should be kept
> >> same both in Guest and Host. When doing direct io in Guest, the user may
> >> want the data go to disk rather than kept in Host page cache which means
> >> volatile. So it seems more reasonable that Guest and Host have the same
> >> file I/0 semantics.
> > 
> > I look at it other way. If we open a file O_DIRECT, it should bypass
> > page cache on kernel it is running on. It does not guarantee anything
> > on underlying storage stack. For example, virtual block device or physical
> > disks could still be caching data with O_DIRECT.
> > 
> > To me same analogy should apply for caching on host. Atleast by default
> > O_DIRECT should only mean bypassing page cache in guest. If there is
> > an application which requires bypassing page cache on host as well, then
> > it should be a server option to control this behavior. But we can do when
> > there is a real user.
> 
> This strategy also looks good to me if the user could control caches
> both in Guest and Host separatly. So the knob you mentioned will be OK.

I am not adding any more knobs right now. Once there are real users who
need this functionality, we can add it later.

Thanks
Vivek