9pfs: don't ignore O_DIRECT flag in the 9pfs server
PATCH 1/1 9pfs: don't ignore O_DIRECT flag in the 9pfs server
Now v9fs in linux has already supported O_DIRECT(v9fs_direct_IO), when guest user open file with O_DIRECT flag and return success, so user hopes data doesn't pass through page cache, but 9pfs in qemu ignore direct disk access and use host page cache, it is not match to DIRECT_IO semantic, so we should not ignore O_DIRECT in 9pfs unless v9fs in linux don't support DIRECT_IO.
And if server fs don't support O_DIRECT, user will receive -EINVAL and know the filesystem don't support O_DIRECT.
So in order to ensure semantic consistency, don't ignore direct disk access in 9pfs.
--- hw/9pfs/9p.c | 4 ---- 1 file changed, 4 deletions(-)
hw/9pfs/9p.c +0 -4
@@ -155,10 +155,6 @@ static int get_dotl_openflags(V9fsState *s, int oflags)*/ 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; }
Quoted 7 lines · On Mon, 20 Nov 2017 13:48:59 +0800 jiangyiwen <[email protected]> wrote:
Now v9fs in linux has already supported
O_DIRECT(v9fs_direct_IO), when guest user open file withO_DIRECTflag and return success, so user hopes data doesn't pass through page cache, but 9pfs in qemu ignore direct disk access and use host page cache, it is not match toDIRECT_IOsemantic, so we should not ignoreO_DIRECTin 9pfs unless v9fs in linux don't supportDIRECT_IO.
Quoted 6 lines
And if server fs don't support
O_DIRECT, user will receive-EINVALand know the filesystem don't supportO_DIRECT.So in order to ensure semantic consistency, don't ignore direct disk access in 9pfs.
There are good reasons for us to ignore O_DIRECT. AFAIK, O_DIRECT requires the application to take care of the alignment of the buffers with either the logical block size of the filesystem (linux <= 2.4) or the logical block size of the underlying storage... I don't really see how you can achieve that since the linux 9p client only cares to break up requests into msize-chunks, and ignores alignment. ie, you're likely to not ensure semantic consistency on the host side anyway and linux will fallback to cached I/O.
Also, this change would silently break existing users of O_DIRECT, which isn't acceptable... it would require some fsdev property to enable this new behavior.
Quoted 20 lines
Signed-off-by Yiwen Jiang <[email protected]>--- hw/9pfs/9p.c | 4 ---- 1 file changed, 4 deletions(-)hw/9pfs/9p.c +0
-4@@ -155,10 +155,6 @@ static int get_dotl_openflags(V9fsState *s, int oflags)*/ 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; }
Quoted 25 lines · Greg Kurz
Quoted 7 lines · On Mon, 20 Nov 2017 13:48:59 +0800 jiangyiwen <[email protected]> wrote:
Now v9fs in linux has already supported
O_DIRECT(v9fs_direct_IO), when guest user open file withO_DIRECTflag and return success, so user hopes data doesn't pass through page cache, but 9pfs in qemu ignore direct disk access and use host page cache, it is not match toDIRECT_IOsemantic, so we should not ignoreO_DIRECTin 9pfs unless v9fs in linux don't supportDIRECT_IO.Quoted 6 lines
And if server fs don't support
O_DIRECT, user will receive-EINVALand know the filesystem don't supportO_DIRECT.So in order to ensure semantic consistency, don't ignore direct disk access in 9pfs.
There are good reasons for us to ignore
O_DIRECT. AFAIK,O_DIRECTrequires the application to take care of the alignment of the buffers with either the logical block size of the filesystem (linux <= 2.4) or the logical block size of the underlying storage... I don't really see how you can achieve that since the linux 9p client only cares to break up requests into msize-chunks, and ignores alignment. ie, you're likely to not ensure semantic consistency on the host side anyway and linux will fallback to cached I/O.
Quoted 3 lines
Also, this change would silently break existing users of
O_DIRECT, which isn't acceptable... it would require some fsdev property to enable this new behavior.
Thank you very much, I understand what your mean, v9fs in linux doesn't take care of the alignment, only split as the msize. Then there is another problem, should v9fs support DIRECT_IO? or delete DIRECT_IO ops in v9fs?
Thanks,
Yiwen.
Quoted 24 lines
Quoted 10 lines
Signed-off-by Yiwen Jiang <[email protected]>--- hw/9pfs/9p.c | 4 ---- 1 file changed, 4 deletions(-)hw/9pfs/9p.c +0
-0hw/9pfs/9p.c static int get_dotl_openflags(V9fsState *s, int oflags) · line 160
Quoted 10 lines
hw/9pfs/9p.c +0
-4@@ -155,10 +155,6 @@ static int get_dotl_openflags(V9fsState *s, int oflags)*/ 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; }.
Quoted 26 lines · On Tue, 21 Nov 2017 11:28:29 +0800 jiangyiwen <[email protected]> wrote:
Quoted 25 lines · Greg Kurz
Quoted 7 lines · On Mon, 20 Nov 2017 13:48:59 +0800 jiangyiwen <[email protected]> wrote:
Now v9fs in linux has already supported
O_DIRECT(v9fs_direct_IO), when guest user open file withO_DIRECTflag and return success, so user hopes data doesn't pass through page cache, but 9pfs in qemu ignore direct disk access and use host page cache, it is not match toDIRECT_IOsemantic, so we should not ignoreO_DIRECTin 9pfs unless v9fs in linux don't supportDIRECT_IO.Quoted 6 lines
And if server fs don't support
O_DIRECT, user will receive-EINVALand know the filesystem don't supportO_DIRECT.So in order to ensure semantic consistency, don't ignore direct disk access in 9pfs.
There are good reasons for us to ignore
O_DIRECT. AFAIK,O_DIRECTrequires the application to take care of the alignment of the buffers with either the logical block size of the filesystem (linux <= 2.4) or the logical block size of the underlying storage... I don't really see how you can achieve that since the linux 9p client only cares to break up requests into msize-chunks, and ignores alignment. ie, you're likely to not ensure semantic consistency on the host side anyway and linux will fallback to cached I/O.
Quoted 8 lines
Quoted 3 lines
Also, this change would silently break existing users of
O_DIRECT, which isn't acceptable... it would require some fsdev property to enable this new behavior.Thank you very much, I understand what your mean, v9fs in linux doesn't take care of the alignment, only split as the msize. Then there is another problem, should v9fs support
DIRECT_IO? or deleteDIRECT_IOops in v9fs?
The linux open(2) manual page says:
"O_DIRECT (since Linux 2.4.10)
Try to minimize cache effects of the I/O to and from this file."
ie, it doesn't ensure you necessarily skip any cache that may exist between the application and the ultimate storage. The manual page also elaborates how direct I/O behaves with NFS:
"The behavior of O_DIRECT with NFS will differ from local filesystems. Older
kernels, or kernels configured in certain ways, may not support this combination. The NFS protocol does not support passing the flag to the server, so O_DIRECT I/O will bypass the page cache only on the client; the server may still cache the I/O. The client asks the server to make the I/O synchronous to preserve the synchronous semantics of O_DIRECT. Some servers will perform poorly under these circumstances, especially if the I/O size is small. Some servers may also be configured to lie to clients about the I/O having reached stable storage; this will avoid the performance penalty at some risk to data integrity in the event of server power failure. The Linux NFS client places no alignment restrictions on O_DIRECT I/O."
So, Direct I/O in v9fs basically means the page cache is skipped in the client, which looks acceptable IMHO... And, again, removing this may break existing setups, so I'm not sure we should do that.
Cheers,
--
Greg
Quoted 28 lines
Thanks,
Yiwen.Quoted 24 lines
Quoted 10 lines
Signed-off-by Yiwen Jiang <[email protected]>--- hw/9pfs/9p.c | 4 ---- 1 file changed, 4 deletions(-)hw/9pfs/9p.c +0
-0hw/9pfs/9p.c static int get_dotl_openflags(V9fsState *s, int oflags) · line 160
Quoted 10 lines
hw/9pfs/9p.c +0
-4@@ -155,10 +155,6 @@ static int get_dotl_openflags(V9fsState *s, int oflags)*/ 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; }.