[PATCH] Update atime from future.

[PATCH] Update atime from future.

[yangsheng]

Relatime should update the inode atime if it is more than a day in the
future.  The original problem seen was a tarball that had a bad atime,
but could also happen if someone fat-fingers a "touch".  The future
atime will never be fixed.  Before the relatime patch, the future atime
would be updated back to the current time on the next access.

Only update the atime if it is more than one day in the future.  That
avoids thrashing the atime if the clocks on clients of a network fs are
only slightly out of sync, but still allows fixing bogus atimes.

Signed-off-by: yangsheng <sickamd@gmail.com>
Reviewed-by: adilger@dilger.ca
---
 fs/inode.c | 10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index 14084b7..5c4379a 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1465,6 +1465,7 @@ sector_t bmap(struct inode *inode, sector_t block)
 }
 EXPORT_SYMBOL(bmap);
 
+#define RELATIME_MARGIN (24*60*60)
 /*
  * 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
@@ -1488,10 +1489,17 @@ static int relatime_need_update(struct vfsmount *mnt, struct inode *inode,
 		return 1;
 
 	/*
+	 * Is the previous atime value in future? If yes,
+	 * update atime:
+	 */
+	if ((long)(now.tv_sec - inode->i_atime.tv_sec) < -RELATIME_MARGIN)
+		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)
+	if ((long)(now.tv_sec - inode->i_atime.tv_sec) >= RELATIME_MARGIN)
 		return 1;
 	/*
 	 * Good, we can skip the atime update:
-- 
1.7.11.7

Re: [PATCH] Update atime from future.

[Greg KH]

On Tue, Dec 04, 2012 at 01:56:39AM +0800, yangsheng wrote:
> Relatime should update the inode atime if it is more than a day in the
> future.  The original problem seen was a tarball that had a bad atime,
> but could also happen if someone fat-fingers a "touch".  The future
> atime will never be fixed.  Before the relatime patch, the future atime
> would be updated back to the current time on the next access.
> 
> Only update the atime if it is more than one day in the future.  That
> avoids thrashing the atime if the clocks on clients of a network fs are
> only slightly out of sync, but still allows fixing bogus atimes.
> 
> Signed-off-by: yangsheng <sickamd@gmail.com>
> Reviewed-by: adilger@dilger.ca
> ---
>  fs/inode.c | 10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)


<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

</formletter>

Re: [PATCH] Update atime from future.

[Dave Chinner]

On Tue, Dec 04, 2012 at 01:56:39AM +0800, yangsheng wrote:
> Relatime should update the inode atime if it is more than a day in the
> future.  The original problem seen was a tarball that had a bad atime,
> but could also happen if someone fat-fingers a "touch".  The future
> atime will never be fixed.  Before the relatime patch, the future atime
> would be updated back to the current time on the next access.

So if someone accidentally changes time back a few days, access
times go backwards for everything? That doesn't sound right to me -
it's going to seriously screw up backups and other scanners that use
atime to determine "newly accessed files"....

IMO, if you fat-finger a manual atime update or use atimes direct
from tarballs, then that's your problem as a user and not the
responsibility of the kernel to magically fix for you....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] Update atime from future.

[Andreas Dilger]

On 2012-12-04, at 13:24, Dave Chinner <david@fromorbit.com> wrote:
> On Tue, Dec 04, 2012 at 01:56:39AM +0800, yangsheng wrote:
>> Relatime should update the inode atime if it is more than a day in the
>> future.  The original problem seen was a tarball that had a bad atime,
>> but could also happen if someone fat-fingers a "touch".  The future
>> atime will never be fixed.  Before the relatime patch, the future atime
>> would be updated back to the current time on the next access.
> 
> So if someone accidentally changes time back a few days, access
> times go backwards for everything?

And they will go forward again if the files are accessed again in the future. 

> That doesn't sound right to me -
> it's going to seriously screw up backups and other scanners that use
> atime to determine "newly accessed files"....

I think it is equally as easy to say that if someone accidentally sets the time too far in the future (e.g. 2102 instead of 2012) for a while, then the atimes will be even more broken since the kernel will never update them again even after the clock is fixed. 

The point is that the behaviour before the relatime patch was that the kernel updated the atime to the current time as the kernel knows about it, it didn't make any decision about "the past" or "the future".

Relatime is about reducing the frequency of atime updates, not about deciding that one timestamp is more correct than another. 

I'd be ok to change the margin for what constitutes a "future" time (a few days or a week?), but if atime updates happen once a day for times in the past  I don't think it is incorrect to update the atime if it is more than a day in the future. 

Cheers, Andreas

> IMO, if you fat-finger a manual atime update or use atimes direct
> from tarballs, then that's your problem as a user and not the
> responsibility of the kernel to magically fix for you....
> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

Re: [PATCH] Update atime from future.

[Dave Chinner]

On Tue, Dec 04, 2012 at 05:22:32PM -0700, Andreas Dilger wrote:
> The point is that the behaviour before the relatime patch was that
> the kernel updated the atime to the current time as the kernel
> knows about it, it didn't make any decision about "the past" or
> "the future".
> 
> Relatime is about reducing the frequency of atime updates, not
> about deciding that one timestamp is more correct than another. 

That makes sense.

Indeed, that's what the commit message should say rather than
drawing arbitrary lines in the sand about what is a valid atime
without further justification.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com