Re: [PATCH] vfs: update atimes over one day in the past or future

[Andreas Dilger]

On 2012-12-15, at 19:15, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> 
> On Sat, Dec 15, 2012 at 11:25:23PM +0800, ys wrote:
>> From 3d56c131b58a21c05bcd677b9d2ba915abcbf195 Mon Sep 17 00:00:00 2001
>> From: yangsheng <sickamd@xxxxxxxxx>
>> Date: Sat, 15 Dec 2012 21:46:22 +0800
>> Subject: [PATCH] vfs: update atimes over one day in the past or future
>> 
>> Relatime should update the inode atime if it is more than one day
>> in the future.  The original problem seen was a tarball that had
>> a bad atime in the distant future, but could also happen if someone
>> fat-fingers a "touch".  The future atime will never be fixed.
>> 
>> Without relatime enabled, a future atime is updated to the current
>> kernel time on access.  Relatime is meant to reduce the frequency
>> of atime updates, not decide if whether the system clock or the
>> inode timestamp is correct or not.
>> 
>> Signed-off-by: Yang Sheng <sickamd@xxxxxxxxx>
>> Signed-off-by: Andreas Dilger <adilger@xxxxxxxxx>
>> Acked-by: David Chinner <david@xxxxxxxxxxxxx>
> 
> No I didn't. Please don't add tags that someone has not added
> directly in a reply to the original patch.

That's my fault. I thought you'd OK'd the patch with the revised commit comment. 

>> CC: stable@xxxxxxxxxxxxxxx
>> ---
>> fs/inode.c | 7 ++++---
>> 1 ??????????????????????????? 4 ???(+)????????? 3 ???(-)
> 
> There's something wrong with the character encoding you are using...

Chinese locale, but probably doesn't matter since text below "---" isn't in the commit anyway?

>> diff --git a/fs/inode.c b/fs/inode.c
>> index 14084b7..8713dc8 100644
>> --- a/fs/inode.c
>> +++ b/fs/inode.c
>> @@ -1488,10 +1488,11 @@ static int relatime_need_update(struct vfsmount *mnt, struct inode *inode,
>>        return 1;
>> 
>>    /*
>> -     * Is the previous atime value older than a day? If yes,
>> -     * update atime:
>> +     * Update atime if it's older than a day or more than a day
>> +     * in the future, which we assume is corrupt.
>> +     *         
> 
> A time in the future is not a corruption - the comment should
> reflect exactly what you've put in the commit message. i.e. that
> relatime is for reducing updates, not preventing atime from ever
> moving backwards.

Ok. 

> Also, you've added an extra line of whitespace damage that doesn't
> need to be there.....
> 
> FWIW, could you write a test for xfstests for this behaviour
> so we can confirm that we don't break it in future?

Sure, I'll take a crack at that. 

Cheers, Andreas--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/