Can't set direct_io for open files

[ncw]

Can't set direct_io for open files

[billziss-gh]

@ncw thanks. I am closing this and will follow up with a new release shortly.

[billziss-gh]

direct_io on cgofuse

This should be easy to fix. We should keep it backwards compatible or bump up the version to 2.0. (Bumping it to 2.0 would also allow us to fix ugliness such as FileSystemChflags, etc.)

direct_io on WinFsp

Things are complicated:

• In WinFsp v1.0 there was no way to disable caching for individual files. Either all files in the file system had data cache enabled (-o FileInfoTimeout==-1) or all files in the file system had cache disabled. Because of this the WinFsp-FUSE layer (which was written at the same time) ignores the direct_io bit and does not pass it to the FSD. Later the FSD added support for turning off the cache for individual files, but the WinFsp-FUSE layer was never updated.

  • This would be an easy fix.

• More importantly the WinFsp FSD was designed with the assumption that it knows the true size of a file when an I/O is completed by the user mode file system. The primary reason for this requirement was to properly support caching and memory mapped files (-- even when caching is disabled, a file may be memory mapped, which requires the FSD and the kernel to know the file size to properly setup the mapping). So unfortunately what you want to do is probably not straightforward.

  • This is a much harder thing to change. There are many places where the FSD assumes that the file size is properly calculated.

[ncw]

Do you think just doing the first part direct_io on cgofuse would be an improvement to the status quo?

That would help users of cgofuse on non Windows platforms.

[billziss-gh]

I am happy to add direct_io support as long as it is understood that it will not work as intended on WinFsp.

What do you propose the interface for it to be? I would think a new OpenEx or Open2 call that allows the file system to return the full fuse_file_info structure somehow.

[ncw]

I am happy to add direct_io support as long as it is understood that it will not work as intended on WinFsp.

Great!

What do you propose the interface for it to be? I would think a new OpenEx or Open2 call that allows the file system to return the full fuse_file_info structure somehow.

The current prototype is

Open(path string, flags int) (errc int, fh uint64)

I'd propose something like

OpenEx(path string, flags int) (errc int, fh uint64, openFlags uint32)

Where openFlags is a sanitized version of the flags from fuse_file_info

These are the only ones bazil fuse supports. Not sure what the OS X ones do!

	OpenDirectIO    OpenResponseFlags = 1 << 0 // bypass page cache for this open file
	OpenKeepCache   OpenResponseFlags = 1 << 1 // don't invalidate the data cache on open
	OpenNonSeekable OpenResponseFlags = 1 << 2 // mark the file as non-seekable (not supported on OS X)

	OpenPurgeAttr OpenResponseFlags = 1 << 30 // OS X
	OpenPurgeUBC  OpenResponseFlags = 1 << 31 // OS X

[billziss-gh]

IMO if we introduce OpenEx we should support the full native fuse_file_info to anticipate future requirements.

So I propose that the OpenEx method has a prototype as below (assuming a suitably defined FileInfo struct). The file system implementation is expected to fill the FileInfo struct (similar to what is being done in GetAttr for Stat_t).

/*
 * Go
 */

type FileInfo struct {
    // ...
}

// ...

OpenEx(path string, flags int, fi *FileInfo) int

For reference here are the C definitions for FUSE2 and FUSE3:

/*
 * C
 */

// FUSE2 fuse_file_info
struct fuse_file_info
{
    int flags;
    unsigned int fh_old;
    int writepage;
    unsigned int direct_io:1;
    unsigned int keep_cache:1;
    unsigned int flush:1;
    unsigned int nonseekable:1;
    unsigned int padding:28;
    uint64_t fh;
    uint64_t lock_owner;
};

// FUSE3 fuse_file_info
struct fuse3_file_info
{
    int flags;
    unsigned int writepage:1;
    unsigned int direct_io:1;
    unsigned int keep_cache:1;
    unsigned int flush:1;
    unsigned int nonseekable:1;
    unsigned int flock_release:1;
    unsigned int padding:27;
    uint64_t fh;
    uint64_t lock_owner;
    uint32_t poll_events;
};

---

What timeframe would you need this or a similar change in?

[ncw]

IMO if we introduce OpenEx we should support the full native fuse_file_info to anticipate future requirements.

That looks like it is future proof :-)

What timeframe would you need this or a similar change in?

I haven't got a pressing need for this if Windows can't support direct_io, it would just enable the two different mounting systems to achieve parity in rclone.

This should be easy to fix. We should keep it backwards compatible or bump up the version to 2.0. (Bumping it to 2.0 would also allow us to fix ugliness such as FileSystemChflags, etc.)

Note that there be dragons with go modules and going from v1 to v2 if you haven't seen that already.

[billziss-gh]

I have a couple of fires to fight right now, plus my regular development of winfuse. If I have not done this in a week or 2, please ping me!

[billziss-gh]

My fires ended up fighting themselves for now :) So I found some time to get this done. See commit: 682213d. Feedback welcome.

Of course this raises the question of whether we should support FileInfo_t on Read, Write, etc. as is done in native FUSE with struct fuse_file_info. I do not think so, but wanted to raise this nevertheless.

EDIT 1: the OpendirEx method may be superfluous.

