file: Skip alignment probes for buffered read-only files - #5
Conversation
There was a problem hiding this comment.
I don’t really follow what the first commit does. (Edit: Sorry, wrote this before I did follow through it. I do follow through it, and it looks good to me, but more explanation would still be nice.)
The commit message is quite sparse, it would be good if it could say more about how it is accomplishing this, and also, why this is safe and good. Also, why do we need to do probing when the file is buffered and R/W? (I know I write the code, but I don’t remember 🙃 — maybe I was trying to be rather safe than sorry.)
| /// | ||
| /// `direct_io` should be `true` if direct I/O was requested, and can be `false` if that status | ||
| /// is unknown. | ||
| fn new( |
There was a problem hiding this comment.
This should be either just rephrased, or, if indeed dropped, the /// line above must go as well.
| /// Detect access and direct I/O modes for an externally supplied file. | ||
| #[cfg(unix)] | ||
| fn detect_open_mode(file: &fs::File) -> (bool, bool) { | ||
| // Safe: No argument, returns result. |
There was a problem hiding this comment.
Can you add documentation on the return values?
ce74ae5 to
cffe911
Compare
|
Hmm, there’s still no notes in the commit message, and is there a reason you dropped the second patch? It seemed good to me. |
Buffered I/O does not impose alignment restrictions, so probing it only adds fstatfs(), pread(), and pwrite() calls. The pwrite() probe can also grow an empty writable file. Use byte alignment for buffered files and keep the existing probes for O_DIRECT. Read-only O_DIRECT files still skip the write probe. Signed-off-by: ChengyuZhu6 <hudson@cyzhu.com>
Signed-off-by: ChengyuZhu6 <hudson@cyzhu.com>
cffe911 to
e46b133
Compare
@XanClic My bad. Now buffered io now uses byte alignment regardless of whether the file is writable. This also avoids the pwrite probe growing an empty writable file. ro |
2 participants
No description provided.