vfs: update atimes over one day in the past or future
PATCH 1/1 vfs: update atimes over one day in the past or future
Quoted 1 line
From
3d56c131b58a21c05bcd677b9d2ba915abcbf195Mon Sep 17 00:00:00 2001
From: yangsheng <[email protected]>
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.
--- fs/inode.c | 7 ++++---
1 个文件被修改，插入 4 行(+)，删除 3 行(-)
fs/inode.c +4 -3
@@ -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. +* */ -if ((long )(now.tv_sec - inode->i_atime.tv_sec) >=24 *60 *60 ) +if (abs (now.tv_sec - inode->i_atime.tv_sec) >=24 *60 *60 )return 1 ;/* * Good, we can skip the atime update:
Signature · 1 line
1.7.11.7
[ please place patches inline, not as attachments. ]
Quoted 10 lines · ys
From
3d56c131b58a21c05bcd677b9d2ba915abcbf195Mon Sep 17 00:00:00 2001
From: yangsheng <[email protected]>
Date: Sat, 15 Dec 2012 21:46:22 +0800
Subject: [PATCH] vfs: update atimes over one day in the past or futureRelatime 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.
Quoted 8 lines
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 <[email protected]>Signed-off-by Andreas Dilger <[email protected]>Acked-by David Chinner <[email protected]>
No I didn't. Please don't add tags that someone has not added directly in a reply to the original patch.
Quoted 4 lines
--- fs/inode.c | 7 ++++---1 ??????????????????????????? 4 ???(+)????????? 3 ???(-)
There's something wrong with the character encoding you are using...
Quoted 6 lines
fs/inode.c +0
-0
fs/inode.c static int relatime_need_update(struct vfsmount *mnt, struct inode *inode, · line 1493
Quoted 8 lines
fs/inode.c +3
-2@@ -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.
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?
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Quoted 17 lines · Dave Chinner
Quoted 15 lines · ys
From
3d56c131b58a21c05bcd677b9d2ba915abcbf195Mon Sep 17 00:00:00 2001
From: yangsheng <[email protected]>
Date: Sat, 15 Dec 2012 21:46:22 +0800
Subject: [PATCH] vfs: update atimes over one day in the past or futureRelatime 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.
Quoted 6 lines
Quoted 3 lines
Signed-off-by Yang Sheng <[email protected]>Signed-off-by Andreas Dilger <[email protected]>Acked-by David Chinner <[email protected]>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.
Quoted 6 lines
Quoted 4 lines
---
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?
Quoted 7 lines
Quoted 7 lines
fs/inode.c +0
-0@@ -1488,10 +1488,11 @@ static int relatime_need_update(struct vfsmount *mnt, struct inode *inode,return 1 ;
Quoted 11 lines
fs/inode.c static int relatime_need_update(struct vfsmount *mnt, struct inode *inode, · line 1493
Quoted 6 lines
fs/inode.c +3
-2/* -* 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.
Quoted 5 lines
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
Quoted 18 lines · Andreas Dilger
Quoted 17 lines · Dave Chinner
Quoted 15 lines · ys
From
3d56c131b58a21c05bcd677b9d2ba915abcbf195Mon Sep 17 00:00:00 2001
From: yangsheng <[email protected]>
Date: Sat, 15 Dec 2012 21:46:22 +0800
Subject: [PATCH] vfs: update atimes over one day in the past or futureRelatime 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.
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Signed-off-by Yang Sheng <[email protected]>Signed-off-by Andreas Dilger <[email protected]>Acked-by David Chinner <[email protected]>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.
Easy enough to misunderstand. :/
I simply avoid the issue by only adding tags that have been directly sent by people...
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
---
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?
Yup, but it doesn't inspire confidence that the patch is going to be clean when multiple encodings appear in the one message...
Quoted 7 lines
Quoted 5 lines
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.
Great. Thanks for the followup, Andreas.
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Quoted 6 lines · Dave Chinner
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
---
fs/inode.c| 7 ++++---
1 ??????????????????????????? 4 ???(+)????????? 3 ???(-)
Quoted 6 lines
Quoted 3 lines
Quoted 1 line
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?
Yup, but it doesn't inspire confidence that the patch is going to be clean when multiple encodings appear in the one message...
Multiple encodings are not much of a problem. Lack of specifying them, as happened with that attachment, is.
That's another reason why inline patches work better, because the main message part has the encoding set most of the time :)