EDIT 2: here is an alternative commit without OpendirEx: 8e41a0c

[billziss-gh]

Ping @ncw

[ncw]

My fires ended up fighting themselves for now :)

The best kind of fires!

Apologies for my own delay responding.

So I found some time to get this done. See commit: 682213d. Feedback welcome.

Of course this raises the question of whether we should support FileInfo_t on Read, Write, etc. as is done in native FUSE with struct fuse_file_info. I do not think so, but wanted to raise this nevertheless.

Looking at the fuse docs almost none of those flags apply to read and write. Those that do, do not look useful to me so I'd agree that not supporting FileInfo_t on anything other than Open is probably sufficient.

EDIT 1: the OpendirEx method may be superfluous.

The only flag that opendir uses is cache_readdir which

signals the kernel to enable caching of entries returned by readdir().

I think that for fuse file systems the global control over direntry caching is surely enough.

EDIT 2: here is an alternative commit without OpendirEx: 8e41a0c

I think I prefer that. Narrowing the interface is good.

In fact we might consider narrowing the interface further to this which would have the advantage that the user need only implement one or the other. This appears more go style to me.

type FileSystemCreateEx interface {
 	CreateEx(path string, mode uint32, fi *FileInfo_t) int
 }

type FileSystemOpenEx interface {
 	OpenEx(path string, fi *FileInfo_t) int
 }

[billziss-gh]

In fact we might consider narrowing the interface further to this which would have the advantage
that the user need only implement one or the other. This appears more go style to me.

type FileSystemCreateEx interface {
 	CreateEx(path string, mode uint32, fi *FileInfo_t) int
 }

type FileSystemOpenEx interface {
 	OpenEx(path string, fi *FileInfo_t) int
 }

That was my first attempt, but then I run into the implementation of hostCreate:

func hostCreate(path0 *c_char, mode0 c_fuse_mode_t, fi0 *c_struct_fuse_file_info) (errc0 c_int) {
	defer recoverAsErrno(&errc0)
	fsop := hostHandleGet(c_fuse_get_context().private_data).fsop
	path := c_GoString(path0)
	intf, ok := fsop.(FileSystemOpenEx)
	if ok {
		fi := FileInfo_t{Flags: int(fi0.flags)}
		errc := intf.CreateEx(path, uint32(mode0), &fi)
		if -ENOSYS == errc {
			errc = fsop.Mknod(path, S_IFREG|uint32(mode0), 0)
			if 0 == errc {
				errc = intf.OpenEx(path, &fi)
			}
		}
		c_hostAsgnCfileinfo(fi0,
			c_bool(fi.DirectIo),
			c_bool(fi.KeepCache),
			c_bool(fi.NonSeekable),
			c_uint64_t(fi.Fh))
		return c_int(errc)
	} else {
		errc, rslt := fsop.Create(path, int(fi0.flags), uint32(mode0))
		if -ENOSYS == errc {
			errc = fsop.Mknod(path, S_IFREG|uint32(mode0), 0)
			if 0 == errc {
				errc, rslt = fsop.Open(path, int(fi0.flags))
			}
		}
		fi0.fh = c_uint64_t(rslt)
		return c_int(errc)
	}
}

If we have 2 separate interfaces then the implementation becomes more complicated. Of course this is not the real problem, but it made me realize that a file system that supports OpenEx should probably also support CreateEx. The semantics of Create is to create a new file and Open it. Likewise the semantics of CreateEx are to create a new file and OpenEx it.

[ncw]

That was my first attempt, but then I run into the implementation of hostCreate:

If we have 2 separate interfaces then the implementation becomes more complicated. Of course this is not the real problem, but it made me realize that a file system that supports OpenEx should probably also support CreateEx. The semantics of Create is to create a new file and Open it. Likewise the semantics of CreateEx are to create a new file and OpenEx it.

That is a good argument that they should be in the same interface.

OK so EDIT 2 for the win?

[billziss-gh]

I have merged commit 8e41a0c ("EDIT 2") into master. Please pull and see if it works for you.

[billziss-gh]

@ncw let me know if you are happy with these changes so we can close this.

[s3rj1k]

@billziss-gh Can we have a simple example in how to use direct_io ?

[ncw]

@ncw let me know if you are happy with these changes so we can close this.

Sorry for the long delay!

I've implemented this now and it works very well thank you @billziss-gh

To answer @s3rj1k 's question - you'll need to implement the optional OpenEx and CreateEx methods. If you do that you can set direct io in them. Here is a snippet from rclone code

func (fsys *FS) OpenEx(path string, fi *fuse.FileInfo_t) (errc int) {
	defer log.Trace(path, "flags=0x%X", fi.Flags)("errc=%d, fh=0x%X", &errc, &fi.Fh)
	fi.Fh = fhUnset

	// translate the fuse flags to os flags
	flags := translateOpenFlags(fi.Flags)
	handle, err := fsys.VFS.OpenFile(path, flags, 0777)
	if err != nil {
		return translateError(err)
	}

	// If size unknown then use direct io to read
	if entry := handle.Node().DirEntry(); entry != nil && entry.Size() < 0 {
		fi.DirectIo = true
	}

	fi.Fh = fsys.openHandle(handle)
	return 0
}