| Message ID | 1294131418-3835-1-git-send-email-sickamd@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
yangsheng <sickamd@gmail.com> 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.
On 01/04/2011 05:02 PM, Andreas Schwab wrote: > yangsheng<sickamd@gmail.com> 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. > > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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?
On 2011-01-04, at 11:21, Valdis.Kletnieks@vt.edu 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 -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 04 Jan 2011 12:13:37 MST, Andreas Dilger said: > On 2011-01-04, at 11:21, Valdis.Kletnieks@vt.edu 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.
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: