Re: [PULL 08/28] fuse: Set direct_io and parallel_direct_writes
On 05.05.26 13:01, Fiona Ebner wrote: Am 05.05.26 um 11:03 AM schrieb Fiona Ebner: Am 30.04.26 um 4:20 PM schrieb Fiona Ebner: Dear Maintainers, Am 10.03.26 um 6:33 PM schrieb Kevin Wolf: From: Hanna Czenczek In fuse_open(), set these flags: - direct_io: We probably actually don't want to have the host page cache be used for our exports. QEMU block exports are supposed to represent the image as-is (and thus potentially changing). This causes a change in iotest 308's reference output. - parallel_direct_writes: We can (now) cope with parallel writes, so we should set this flag. For some reason, it doesn't seem to make an actual performance difference with libfuse, but it does make a difference without it, so let's set it. (See "fuse: Copy write buffer content before polling" for further discussion.) Reviewed-by: Stefan Hajnoczi Signed-off-by: Hanna Czenczek Message-ID: <[email protected]> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/export/fuse.c| 2 ++ tests/qemu-iotests/308.out | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/block/export/fuse.c b/block/export/fuse.c index 0422cf4b8af..d0e3c6bf61f 100644 --- a/block/export/fuse.c +++ b/block/export/fuse.c @@ -582,6 +582,8 @@ static void fuse_setattr(fuse_req_t req, fuse_ino_t inode, struct stat *statbuf, static void fuse_open(fuse_req_t req, fuse_ino_t inode, struct fuse_file_info *fi) { +fi->direct_io = true; unfortunately, turning on direct IO here breaks a use case with swtpm. Reproducer below [0], which fails with an error message /usr/bin/swtpm exit with status 256: after this commit. Dropping the FOPEN_DIRECT_IO flag in current master makes the reproducer work again. Should there be a BlockExportOptionsFuse property to control the direct IO flag and have it be opt-in to avoid this breakage (and potentially others)? Best Regards, Fiona [0]: #!/bin/bash rm /tmp/disk.qcow2 rm /tmp/export.fuse ./qemu-img create -f qcow2 /tmp/disk.qcow2 4M touch /tmp/export.fuse ( ./storage-daemon/qemu-storage-daemon \ --blockdev qcow2,node-name=node0,file.driver=file,file.filename=/tmp/disk.qcow2 \ --export fuse,id=exp0,mountpoint=/tmp/export.fuse,node-name=node0,writable=true \ --chardev socket,id=qmp,path=/run/qsd.qmp,server=on,wait=off \ --monitor chardev=qmp,mode=control ) & sleep 1 # too lazy to do proper synchronization for the reproducer here swtpm_setup --tpmstate file:///tmp/export.fuse --createek echo '{"execute": "qmp_capabilities"}{"execute": "quit"}' | socat - /run/qsd.qmp Self-contained version modeling the relevant part of swtpm_setup: #!/bin/bash cat - > /tmp/mmap.c < #include #include #include #include int main(int argc, char **argv) { struct stat st; int fd; void *ptr; assert(argc == 2); fd = open(argv[1], O_RDWR|O_CREAT, 0x640); assert(fd > 0); fstat(fd, &st); ptr = mmap(NULL, st.st_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); assert(ptr != MAP_FAILED); return 0; } EOF gcc /tmp/mmap.c -o /tmp/mmap rm /tmp/disk.qcow2 rm /tmp/export.fuse ./qemu-img create -f qcow2 /tmp/disk.qcow2 4M touch /tmp/export.fuse ( ./storage-daemon/qemu-storage-daemon \ --blockdev qcow2,node-name=node0,file.driver=file,file.filename=/tmp/disk.qcow2 \ --export fuse,id=exp0,mountpoint=/tmp/export.fuse,node-name=node0,writable=true \ --chardev socket,id=qmp,path=/run/qsd.qmp,server=on,wait=off \ --monitor chardev=qmp,mode=control ) & sleep 1 # too lazy to do proper synchronization for the reproducer here /tmp/mmap /tmp/export.fuse echo '{"execute": "qmp_capabilities"}{"execute": "quit"}' | socat - /run/qsd.qmp The kernel docs [0] mention: "In direct-io mode the page cache is completely bypassed for reads and writes. No read-ahead takes place. Shared mmap is disabled by default. To allow shared mmap, the FUSE_DIRECT_IO_ALLOW_MMAP flag may be enabled in the FUSE_INIT reply." I quickly tested with setting FUSE_DIRECT_IO_ALLOW_MMAP in fuse_co_init() (it's part of the flags2 ones, so also requires FUSE_INIT_EXT to be set in the flags) and this makes the swtpm use case work again :) So, should that flag be set by default (if supported by the kernel) for backwards compatibility? Or what should we do about the issue? I see no reason not to set the flag (FUSE_DIRECT_IO_ALLOW_MMAP). Sounds very good to me! Hanna
Re: [PULL 08/28] fuse: Set direct_io and parallel_direct_writes
Am 05.05.26 um 11:03 AM schrieb Fiona Ebner: > Am 30.04.26 um 4:20 PM schrieb Fiona Ebner: >> Dear Maintainers, >> >> Am 10.03.26 um 6:33 PM schrieb Kevin Wolf: >>> From: Hanna Czenczek >>> >>> In fuse_open(), set these flags: >>> - direct_io: We probably actually don't want to have the host page cache >>> be used for our exports. QEMU block exports are supposed to represent >>> the image as-is (and thus potentially changing). >>> This causes a change in iotest 308's reference output. >>> >>> - parallel_direct_writes: We can (now) cope with parallel writes, so we >>> should set this flag. For some reason, it doesn't seem to make an >>> actual performance difference with libfuse, but it does make a >>> difference without it, so let's set it. >>> (See "fuse: Copy write buffer content before polling" for further >>> discussion.) >>> >>> Reviewed-by: Stefan Hajnoczi >>> Signed-off-by: Hanna Czenczek >>> Message-ID: <[email protected]> >>> Reviewed-by: Kevin Wolf >>> Signed-off-by: Kevin Wolf >>> --- >>> block/export/fuse.c| 2 ++ >>> tests/qemu-iotests/308.out | 2 +- >>> 2 files changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/block/export/fuse.c b/block/export/fuse.c >>> index 0422cf4b8af..d0e3c6bf61f 100644 >>> --- a/block/export/fuse.c >>> +++ b/block/export/fuse.c >>> @@ -582,6 +582,8 @@ static void fuse_setattr(fuse_req_t req, fuse_ino_t >>> inode, struct stat *statbuf, >>> static void fuse_open(fuse_req_t req, fuse_ino_t inode, >>>struct fuse_file_info *fi) >>> { >>> +fi->direct_io = true; >> >> unfortunately, turning on direct IO here breaks a use case with swtpm. >> Reproducer below [0], which fails with an error message >> /usr/bin/swtpm exit with status 256: >> after this commit. >> >> Dropping the FOPEN_DIRECT_IO flag in current master makes the reproducer >> work again. >> >> Should there be a BlockExportOptionsFuse property to control the direct >> IO flag and have it be opt-in to avoid this breakage (and potentially >> others)? >> >> Best Regards, >> Fiona >> >> [0]: >> >>> #!/bin/bash >>> rm /tmp/disk.qcow2 >>> rm /tmp/export.fuse >>> ./qemu-img create -f qcow2 /tmp/disk.qcow2 4M >>> touch /tmp/export.fuse >>> ( >>> ./storage-daemon/qemu-storage-daemon \ >>> --blockdev >>> qcow2,node-name=node0,file.driver=file,file.filename=/tmp/disk.qcow2 \ >>> --export >>> fuse,id=exp0,mountpoint=/tmp/export.fuse,node-name=node0,writable=true \ >>> --chardev socket,id=qmp,path=/run/qsd.qmp,server=on,wait=off \ >>> --monitor chardev=qmp,mode=control >>> ) & >>> sleep 1 # too lazy to do proper synchronization for the reproducer here >>> swtpm_setup --tpmstate file:///tmp/export.fuse --createek >>> echo '{"execute": "qmp_capabilities"}{"execute": "quit"}' | socat - >>> /run/qsd.qmp > > Self-contained version modeling the relevant part of swtpm_setup: > >> #!/bin/bash >> >> cat - > /tmp/mmap.c <> #include >> #include >> #include >> #include >> #include >> >> int main(int argc, char **argv) >> { >> struct stat st; >> int fd; >> void *ptr; >> >> assert(argc == 2); >> fd = open(argv[1], O_RDWR|O_CREAT, 0x640); >> assert(fd > 0); >> fstat(fd, &st); >> ptr = mmap(NULL, st.st_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); >> assert(ptr != MAP_FAILED); >> return 0; >> } >> EOF >> gcc /tmp/mmap.c -o /tmp/mmap >> >> rm /tmp/disk.qcow2 >> rm /tmp/export.fuse >> ./qemu-img create -f qcow2 /tmp/disk.qcow2 4M >> touch /tmp/export.fuse >> >> ( >> ./storage-daemon/qemu-storage-daemon \ >> --blockdev >> qcow2,node-name=node0,file.driver=file,file.filename=/tmp/disk.qcow2 \ >> --export >> fuse,id=exp0,mountpoint=/tmp/export.fuse,node-name=node0,writable=true \ >> --chardev socket,id=qmp,path=/run/qsd.qmp,server=on,wait=off \ >> --monitor chardev=qmp,mode=control >> ) & >> >> sleep 1 # too lazy to do proper synchronization for the reproducer here >> /tmp/mmap /tmp/export.fuse >> echo '{"execute": "qmp_capabilities"}{"execute": "quit"}' | socat - >> /run/qsd.qmp The kernel docs [0] mention: "In direct-io mode the page cache is completely bypassed for reads and writes. No read-ahead takes place. Shared mmap is disabled by default. To allow shared mmap, the FUSE_DIRECT_IO_ALLOW_MMAP flag may be enabled in the FUSE_INIT reply." I quickly tested with setting FUSE_DIRECT_IO_ALLOW_MMAP in fuse_co_init() (it's part of the flags2 ones, so also requires FUSE_INIT_EXT to be set in the flags) and this makes the swtpm use case work again :) So, should that flag be set by default (if supported by the kernel) for backwards compatibility? Or what should we do about the issue? Best Regards, Fiona [0]: https://www.kernel.org/doc/html/next/filesystems/fuse/fuse-io.html
Re: [PULL 08/28] fuse: Set direct_io and parallel_direct_writes
Am 30.04.26 um 4:20 PM schrieb Fiona Ebner: > Dear Maintainers, > > Am 10.03.26 um 6:33 PM schrieb Kevin Wolf: >> From: Hanna Czenczek >> >> In fuse_open(), set these flags: >> - direct_io: We probably actually don't want to have the host page cache >> be used for our exports. QEMU block exports are supposed to represent >> the image as-is (and thus potentially changing). >> This causes a change in iotest 308's reference output. >> >> - parallel_direct_writes: We can (now) cope with parallel writes, so we >> should set this flag. For some reason, it doesn't seem to make an >> actual performance difference with libfuse, but it does make a >> difference without it, so let's set it. >> (See "fuse: Copy write buffer content before polling" for further >> discussion.) >> >> Reviewed-by: Stefan Hajnoczi >> Signed-off-by: Hanna Czenczek >> Message-ID: <[email protected]> >> Reviewed-by: Kevin Wolf >> Signed-off-by: Kevin Wolf >> --- >> block/export/fuse.c| 2 ++ >> tests/qemu-iotests/308.out | 2 +- >> 2 files changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/block/export/fuse.c b/block/export/fuse.c >> index 0422cf4b8af..d0e3c6bf61f 100644 >> --- a/block/export/fuse.c >> +++ b/block/export/fuse.c >> @@ -582,6 +582,8 @@ static void fuse_setattr(fuse_req_t req, fuse_ino_t >> inode, struct stat *statbuf, >> static void fuse_open(fuse_req_t req, fuse_ino_t inode, >>struct fuse_file_info *fi) >> { >> +fi->direct_io = true; > > unfortunately, turning on direct IO here breaks a use case with swtpm. > Reproducer below [0], which fails with an error message > /usr/bin/swtpm exit with status 256: > after this commit. > > Dropping the FOPEN_DIRECT_IO flag in current master makes the reproducer > work again. > > Should there be a BlockExportOptionsFuse property to control the direct > IO flag and have it be opt-in to avoid this breakage (and potentially > others)? > > Best Regards, > Fiona > > [0]: > >> #!/bin/bash >> rm /tmp/disk.qcow2 >> rm /tmp/export.fuse >> ./qemu-img create -f qcow2 /tmp/disk.qcow2 4M >> touch /tmp/export.fuse >> ( >> ./storage-daemon/qemu-storage-daemon \ >> --blockdev >> qcow2,node-name=node0,file.driver=file,file.filename=/tmp/disk.qcow2 \ >> --export >> fuse,id=exp0,mountpoint=/tmp/export.fuse,node-name=node0,writable=true \ >> --chardev socket,id=qmp,path=/run/qsd.qmp,server=on,wait=off \ >> --monitor chardev=qmp,mode=control >> ) & >> sleep 1 # too lazy to do proper synchronization for the reproducer here >> swtpm_setup --tpmstate file:///tmp/export.fuse --createek >> echo '{"execute": "qmp_capabilities"}{"execute": "quit"}' | socat - >> /run/qsd.qmp Self-contained version modeling the relevant part of swtpm_setup: > #!/bin/bash > > cat - > /tmp/mmap.c < #include > #include > #include > #include > #include > > int main(int argc, char **argv) > { > struct stat st; > int fd; > void *ptr; > > assert(argc == 2); > fd = open(argv[1], O_RDWR|O_CREAT, 0x640); > assert(fd > 0); > fstat(fd, &st); > ptr = mmap(NULL, st.st_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); > assert(ptr != MAP_FAILED); > return 0; > } > EOF > gcc /tmp/mmap.c -o /tmp/mmap > > rm /tmp/disk.qcow2 > rm /tmp/export.fuse > ./qemu-img create -f qcow2 /tmp/disk.qcow2 4M > touch /tmp/export.fuse > > ( > ./storage-daemon/qemu-storage-daemon \ > --blockdev > qcow2,node-name=node0,file.driver=file,file.filename=/tmp/disk.qcow2 \ > --export > fuse,id=exp0,mountpoint=/tmp/export.fuse,node-name=node0,writable=true \ > --chardev socket,id=qmp,path=/run/qsd.qmp,server=on,wait=off \ > --monitor chardev=qmp,mode=control > ) & > > sleep 1 # too lazy to do proper synchronization for the reproducer here > /tmp/mmap /tmp/export.fuse > echo '{"execute": "qmp_capabilities"}{"execute": "quit"}' | socat - > /run/qsd.qmp Best Regards, Fiona
Re: [PULL 08/28] fuse: Set direct_io and parallel_direct_writes
Dear Maintainers, Am 10.03.26 um 6:33 PM schrieb Kevin Wolf: > From: Hanna Czenczek > > In fuse_open(), set these flags: > - direct_io: We probably actually don't want to have the host page cache > be used for our exports. QEMU block exports are supposed to represent > the image as-is (and thus potentially changing). > This causes a change in iotest 308's reference output. > > - parallel_direct_writes: We can (now) cope with parallel writes, so we > should set this flag. For some reason, it doesn't seem to make an > actual performance difference with libfuse, but it does make a > difference without it, so let's set it. > (See "fuse: Copy write buffer content before polling" for further > discussion.) > > Reviewed-by: Stefan Hajnoczi > Signed-off-by: Hanna Czenczek > Message-ID: <[email protected]> > Reviewed-by: Kevin Wolf > Signed-off-by: Kevin Wolf > --- > block/export/fuse.c| 2 ++ > tests/qemu-iotests/308.out | 2 +- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/block/export/fuse.c b/block/export/fuse.c > index 0422cf4b8af..d0e3c6bf61f 100644 > --- a/block/export/fuse.c > +++ b/block/export/fuse.c > @@ -582,6 +582,8 @@ static void fuse_setattr(fuse_req_t req, fuse_ino_t > inode, struct stat *statbuf, > static void fuse_open(fuse_req_t req, fuse_ino_t inode, >struct fuse_file_info *fi) > { > +fi->direct_io = true; unfortunately, turning on direct IO here breaks a use case with swtpm. Reproducer below [0], which fails with an error message /usr/bin/swtpm exit with status 256: after this commit. Dropping the FOPEN_DIRECT_IO flag in current master makes the reproducer work again. Should there be a BlockExportOptionsFuse property to control the direct IO flag and have it be opt-in to avoid this breakage (and potentially others)? Best Regards, Fiona [0]: > #!/bin/bash > rm /tmp/disk.qcow2 > rm /tmp/export.fuse > ./qemu-img create -f qcow2 /tmp/disk.qcow2 4M > touch /tmp/export.fuse > ( > ./storage-daemon/qemu-storage-daemon \ > --blockdev > qcow2,node-name=node0,file.driver=file,file.filename=/tmp/disk.qcow2 \ > --export > fuse,id=exp0,mountpoint=/tmp/export.fuse,node-name=node0,writable=true \ > --chardev socket,id=qmp,path=/run/qsd.qmp,server=on,wait=off \ > --monitor chardev=qmp,mode=control > ) & > sleep 1 # too lazy to do proper synchronization for the reproducer here > swtpm_setup --tpmstate file:///tmp/export.fuse --createek > echo '{"execute": "qmp_capabilities"}{"execute": "quit"}' | socat - > /run/qsd.qmp
[PULL 08/28] fuse: Set direct_io and parallel_direct_writes
From: Hanna Czenczek In fuse_open(), set these flags: - direct_io: We probably actually don't want to have the host page cache be used for our exports. QEMU block exports are supposed to represent the image as-is (and thus potentially changing). This causes a change in iotest 308's reference output. - parallel_direct_writes: We can (now) cope with parallel writes, so we should set this flag. For some reason, it doesn't seem to make an actual performance difference with libfuse, but it does make a difference without it, so let's set it. (See "fuse: Copy write buffer content before polling" for further discussion.) Reviewed-by: Stefan Hajnoczi Signed-off-by: Hanna Czenczek Message-ID: <[email protected]> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/export/fuse.c| 2 ++ tests/qemu-iotests/308.out | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/block/export/fuse.c b/block/export/fuse.c index 0422cf4b8af..d0e3c6bf61f 100644 --- a/block/export/fuse.c +++ b/block/export/fuse.c @@ -582,6 +582,8 @@ static void fuse_setattr(fuse_req_t req, fuse_ino_t inode, struct stat *statbuf, static void fuse_open(fuse_req_t req, fuse_ino_t inode, struct fuse_file_info *fi) { +fi->direct_io = true; +fi->parallel_direct_writes = true; fuse_reply_open(req, fi); } diff --git a/tests/qemu-iotests/308.out b/tests/qemu-iotests/308.out index aa96faab6d0..2d7a38d63d2 100644 --- a/tests/qemu-iotests/308.out +++ b/tests/qemu-iotests/308.out @@ -131,7 +131,7 @@ wrote 65536/65536 bytes at offset 1048576 --- Try growing non-growable export --- (OK: Lengths of export and original are the same) -dd: error writing 'TEST_DIR/t.IMGFMT.fuse': Input/output error +dd: error writing 'TEST_DIR/t.IMGFMT.fuse': No space left on device 1+0 records in 0+0 records out -- 2.53.0
