Ensure relatime updates atime at least once per day
Allow atime to be updated once per day even with relatime. This lets
utilities like tmpreaper (which delete files based on last access time)
continue working.
Signed-off-by: Matthew Garrett <[email protected]>
Reviewed-by: Matthew Wilcox <[email protected]>
Acked-by: Valerie Aurora Henson <[email protected]>
Acked-by: Alan Cox <[email protected]>
Acked-by: Ingo Molnar <[email protected]>
---
diff --git a/fs/inode.c b/fs/inode.c
index 0487ddb..057c92b 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1179,6 +1179,40 @@ sector_t bmap(struct inode * inode, sector_t block)
}
EXPORT_SYMBOL(bmap);
+/*
+ * With relative atime, only update atime if the previous atime is
+ * earlier than either the ctime or mtime or if at least a day has
+ * passed since the last atime update.
+ */
+static int relatime_need_update(struct vfsmount *mnt, struct inode *inode,
+ struct timespec now)
+{
+
+ if (!(mnt->mnt_flags & MNT_RELATIME))
+ return 1;
+ /*
+ * Is mtime younger than atime? If yes, update atime:
+ */
+ if (timespec_compare(&inode->i_mtime, &inode->i_atime) >= 0)
+ return 1;
+ /*
+ * Is ctime younger than atime? If yes, update atime:
+ */
+ if (timespec_compare(&inode->i_ctime, &inode->i_atime) >= 0)
+ return 1;
+
+ /*
+ * Is the previous atime value older than a day? If yes,
+ * update atime:
+ */
+ if ((long)(now.tv_sec - inode->i_atime.tv_sec) >= 24*60*60)
+ return 1;
+ /*
+ * Good, we can skip the atime update:
+ */
+ return 0;
+}
+
/**
* touch_atime - update the access time
* @mnt: mount the inode is accessed on
@@ -1206,17 +1240,12 @@ void touch_atime(struct vfsmount *mnt, struct dentry *dentry)
goto out;
if ((mnt->mnt_flags & MNT_NODIRATIME) && S_ISDIR(inode->i_mode))
goto out;
- if (mnt->mnt_flags & MNT_RELATIME) {
- /*
- * With relative atime, only update atime if the previous
- * atime is earlier than either the ctime or mtime.
- */
- if (timespec_compare(&inode->i_mtime, &inode->i_atime) < 0 &&
- timespec_compare(&inode->i_ctime, &inode->i_atime) < 0)
- goto out;
- }
now = current_fs_time(inode->i_sb);
+
+ if (!relatime_need_update(mnt, inode, now))
+ goto out;
+
if (timespec_equal(&inode->i_atime, &now))
goto out;
--
Matthew Garrett | [email protected]
On Sun, 28 Dec 2008, Matthew Garrett wrote:
> Ensure relatime updates atime at least once per day
>
> Allow atime to be updated once per day even with relatime. This lets
> utilities like tmpreaper (which delete files based on last access time)
> continue working.
>
> Signed-off-by: Matthew Garrett <[email protected]>
> Reviewed-by: Matthew Wilcox <[email protected]>
> Acked-by: Valerie Aurora Henson <[email protected]>
> Acked-by: Alan Cox <[email protected]>
> Acked-by: Ingo Molnar <[email protected]>
>
Overall I think the patch looks good.
Feel free to add
Reviewed-by: Jesper Juhl <[email protected]>
if you like.
I only have a single pedantic comment below.
> ---
>
> diff --git a/fs/inode.c b/fs/inode.c
> index 0487ddb..057c92b 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1179,6 +1179,40 @@ sector_t bmap(struct inode * inode, sector_t block)
> }
> EXPORT_SYMBOL(bmap);
>
> +/*
> + * With relative atime, only update atime if the previous atime is
> + * earlier than either the ctime or mtime or if at least a day has
> + * passed since the last atime update.
> + */
> +static int relatime_need_update(struct vfsmount *mnt, struct inode *inode,
> + struct timespec now)
> +{
> +
> + if (!(mnt->mnt_flags & MNT_RELATIME))
> + return 1;
> + /*
> + * Is mtime younger than atime? If yes, update atime:
> + */
> + if (timespec_compare(&inode->i_mtime, &inode->i_atime) >= 0)
> + return 1;
> + /*
> + * Is ctime younger than atime? If yes, update atime:
> + */
> + if (timespec_compare(&inode->i_ctime, &inode->i_atime) >= 0)
> + return 1;
> +
> + /*
> + * Is the previous atime value older than a day? If yes,
> + * update atime:
> + */
> + if ((long)(now.tv_sec - inode->i_atime.tv_sec) >= 24*60*60)
> + return 1;
Not all days are 24*60*60 seconds long. Daylight savings time as well as
leap seconds make this an inaccurate/incorrect constant for representing
"one day".
I don't think we really care, but perhaps the comment above should
acknowledge the fact that this is aproximately one day?
> + /*
> + * Good, we can skip the atime update:
> + */
> + return 0;
> +}
> +
> /**
> * touch_atime - update the access time
> * @mnt: mount the inode is accessed on
> @@ -1206,17 +1240,12 @@ void touch_atime(struct vfsmount *mnt, struct dentry *dentry)
> goto out;
> if ((mnt->mnt_flags & MNT_NODIRATIME) && S_ISDIR(inode->i_mode))
> goto out;
> - if (mnt->mnt_flags & MNT_RELATIME) {
> - /*
> - * With relative atime, only update atime if the previous
> - * atime is earlier than either the ctime or mtime.
> - */
> - if (timespec_compare(&inode->i_mtime, &inode->i_atime) < 0 &&
> - timespec_compare(&inode->i_ctime, &inode->i_atime) < 0)
> - goto out;
> - }
>
> now = current_fs_time(inode->i_sb);
> +
> + if (!relatime_need_update(mnt, inode, now))
> + goto out;
> +
> if (timespec_equal(&inode->i_atime, &now))
> goto out;
>
--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
Matthew Garrett schreef:
> Ensure relatime updates atime at least once per day
>
> Allow atime to be updated once per day even with relatime. This lets
> utilities like tmpreaper (which delete files based on last access time)
> continue working.
:
Sorry, but I doubt it's a good idea. First, it breaks the simple
semantic of relatime (mtime > atime?), mixing it with a rather arbitrary
constant. Second, and most important, there are lots of workloads which
will be strongly affected by this modification. For instance, running
md5sum daily on the filesystem will cause a write for every file.
I think that to solve the problem for your use case, it's better to use
a different approach such as mounting separately /tmp (with the atime
option).
Eric
On Sun, Dec 28, 2008 at 08:04:50PM +0100, ?ric Piel wrote:
> Matthew Garrett schreef:
> > Ensure relatime updates atime at least once per day
> >
> > Allow atime to be updated once per day even with relatime. This lets
> > utilities like tmpreaper (which delete files based on last access time)
> > continue working.
> :
> Sorry, but I doubt it's a good idea. First, it breaks the simple
> semantic of relatime (mtime > atime?), mixing it with a rather arbitrary
> constant. Second, and most important, there are lots of workloads which
> will be strongly affected by this modification. For instance, running
> md5sum daily on the filesystem will cause a write for every file.
Yes. And? I can't think of a single case where something could
absolutely depend on the current relatime semantics, so altering them to
more usefully match the atime semantics doesn't seem likely to cause any
trouble.
> I think that to solve the problem for your use case, it's better to use
> a different approach such as mounting separately /tmp (with the atime
> option).
The use case in this case is the significant body of currently installed
machines that don't have /tmp on a separate filesystem. In the very
common setup of tmpreaper being used, the current relatime semantics
will result in undesired data loss. I think the proposed alteration
makes the behaviour of relatime massively more useful without any
obvious drawbacks.
--
Matthew Garrett | [email protected]
On Sun, Dec 28, 2008 at 07:35:25PM +0100, Jesper Juhl wrote:
> Not all days are 24*60*60 seconds long. Daylight savings time as well as
> leap seconds make this an inaccurate/incorrect constant for representing
> "one day".
> I don't think we really care, but perhaps the comment above should
> acknowledge the fact that this is aproximately one day?
"Day" in the ISO 8601 sense, ie:
2.2.5 day
unit of time, equal to 24 hours
as opposed to
2.2.6 calendar day
time interval starting at midnight and ending at the next midnight, the
latter being also the starting instant of the next calendar day
--
Matthew Garrett | [email protected]
Matthew Garrett schreef:
> On Sun, Dec 28, 2008 at 08:04:50PM +0100, ?ric Piel wrote:
>> Matthew Garrett schreef:
>>> Ensure relatime updates atime at least once per day
>>>
>>> Allow atime to be updated once per day even with relatime. This lets
>>> utilities like tmpreaper (which delete files based on last access time)
>>> continue working.
>> :
>> Sorry, but I doubt it's a good idea. First, it breaks the simple
>> semantic of relatime (mtime > atime?), mixing it with a rather arbitrary
>> constant. Second, and most important, there are lots of workloads which
>> will be strongly affected by this modification. For instance, running
>> md5sum daily on the filesystem will cause a write for every file.
>
> Yes. And? I can't think of a single case where something could
> absolutely depend on the current relatime semantics, so altering them to
> more usefully match the atime semantics doesn't seem likely to cause any
> trouble.
Of course, by construction, there is nothing relying on the current
relatime semantics. The problem is that whenever you are making relatime
closer to the atime behaviour, you are also getting closer to the
original drawbacks of atime (one read generates one write).
> The use case in this case is the significant body of currently installed
> machines that don't have /tmp on a separate filesystem. In the very
> common setup of tmpreaper being used, the current relatime semantics
> will result in undesired data loss. I think the proposed alteration
> makes the behaviour of relatime massively more useful without any
> obvious drawbacks.
Yes, it might bring important drawbacks: performance-wise, relatime will
become more like atime, making it much less useful. There is also a
significant number of desktop computers that are turned on once a day,
the boot time may get hindered by those additional writes.
Actually, you are changing relatime from a boolean condition (maximum
one additional write per write) to a atime with a coarse grain (maximum
one additional write per day). Today you found a use case that needs a
precision of one day. Tomorrow, someone else will find a use case that
needs a precision of one hour. So maybe what is actually needed is a
third option, a "grainatime" option where you can change the precision
of the atime.
Eric
On Sun, Dec 28, 2008 at 10:24:55PM +0100, ?ric Piel wrote:
> Yes, it might bring important drawbacks: performance-wise, relatime will
> become more like atime, making it much less useful. There is also a
> significant number of desktop computers that are turned on once a day,
> the boot time may get hindered by those additional writes.
Huh? Nobody's ever claimed that atime writes cost a significant amount
of performance. The problem that relatime is designed to solve is
*spin-up* when a file is accessed.
> Actually, you are changing relatime from a boolean condition (maximum
> one additional write per write) to a atime with a coarse grain (maximum
> one additional write per day). Today you found a use case that needs a
> precision of one day. Tomorrow, someone else will find a use case that
> needs a precision of one hour. So maybe what is actually needed is a
> third option, a "grainatime" option where you can change the precision
> of the atime.
You're really over-thinking this.
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
On Sun, Dec 28, 2008 at 10:24:55PM +0100, ?ric Piel wrote:
> Of course, by construction, there is nothing relying on the current
> relatime semantics. The problem is that whenever you are making relatime
> closer to the atime behaviour, you are also getting closer to the
> original drawbacks of atime (one read generates one write).
If your io is sufficiently limited that one write per file per day is a
significant problem, then I think there's a more serious underlying
problem.
> Actually, you are changing relatime from a boolean condition (maximum
> one additional write per write) to a atime with a coarse grain (maximum
> one additional write per day). Today you found a use case that needs a
> precision of one day. Tomorrow, someone else will find a use case that
> needs a precision of one hour. So maybe what is actually needed is a
> third option, a "grainatime" option where you can change the precision
> of the atime.
The answer to "The current two options are suboptimal in almost all
cases" is very rarely "Add a new option". If boot is touching too many
files, it should stop touching as many files. If you have crap disks,
get better disks. If your life is spent tuning fs parameters for 1%
performance gains, find a better job.
--
Matthew Garrett | [email protected]
What was the fate of Ingo's previous patch[*1] to implement this (which,
incidentally, also included updates to documentation and made the interval
configurable as ?ric Piel suggests) ?
[*1]: http://lwn.net/Articles/244384/
Just a minor nit, but the patch description (not included in the patch itself)
"at least once a day" isn't particularly accurate given that the patch enforces
at least a day [minus a second or so] has elapsed since the last update. The
description in the above-referenced patch "only once a day" better captures the
situation, I think.
pjrm.
On Thu, Jan 08, 2009 at 11:29:53PM +1100, Peter Moulder wrote:
> What was the fate of Ingo's previous patch[*1] to implement this (which,
> incidentally, also included updates to documentation and made the interval
> configurable as ?ric Piel suggests) ?
Nobody merged it and Ingo got bored of pushing it. I sent it earlier,
but nobody came up with an especially good argument for the
configurability and everyone seemed to want to paint the patch pink with
yellow polka dots, so I gave up and just sent a minimal one in the hope
that nobody could find anything to complain about.
> Just a minor nit, but the patch description (not included in the patch itself)
> "at least once a day" isn't particularly accurate given that the patch enforces
> at least a day [minus a second or so] has elapsed since the last update. The
> description in the above-referenced patch "only once a day" better captures the
> situation, I think.
If mtime is changed then atime will be updated on the next access, so
you can easily have more than one atime update a day.
--
Matthew Garrett | [email protected]