Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow files to be opened and created with an extra set of flags #45

Closed
wants to merge 1 commit into from

Conversation

glommer
Copy link

@glommer glommer commented Jul 29, 2020

When dealing with fast storage systems, it is not unheard of to want to
open files with O_DIRECT, O_DSYNC, or a combination thereof.

This patch extends the existing interface to allow such flags to be
passed to open and create.

I have considered, instead of allowing for generic flags, to just add
another "nocache" variant, which would communicate intent better.

However usually systems have corner cases in which small adjustments
to the flag set must be done to accomodate for file system support,
etc. So I have decided for the flags version instead.

Please let me know if this is acceptable.

To use it:

    let mut output = File::create_with_extra_flags(filename, libc::O_DIRECT)
	.await
	.unwrap();

When dealing with fast storage systems, it is not unheard of to want to
open files with O_DIRECT, O_DSYNC, or a combination thereof.

This patch extends the existing interface to allow such flags to be
passed to open and create.

I have considered, instead of allowing for generic flags, to just add
another "nocache" variant, which would communicate intent better.

However usually systems have corner cases in which small adjustments
to the flag set must be done to accomodate for file system support,
etc. So I have decided for the flags version instead.

Please let me know if this is acceptable.

To use it:

        let mut output = File::create_with_extra_flags(filename, libc::O_DIRECT)
		.await
		.unwrap();
@withoutboats
Copy link
Collaborator

I want this behavior to be implemented (and hadn't gotten around to it), but I'd like to support an API analogous to std. Rather than using the libc API, std exposes a type called OpenOptions. https://doc.rust-lang.org/std/fs/struct.OpenOptions.html

Unfortunately, we can't just re-export std's OpenOptions type, because there's no way to convert the std OpenOptions type to a c_int.

So you'd need to create an OpenOptions type with the same methods as std's OpenOptions. That includes implementing std::os::unix::fs::OpenOptionsExt, since this is a UNIX specific library: https://doc.rust-lang.org/std/os/unix/fs/trait.OpenOptionsExt.html

@glommer
Copy link
Author

glommer commented Jul 30, 2020

hi - I am fairly new to rust (coming from Linux Kernel and C++) so sorry if some of this doesn't make a lot of sense. As I ramp up and have been reading more the code, there are other problems that would prevent O_DIRECT from working properly as well. In particular, it seems that you are allocating your own buffers which are not aligned properly to O_DIRECT alignment requirements.

So I was thinking as an alternative that O_DIRECT could happen through its own File struct, let's say UnbufferedFile that would pass the O_DIRECT flag, maybe have a rustier way of communicating O_DSYNC requirements, and guaranteeing buffer alignment.

Would that make more sense ?

@withoutboats
Copy link
Collaborator

The longer term intention (see #34) is for the driver to be able to choose how buffers are allocated, in order to allow for kernel based buffer selection. For O_DIRECT specifically, it would only be safe to set that flag for files running a driver which allocates correctly aligned buffers. Possibly we could develop the APIs around this to make this more natural (for example, the poll_provide method added in #34 currently takes a buffer capacity, it could instead take a size and align layout to allow objects to request overaligned buffers for O_DIRECT purposes).

@glommer
Copy link
Author

glommer commented Jul 30, 2020

Thanks a lot for your time - really appreciated. My long term intention is to also register those buffers with io_uring_register, same as with the file descriptors, and allocate only from the region pre-registered. So I guess in the design of your crate that is something that gets pushed to the Driver abstraction, which I would implement in my application ?

@withoutboats
Copy link
Collaborator

withoutboats commented Jul 30, 2020

Yes, though the idea of ringbahn is that the driver abstraction can exist in a library, separated from the business logic of any particular application. The types in ringbahn provide the necessary links to safely abstract away the semantics of your particular driver.

For example, you could write a library that provides a driver that pre-registers fds and properly aligned buffers, and a wrapper around File<MyDriver> that is opened in O_DIRECT mode, and that would be totally separate from the specific logic of any application that wants to use files with those semantics.

A necessary step to actually support that API would be for File to support opening with other options than the default, so a PR which adds OpenOptions along the same lines as std would definitely be very welcome.

@glommer
Copy link
Author

glommer commented Aug 14, 2020

I want to sincerely thank you again for your time.

Upon investigating this further and learning more on what's out there I concluded that this is not the right level of abstraction for what I intend to do. So for now I am closing this PR.

@glommer glommer closed this Aug 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants