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

Implement relatime. #2064

Closed
wants to merge 1 commit into from
Closed

Implement relatime. #2064

wants to merge 1 commit into from

Conversation

dweeezil
Copy link
Contributor

Add the "relatime" property. When set to "on", a file's atime will only
be updated if the existing atime at least a day old or if the existing
ctime or mtime has been updated since the last access. This behavior
is compatible with the Linux "relatime" mount option.

@dweeezil
Copy link
Contributor Author

A few notes regarding this implementation are in order:

I used a new property, "relatime" rather than overloading the existing "atime" property in order to keep the best compatibility with other ZFS implementations. In the same vein, the default value for relatime is "off".

The zfs_tstamp_update_setup() function in which the actual work is performed as bee restructured a bit based on its existing use. In particular, a new ASSERT() has been added to enforce that it's never called with both the ATTR_ATIME flag and the have_tx flag arguments set. This allows for a more straightforward implementation of the relatime logic.

There are some style issues to fix.

@dweeezil
Copy link
Contributor Author

Fixed style issues.

@behlendorf
Copy link
Contributor

@dweeezil Thanks for jumping on this! It'll be great to get this in the next tag, particularly since it's such a straight forward improvement. Adding another property to set the relatime behavior is fine, but we're going to need to add the new property to the zfs.8 man page. When you refresh this patch based on the inline comments please add a hunk to the Properties section describing this new property and it's expected behavior.

In the meanwhile, I'll run it through automated testing since it all looks functionally correct.

@dweeezil
Copy link
Contributor Author

Updated as requested but not yet (re)tested. I'll give it a run-through this evening.

@behlendorf
Copy link
Contributor

@dweeezil That was fast. I'll test this as well once your happy with it.

Add the "relatime" property.  When set to "on", a file's atime will only
be updated if the existing atime at least a day old or if the existing
ctime or mtime has been updated since the last access.  This behavior
is compatible with the Linux "relatime" mount option.
@dweeezil
Copy link
Contributor Author

@behlendorf I finally got a chance to test the latest commit (dweeezil/zfs@6d265b9) and it seems to work just fine.

FransUrbo pushed a commit to FransUrbo/zfs that referenced this pull request Feb 5, 2014
Add the "relatime" property.  When set to "on", a file's atime will only
be updated if the existing atime at least a day old or if the existing
ctime or mtime has been updated since the last access.  This behavior
is compatible with the Linux "relatime" mount option.

Signed-off-by: Tim Chase <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#2064
Closes openzfs#1917
@spacelama
Copy link

"relatime defaults to off". What am I doing wrong?

sudo zfs get -r all tank3 | grep atime
tank3 atime on default
tank3 relatime on temporary
tank3/.mp3.low_qual atime on default
tank3/.mp3.low_qual relatime on temporary
...
sudo zfs set relatime=off tank3/.mp3.low_qual
0-0-22:13:19, Sun Apr 13 tconnors@fs:~ (bash)
sudo zfs get -r all tank3 | grep atime
tank3 atime on default
tank3 relatime on temporary
tank3/.mp3.low_qual atime on default
tank3/.mp3.low_qual relatime on temporary
...
What's "temporary"? I didn't do anything to change it. /proc/mounts shows relatime option, but I'm not using legacy mounts:
grep mp3.low.*relatime /proc/mounts
tank3/.mp3.low_qual /home/tconnors/.mp3.low_qual zfs rw,relatime,xattr,noacl 0 0

@dweeezil
Copy link
Contributor Author

@spacelama Properties which correspond to a mount option such as "atime", "relatime" and "devices" will show as "temporary" when their ZFS property doesn't match the current value in /etc/mtab (actually, their current value as returned by the hasmntopt() library function).

The relatime option is different from most, if not all of the other mount options in the Linux kernel as of 2.6.30 because it defaults to being set, however, in an apparent attempt at "compatibility, when the option is set, its value is recorded in /etc/mtab (and /proc/self/mounts). Other "normal" options such as "atime" and "exec" do not show up in /etc/mtab when they're set. In the case of those other options, when they are set, a "no" instance is recorded in /etc/mtab. In other words, for all normal mount options, the lack of it in /etc/mtab means it is enabled and the presence of a "no" version means it is enabled. The relatime option, however, is enabled when it is present in /etc/mtab and disabled when it is not present.

The logic in libzfs doesn't understand that some options can default to being set and clearly this could be considered to be a but. We could add an autoconf test to try to intuit a system's default relatime behavior based on the kernel version but even that wouldn't be 100% reliable because it can be overridden in custom kernels. With the results of that test, we could handle relatime differently than all the other options and print the value correctly.

The way things work right now is that if "relatime" is currently determined as being set by hasmntopt() but the ZFS property is not set, it is displayed as "on temporary" and this is what you must be seeing. Yes, this is bogus but it "works" for options handled completely in the kernel (see below).

Some mount options are handled within the Linux kernel, itself and some are handled completely within ZFS. The atime and relatime properties are handled completely within ZFS and a best effort is made to keep the illusion that the kernel's mount properties have some meaning.

I suppose a reasonable workaround for the time being would be to handle, at least, relatime differently and never activate the "temporary detection" logic and always simply display the actual property value.

@dweeezil
Copy link
Contributor Author

@spacelama I think dweeezil/zfs@62ffaeb gives us more rational behavior. Could you please give it a try?

@spacelama
Copy link

On Sun, 13 Apr 2014, Tim Chase wrote:

@spacelama I think dweeezil/zfs@62ffaeb gives us more rational behavior. Could you please give it a try?

Much better, thanks!

sudo zfs get -r all tank3 | grep atime
tank3 atime on default
tank3 relatime off default
tank3/.mp3.low_qual atime on default
tank3/.mp3.low_qual relatime off default
...
(and worked as expected with it on, or off, or inherit)

Tim Connors

@dweeezil
Copy link
Contributor Author

@spacelama I'm going to mull this over a bit more. Additional documentation might be in order; it does describe temporary properties and doesn't include atime/relatime in the list of those to which they apply. I do think my patch makes a bit of sense, however, because a "temporary override" doesn't change ZFS' behavior.

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.

3 participants