Support `direct_io`

[DUOLabs333]

diff --git a/src/fuse_ops.c b/src/fuse_ops.c
index 915ceaa..9d36a6d 100644
--- a/src/fuse_ops.c
+++ b/src/fuse_ops.c
@@ -109,7 +109,10 @@ static int unionfs_create(const char *path, mode_t mode, struct fuse_file_info *

        int i = find_rw_branch_cutlast(path);
        if (i == -1) RETURN(-errno);
-
+    
+    if (uopt.direct_io){
+       fi->direct_io=1;
+    }
        char p[PATHLEN_MAX];
        if (BUILD_PATH(p, uopt.branches[i].path, path)) RETURN(-ENAMETOOLONG);

@@ -399,6 +402,10 @@ static int unionfs_open(const char *path, struct fuse_file_info *fi) {
        DBG("%s\n", path);

        int i;
+
+       if (uopt.direct_io){
+          fi->direct_io=1;
+       }
        if (fi->flags & (O_WRONLY | O_RDWR)) {
                i = find_rw_branch_cutlast(path);
        } else {
diff --git a/src/opts.c b/src/opts.c
index f6c381d..2ba1bbe 100644
--- a/src/opts.c
+++ b/src/opts.c
@@ -362,6 +362,9 @@ int unionfs_opt_proc(void *data, const char *arg, int key, struct fuse_args *out
                case KEY_CHROOT:
                        uopt.chroot = get_opt_str(arg, "chroot");
                        return 0;
+               case KEY_DIRECT_IO:
+                     uopt.direct_io = 1;
+                     return 0;
                case KEY_COW:
                        uopt.cow_enabled = true;
                        return 0;
diff --git a/src/opts.h b/src/opts.h
index e1ce16c..b3bcacb 100644
--- a/src/opts.h
+++ b/src/opts.h
@@ -30,6 +30,7 @@ typedef struct {
        pthread_rwlock_t dbgpath_lock; // locks dbgpath
        bool hide_meta_files;
        bool relaxed_permissions;
+       bool direct_io;

 } uopt_t;

@@ -45,7 +46,8 @@ enum {
        KEY_NOINITGROUPS,
        KEY_RELAXED_PERMISSIONS,
        KEY_STATFS_OMIT_RO,
-       KEY_VERSION
+       KEY_VERSION,
+       KEY_DIRECT_IO
 };

diff --git a/src/unionfs.c b/src/unionfs.c
index 51684a7..482b32f 100644
--- a/src/unionfs.c
+++ b/src/unionfs.c
@@ -36,6 +36,7 @@ static struct fuse_opt unionfs_opts[] = {
        FUSE_OPT_KEY("statfs_omit_ro", KEY_STATFS_OMIT_RO),
        FUSE_OPT_KEY("--version", KEY_VERSION),
        FUSE_OPT_KEY("-V", KEY_VERSION),
+       FUSE_OPT_KEY("direct_io",KEY_DIRECT_IO),
        FUSE_OPT_END
 };

[bsbernd]

Create a pull request? Difficult to add comments here.

1) Help text is mission, see opts.c 2) No need for {}, when the if condition has some one line. In fact Radek always puts it into a single line 3) I would add a "," into the last line of the enum, to avoid two line changes when another option is added.

[rpodgorny]

@DUOLabs333 thanks for the patch! ...has it been tested? how?

@aakefbs thanks for your feedback!

[DUOLabs333]

@rpodgorny Yes, I'm using it in my own project.

[bsbernd]

Setting fi->direct_io in open() and create() looks good to me. (As explained in the libfuse issue), the other option is to set it in unionfs_init() with cfg->direct_io=1

[DUOLabs333]

@aakefbs I'll push my fork and make a pull request.

[DUOLabs333]

See PR #117

[DUOLabs333]

However, xbps-install fails with ERROR: cannot access to pkgdb: No such device and cannot internalize pkgdb dictionary: No such device

[rpodgorny]

sorry for not responding for a while, i have some catching up to do at work. anyway, i'll be back in action in about two weeks and i plan to merge this.

meanwhile, @DUOLabs333 can you please try to also add some tests? should be pretty simple (see already existing tests). something that tests for both the failing and "fixed" case? ...if not, no problem, i'll add them later myself. (i try to insist on having a test for each new feature, sorry. ;-) )

[bsbernd]

Well, what do you do with the now failing proc test, if suceeds with fixed kernel? direct-io is like splice and other things a pretty much kernel-module internal thing - there is no straight definition how it acts on it. The proposed use case is here in my opinion a bug on the page cache reading side - instead of using inode stat information after read fails, it does it before. That behavior is totally wrong in a network environment.

[rpodgorny]

Well, what do you do with the now failing proc test, if suceeds with fixed kernel? direct-io is like splice and other things a pretty much kernel-module internal thing - there is no straight definition how it acts on it. The proposed use case is here in my opinion a bug on the page cache reading side - instead of using inode stat information after read fails, it does it before. That behavior is totally wrong in a network environment.

yes, that's exactly why i want a counter-example. once your work gets merged to the kernel module and the "failing" tests starts to not fail, i'll be considering removing the workaround from unionfs. in other words, if there's a workaround for a kernel module bug that fixes things for someone (@DUOLabs333), i'm willing to merge it and remove later in hopes of widening the unionfs' user base.

...or did i get the whole thing wrong?

anyway, the direct_io is a fuse-supported option and i think we should just support it.

[bsbernd]

I wouldn't remove this option when the test starts to succeed. Some people might just want to use it for something like reducing memory usage - like embedded system short of memory? To be honest, I don't get why the -odirect_io option was removed from libfuse in the first place. The commit message said that it removed file system specific settings, but in the end these were mostly fuse-kernel-module internal settings (splice was also in there, which we should not enabled in unionfs - another option I wouldn't like write a unit test for - it 'just' speeds up large IO). Funny part is that libfuse-3 actually enabled read-splice by default and slows down small IO (<16kb) quite a bit with that, while write-splice doesn't have side effects to my knowledge.

[bsbernd]

@rpodgorny any reason not to just merge this in?

[rpodgorny]

@rpodgorny any reason not to just merge this in?

...i was just hoping for some tests to be made to actually verify it does something... ...but i'll merge this next week, anyway, even without the tests...

[bsbernd]

This is a pure setting for the kernel - except of bugs (and unsupported features (mmap MAP_SHARED), it does not make a difference. And so testing might easily give different results. What you actually want to have is a fuse ioctl to get the activated configuration - it does not exists. I still find it very debatable that the direct-io option was removed from libfuse3.

[rpodgorny]

so, i've just created the "directio" branch with the proposed changes. @aakefbs @DUOLabs333 feel free to comment/test - i'll merge it in few days...

[rpodgorny]

i've just released v3.3 with this included. thank you for your contribution and sorry for the long wait!