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

Use f-variants of chmod() and chown() #3479

Merged
merged 2 commits into from
Feb 16, 2023

Conversation

felixhandte
Copy link
Contributor

@felixhandte felixhandte commented Feb 6, 2023

These variants have lower overhead than their path-taking counterparts. This makes compressing the github dataset something like 10% faster, recovering much of the regression introduced in the 3 added syscalls of #3432.

Note that this requires making some changes to the flow of these syscalls. Previously, we close()-ed the file, then utime()-ed it, then ch{mod,own}()-ed it. However, an fd is only valid while the file is open, so in order to call fchmod() and fchown(), we need to move those calls to before we close() the file. While there is also a futimens(), we can't use it, since the utime() call has to come after we close() the file, so that we end up with the desired atime. (If we utime() then close(), the close() will override the atime we set.) So the new order is fch{mod,own}(), close(), utime(). This raises minor questions about whether this will now produce situations where we chown() the file away in such a way that we then don't have permissions to utime() it. But I think those concerns are unfounded. man utimensat says we need:

  1. the caller's effective user ID must match the owner of the file; or
  2. the caller must have appropriate privileges.

But we can only chown() the file away to a different uid if:

Only a privileged process (Linux: one with the CAP_CHOWN capability) may change the owner of a file.

So I think it's unlikely there are situations where we can chown() something away to a state where we can't then utime() it.

Somewhat surprisingly, calling `fchmod()` is non-trivially faster than calling
`chmod()`, and so on.

This commit introduces alternate variants to some common file util functions
that take an optional fd. If present, they call the `f`-variant of the
underlying function. Otherwise, they fall back to the regular filename-taking
version of the function.
Note that the `fd` is only valid while the file is still open. So we need to
move the setting calls to before we close the file. However! We cannot do so
with the `utime()` call (even though `futimens()` exists) because the follow-
ing `close()` call to the `fd` will reset the atime of the file. So it seems
the `utime()` call has to happen after the file is closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants