2011-01-04 08:57:28

by yangsheng

[permalink] [raw]
Subject: [PATCH resend] Update atime from future.

If atime has been wrong set to future, then it cannot
be updated back to current time.

CC: [email protected]
Signed-off-by: [email protected]
Reviewed-by: [email protected]
---
fs/inode.c | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index da85e56..9cf7375 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1446,6 +1446,8 @@ 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
@@ -1469,10 +1471,16 @@ static int relatime_need_update(struct vfsmount *mnt, struct inode *inode,
return 1;

/*
- * Is the previous atime value older than a day? If yes,
+ * 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 old 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.2.3



2011-01-04 09:02:34

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH resend] Update atime from future.

yangsheng <[email protected]> writes:

> - * Is the previous atime value older than a day? If yes,
> + * Is the previous atime value old than a day? If yes,

Why did you change that comment?

Andreas.

--
Andreas Schwab, [email protected]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."

2011-01-04 09:05:31

by yangsheng

[permalink] [raw]
Subject: Re: [PATCH resend] Update atime from future.

On 01/04/2011 05:02 PM, Andreas Schwab wrote:
> yangsheng<[email protected]> writes:
>
>
>> - * Is the previous atime value older than a day? If yes,
>> + * Is the previous atime value old than a day? If yes,
>>
> Why did you change that comment?
>
Sorry, It is a typo. I'll resent it.
> Andreas.
>
>


2011-01-04 18:22:05

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: [PATCH resend] Update atime from future.

On Tue, 04 Jan 2011 16:56:58 +0800, yangsheng said:
> If atime has been wrong set to future, then it cannot
> be updated back to current time.

> +#define RELATIME_MARGIN (24 * 60 * 60)

Nice patch overall. Should this be a #define, or a CONFIG_ variable,
or a tweakable /proc/sys/fs variable? Or am I senile and we thrashed
all this out once before when the relatime code landed?


Attachments:
(No filename) (227.00 B)

2011-01-04 19:13:37

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH resend] Update atime from future.

On 2011-01-04, at 11:21, [email protected] wrote:
> On Tue, 04 Jan 2011 16:56:58 +0800, yangsheng said:
>> If atime has been wrong set to future, then it cannot
>> be updated back to current time.
>>
>> +#define RELATIME_MARGIN (24 * 60 * 60)
>
> Nice patch overall. Should this be a #define, or a CONFIG_ variable,
> or a tweakable /proc/sys/fs variable? Or am I senile and we thrashed
> all this out once before when the relatime code landed?

I recall the consensus was that a /proc tunable was "too much" for the initial patch. An atime update interval of 1 day is sufficient for most applications, since they run daily to do file access scanning. The #define was added because I dislike having multiple hard-coded values in any code.

I haven't heard of any complaints about the relatime update frequency, except for this "atime in the future" problem, so until that happens we may as well leave it as-is.

Cheers, Andreas

2011-01-04 20:32:08

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: [PATCH resend] Update atime from future.

On Tue, 04 Jan 2011 12:13:37 MST, Andreas Dilger said:
> On 2011-01-04, at 11:21, [email protected] wrote:
> > On Tue, 04 Jan 2011 16:56:58 +0800, yangsheng said:
> >> If atime has been wrong set to future, then it cannot
> >> be updated back to current time.
> >>
> >> +#define RELATIME_MARGIN (24 * 60 * 60)
> >
> > Nice patch overall. Should this be a #define, or a CONFIG_ variable,
> > or a tweakable /proc/sys/fs variable? Or am I senile and we thrashed
> > all this out once before when the relatime code landed?
>
> I recall the consensus was that a /proc tunable was "too much" for the
> initial patch.

OK, in that case yangsheng's patch is probably good to go.


Attachments:
(No filename) (227.00 B)