* [PATCH] vfs: update atimes over one day in the past or future
@ 2012-12-15 15:25 ys
2012-12-16 2:15 ` Dave Chinner
0 siblings, 1 reply; 5+ messages in thread
From: ys @ 2012-12-15 15:25 UTC (permalink / raw)
To: david; +Cc: linux-kernel, linux-fsdevel, adilger
[-- Attachment #1: Type: text/plain, Size: 1 bytes --]
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-vfs-update-atimes-over-one-day-in-the-past-or-future.patch --]
[-- Type: text/x-patch; name="0001-vfs-update-atimes-over-one-day-in-the-past-or-future.patch", Size: 1587 bytes --]
>From 3d56c131b58a21c05bcd677b9d2ba915abcbf195 Mon Sep 17 00:00:00 2001
From: yangsheng <sickamd@gmail.com>
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@gmail.com>
Signed-off-by: Andreas Dilger <adilger@dilger.ca>
Acked-by: David Chinner <david@fromorbit.com>
CC: stable@vger.kernel.org
---
fs/inode.c | 7 ++++---
1 个文件被修改，插入 4 行(+)，删除 3 行(-)
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.
+ *
*/
- 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:
--
1.7.11.7
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] vfs: update atimes over one day in the past or future
2012-12-15 15:25 [PATCH] vfs: update atimes over one day in the past or future ys
@ 2012-12-16 2:15 ` Dave Chinner
2012-12-18 12:14 ` Andreas Dilger
0 siblings, 1 reply; 5+ messages in thread
From: Dave Chinner @ 2012-12-16 2:15 UTC (permalink / raw)
To: ys; +Cc: linux-kernel, linux-fsdevel, adilger
[ please place patches inline, not as attachments. ]
On Sat, Dec 15, 2012 at 11:25:23PM +0800, ys wrote:
> From 3d56c131b58a21c05bcd677b9d2ba915abcbf195 Mon Sep 17 00:00:00 2001
> From: yangsheng <sickamd@gmail.com>
> 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@gmail.com>
> Signed-off-by: Andreas Dilger <adilger@dilger.ca>
> Acked-by: David Chinner <david@fromorbit.com>
No I didn't. Please don't add tags that someone has not added
directly in a reply to the original patch.
> CC: stable@vger.kernel.org
> ---
> fs/inode.c | 7 ++++---
> 1 ??????????????????????????? 4 ???(+)????????? 3 ???(-)
There's something wrong with the character encoding you are using...
>
> 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.
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.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] vfs: update atimes over one day in the past or future
2012-12-16 2:15 ` Dave Chinner
@ 2012-12-18 12:14 ` Andreas Dilger
2012-12-18 21:14 ` Dave Chinner
0 siblings, 1 reply; 5+ messages in thread
From: Andreas Dilger @ 2012-12-18 12:14 UTC (permalink / raw)
To: Dave Chinner
Cc: ys, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
On 2012-12-15, at 19:15, Dave Chinner <david@fromorbit.com> 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@gmail.com>
>> 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@gmail.com>
>> Signed-off-by: Andreas Dilger <adilger@dilger.ca>
>> Acked-by: David Chinner <david@fromorbit.com>
>
> 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@vger.kernel.org
>> ---
>> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] vfs: update atimes over one day in the past or future
2012-12-18 12:14 ` Andreas Dilger
@ 2012-12-18 21:14 ` Dave Chinner
2012-12-26 4:33 ` Jan Engelhardt
0 siblings, 1 reply; 5+ messages in thread
From: Dave Chinner @ 2012-12-18 21:14 UTC (permalink / raw)
To: Andreas Dilger
Cc: ys, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
On Tue, Dec 18, 2012 at 05:14:57AM -0700, Andreas Dilger wrote:
> On 2012-12-15, at 19:15, Dave Chinner <david@fromorbit.com> 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@gmail.com>
> >> 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@gmail.com>
> >> Signed-off-by: Andreas Dilger <adilger@dilger.ca>
> >> Acked-by: David Chinner <david@fromorbit.com>
> >
> > 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...
> >> CC: stable@vger.kernel.org
> >> ---
> >> 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...
> > 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.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] vfs: update atimes over one day in the past or future
2012-12-18 21:14 ` Dave Chinner
@ 2012-12-26 4:33 ` Jan Engelhardt
0 siblings, 0 replies; 5+ messages in thread
From: Jan Engelhardt @ 2012-12-26 4:33 UTC (permalink / raw)
To: Dave Chinner
Cc: Andreas Dilger, ys, linux-kernel@vger.kernel.org,
linux-fsdevel@vger.kernel.org
On Tuesday 2012-12-18 22:14, Dave Chinner wrote:
>
>> >> CC: stable@vger.kernel.org
>> >> ---
>> >> 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...
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 :)
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-12-26 4:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-15 15:25 [PATCH] vfs: update atimes over one day in the past or future ys
2012-12-16 2:15 ` Dave Chinner
2012-12-18 12:14 ` Andreas Dilger
2012-12-18 21:14 ` Dave Chinner
2012-12-26 4:33 ` Jan Engelhardt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).