[PATCH, resend] relatime: Let relatime update atime at least once per day

[PATCH, resend] relatime: Let relatime update atime at least once per day

[Matthew Garrett]

Ensure relatime updates atime at least once per day
    
Allow atime to be updated once per day even with relatime. This lets
utilities like tmpreaper (which delete files based on last access time)
continue working.
    
Signed-off-by: Matthew Garrett <mjg@redhat.com>
Reviewed-by: Matthew Wilcox <willy@linux.intel.com>
Acked-by: Valerie Aurora Henson <vaurora@redhat.com>
Acked-by: Alan Cox <alan@redhat.com>
Acked-by: Ingo Molnar <mingo@elte.hu>

---

diff --git a/fs/inode.c b/fs/inode.c
index 0487ddb..057c92b 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1179,6 +1179,40 @@ sector_t bmap(struct inode * inode, sector_t block)
 }
 EXPORT_SYMBOL(bmap);
 
+/*
+ * 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
+ * passed since the last atime update.
+ */
+static int relatime_need_update(struct vfsmount *mnt, struct inode *inode,
+			     struct timespec now)
+{
+
+	if (!(mnt->mnt_flags & MNT_RELATIME))
+		return 1;
+	/*
+	 * Is mtime younger than atime? If yes, update atime:
+	 */
+	if (timespec_compare(&inode->i_mtime, &inode->i_atime) >= 0)
+		return 1;
+	/*
+	 * Is ctime younger than atime? If yes, update atime:
+	 */
+	if (timespec_compare(&inode->i_ctime, &inode->i_atime) >= 0)
+		return 1;
+
+	/*
+	 * Is the previous atime value older than a day? If yes,
+	 * update atime:
+	 */
+	if ((long)(now.tv_sec - inode->i_atime.tv_sec) >= 24*60*60)
+		return 1;
+	/*
+	 * Good, we can skip the atime update:
+	 */
+	return 0;
+}
+
 /**
  *	touch_atime	-	update the access time
  *	@mnt: mount the inode is accessed on
@@ -1206,17 +1240,12 @@ void touch_atime(struct vfsmount *mnt, struct dentry *dentry)
 		goto out;
 	if ((mnt->mnt_flags & MNT_NODIRATIME) && S_ISDIR(inode->i_mode))
 		goto out;
-	if (mnt->mnt_flags & MNT_RELATIME) {
-		/*
-		 * With relative atime, only update atime if the previous
-		 * atime is earlier than either the ctime or mtime.
-		 */
-		if (timespec_compare(&inode->i_mtime, &inode->i_atime) < 0 &&
-		    timespec_compare(&inode->i_ctime, &inode->i_atime) < 0)
-			goto out;
-	}
 
 	now = current_fs_time(inode->i_sb);
+
+	if (!relatime_need_update(mnt, inode, now))
+		goto out;
+
 	if (timespec_equal(&inode->i_atime, &now))
 		goto out;

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH, resend] relatime: Let relatime update atime at least once per day

[Jesper Juhl]

On Sun, 28 Dec 2008, Matthew Garrett wrote:

> Ensure relatime updates atime at least once per day
>     
> Allow atime to be updated once per day even with relatime. This lets
> utilities like tmpreaper (which delete files based on last access time)
> continue working.
>     
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> Reviewed-by: Matthew Wilcox <willy@linux.intel.com>
> Acked-by: Valerie Aurora Henson <vaurora@redhat.com>
> Acked-by: Alan Cox <alan@redhat.com>
> Acked-by: Ingo Molnar <mingo@elte.hu>
> 

Overall I think the patch looks good.
Feel free to add
 Reviewed-by: Jesper Juhl <jj@chaosbits.net>
if you like.

I only have a single pedantic comment below.

> ---
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index 0487ddb..057c92b 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1179,6 +1179,40 @@ sector_t bmap(struct inode * inode, sector_t block)
>  }
>  EXPORT_SYMBOL(bmap);
>  
> +/*
> + * 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
> + * passed since the last atime update.
> + */
> +static int relatime_need_update(struct vfsmount *mnt, struct inode *inode,
> +			     struct timespec now)
> +{
> +
> +	if (!(mnt->mnt_flags & MNT_RELATIME))
> +		return 1;
> +	/*
> +	 * Is mtime younger than atime? If yes, update atime:
> +	 */
> +	if (timespec_compare(&inode->i_mtime, &inode->i_atime) >= 0)
> +		return 1;
> +	/*
> +	 * Is ctime younger than atime? If yes, update atime:
> +	 */
> +	if (timespec_compare(&inode->i_ctime, &inode->i_atime) >= 0)
> +		return 1;
> +
> +	/*
> +	 * Is the previous atime value older than a day? If yes,
> +	 * update atime:
> +	 */
> +	if ((long)(now.tv_sec - inode->i_atime.tv_sec) >= 24*60*60)
> +		return 1;

Not all days are 24*60*60 seconds long. Daylight savings time as well as 
leap seconds make this an inaccurate/incorrect constant for representing 
"one day".
I don't think we really care, but perhaps the comment above should 
acknowledge the fact that this is aproximately one day?

> +	/*
> +	 * Good, we can skip the atime update:
> +	 */
> +	return 0;
> +}
> +
>  /**
>   *	touch_atime	-	update the access time
>   *	@mnt: mount the inode is accessed on
> @@ -1206,17 +1240,12 @@ void touch_atime(struct vfsmount *mnt, struct dentry *dentry)
>  		goto out;
>  	if ((mnt->mnt_flags & MNT_NODIRATIME) && S_ISDIR(inode->i_mode))
>  		goto out;
> -	if (mnt->mnt_flags & MNT_RELATIME) {
> -		/*
> -		 * With relative atime, only update atime if the previous
> -		 * atime is earlier than either the ctime or mtime.
> -		 */
> -		if (timespec_compare(&inode->i_mtime, &inode->i_atime) < 0 &&
> -		    timespec_compare(&inode->i_ctime, &inode->i_atime) < 0)
> -			goto out;
> -	}
>  
>  	now = current_fs_time(inode->i_sb);
> +
> +	if (!relatime_need_update(mnt, inode, now))
> +		goto out;
> +
>  	if (timespec_equal(&inode->i_atime, &now))
>  		goto out;
> 

-- 
Jesper Juhl <jj@chaosbits.net>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

Re: [PATCH, resend] relatime: Let relatime update atime at least once per day

[Matthew Garrett]

On Sun, Dec 28, 2008 at 07:35:25PM +0100, Jesper Juhl wrote:

> Not all days are 24*60*60 seconds long. Daylight savings time as well as 
> leap seconds make this an inaccurate/incorrect constant for representing 
> "one day".
> I don't think we really care, but perhaps the comment above should 
> acknowledge the fact that this is aproximately one day?

"Day" in the ISO 8601 sense, ie:

2.2.5 day
unit of time, equal to 24 hours

as opposed to

2.2.6 calendar day
time interval starting at midnight and ending at the next midnight, the 
latter being also the starting instant of the next calendar day

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH, resend] relatime: Let relatime update atime at least once per day

[Éric Piel]

[-- Attachment #1: Type: text/plain, Size: 757 bytes --]

Matthew Garrett schreef:
> Ensure relatime updates atime at least once per day
>     
> Allow atime to be updated once per day even with relatime. This lets
> utilities like tmpreaper (which delete files based on last access time)
> continue working.
:
Sorry, but I doubt it's a good idea. First, it breaks the simple
semantic of relatime (mtime > atime?), mixing it with a rather arbitrary
constant. Second, and most important, there are lots of workloads which
will be strongly affected by this modification. For instance, running
md5sum daily on the filesystem will cause a write for every file.

I think that to solve the problem for your use case, it's better to use
a different approach such as mounting separately /tmp (with the atime
option).

Eric

[-- Attachment #2: E_A_B_Piel.vcf --]
[-- Type: text/x-vcard, Size: 367 bytes --]

begin:vcard
fn;quoted-printable:=C3=89ric Piel
n;quoted-printable:Piel;=C3=89ric
org:Technical University of Delft;Software Engineering Research Group
adr:HB 08.080;;Mekelweg 4;Delft;;2628 CD;The Netherlands
email;internet:E.A.B.Piel@tudelft.nl
tel;work:+31 15 278 6338
tel;cell:+31 6 2437 9135
x-mozilla-html:FALSE
url:http://pieleric.free.fr
version:2.1
end:vcard

Re: [PATCH, resend] relatime: Let relatime update atime at least once per day

[Matthew Garrett]

On Sun, Dec 28, 2008 at 08:04:50PM +0100, Éric Piel wrote:
> Matthew Garrett schreef:
> > Ensure relatime updates atime at least once per day
> >     
> > Allow atime to be updated once per day even with relatime. This lets
> > utilities like tmpreaper (which delete files based on last access time)
> > continue working.
> :
> Sorry, but I doubt it's a good idea. First, it breaks the simple
> semantic of relatime (mtime > atime?), mixing it with a rather arbitrary
> constant. Second, and most important, there are lots of workloads which
> will be strongly affected by this modification. For instance, running
> md5sum daily on the filesystem will cause a write for every file.

Yes. And? I can't think of a single case where something could 
absolutely depend on the current relatime semantics, so altering them to 
more usefully match the atime semantics doesn't seem likely to cause any 
trouble.

> I think that to solve the problem for your use case, it's better to use
> a different approach such as mounting separately /tmp (with the atime
> option).

The use case in this case is the significant body of currently installed 
machines that don't have /tmp on a separate filesystem. In the very 
common setup of tmpreaper being used, the current relatime semantics 
will result in undesired data loss. I think the proposed alteration 
makes the behaviour of relatime massively more useful without any 
obvious drawbacks.

-- 
Matthew Garrett | mjg59@srcf.ucam.org
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH, resend] relatime: Let relatime update atime at least once per day

[Éric Piel]

[-- Attachment #1: Type: text/plain, Size: 2312 bytes --]

Matthew Garrett schreef:
> On Sun, Dec 28, 2008 at 08:04:50PM +0100, Éric Piel wrote:
>> Matthew Garrett schreef:
>>> Ensure relatime updates atime at least once per day
>>>     
>>> Allow atime to be updated once per day even with relatime. This lets
>>> utilities like tmpreaper (which delete files based on last access time)
>>> continue working.
>> :
>> Sorry, but I doubt it's a good idea. First, it breaks the simple
>> semantic of relatime (mtime > atime?), mixing it with a rather arbitrary
>> constant. Second, and most important, there are lots of workloads which
>> will be strongly affected by this modification. For instance, running
>> md5sum daily on the filesystem will cause a write for every file.
> 
> Yes. And? I can't think of a single case where something could 
> absolutely depend on the current relatime semantics, so altering them to 
> more usefully match the atime semantics doesn't seem likely to cause any 
> trouble.
Of course, by construction, there is nothing relying on the current
relatime semantics. The problem is that whenever you are making relatime
closer to the atime behaviour, you are also getting closer to the
original drawbacks of atime (one read generates one write).

> The use case in this case is the significant body of currently installed 
> machines that don't have /tmp on a separate filesystem. In the very 
> common setup of tmpreaper being used, the current relatime semantics 
> will result in undesired data loss. I think the proposed alteration 
> makes the behaviour of relatime massively more useful without any 
> obvious drawbacks.
Yes, it might bring important drawbacks: performance-wise, relatime will
become more like atime, making it much less useful. There is also a
significant number of desktop computers that are turned on once a day,
the boot time may get hindered by those additional writes.

Actually, you are changing relatime from a boolean condition (maximum
one additional write per write) to a atime with a coarse grain (maximum
one additional write per day). Today you found a use case that needs a
precision of one day. Tomorrow, someone else will find a use case that
needs a precision of one hour. So maybe what is actually needed is a
third option, a "grainatime" option where you can change the precision
of the atime.

Eric

[-- Attachment #2: E_A_B_Piel.vcf --]
[-- Type: text/x-vcard, Size: 367 bytes --]

begin:vcard
fn;quoted-printable:=C3=89ric Piel
n;quoted-printable:Piel;=C3=89ric
org:Technical University of Delft;Software Engineering Research Group
adr:HB 08.080;;Mekelweg 4;Delft;;2628 CD;The Netherlands
email;internet:E.A.B.Piel@tudelft.nl
tel;work:+31 15 278 6338
tel;cell:+31 6 2437 9135
x-mozilla-html:FALSE
url:http://pieleric.free.fr
version:2.1
end:vcard

Re: [PATCH, resend] relatime: Let relatime update atime at least once per day

[Matthew Wilcox]

On Sun, Dec 28, 2008 at 10:24:55PM +0100, ?ric Piel wrote:
> Yes, it might bring important drawbacks: performance-wise, relatime will
> become more like atime, making it much less useful. There is also a
> significant number of desktop computers that are turned on once a day,
> the boot time may get hindered by those additional writes.

Huh?  Nobody's ever claimed that atime writes cost a significant amount
of performance.  The problem that relatime is designed to solve is
*spin-up* when a file is accessed.

> Actually, you are changing relatime from a boolean condition (maximum
> one additional write per write) to a atime with a coarse grain (maximum
> one additional write per day). Today you found a use case that needs a
> precision of one day. Tomorrow, someone else will find a use case that
> needs a precision of one hour. So maybe what is actually needed is a
> third option, a "grainatime" option where you can change the precision
> of the atime.

You're really over-thinking this.

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH, resend] relatime: Let relatime update atime at least once per day

[Matthew Garrett]

On Sun, Dec 28, 2008 at 10:24:55PM +0100, Éric Piel wrote:

> Of course, by construction, there is nothing relying on the current
> relatime semantics. The problem is that whenever you are making relatime
> closer to the atime behaviour, you are also getting closer to the
> original drawbacks of atime (one read generates one write).

If your io is sufficiently limited that one write per file per day is a 
significant problem, then I think there's a more serious underlying 
problem.

> Actually, you are changing relatime from a boolean condition (maximum
> one additional write per write) to a atime with a coarse grain (maximum
> one additional write per day). Today you found a use case that needs a
> precision of one day. Tomorrow, someone else will find a use case that
> needs a precision of one hour. So maybe what is actually needed is a
> third option, a "grainatime" option where you can change the precision
> of the atime.

The answer to "The current two options are suboptimal in almost all 
cases" is very rarely "Add a new option". If boot is touching too many 
files, it should stop touching as many files. If you have crap disks, 
get better disks. If your life is spent tuning fs parameters for 1% 
performance gains, find a better job.

-- 
Matthew Garrett | mjg59@srcf.ucam.org
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH, resend] relatime: Let relatime update atime at least once per day

[Peter Moulder]

What was the fate of Ingo's previous patch[*1] to implement this (which,
incidentally, also included updates to documentation and made the interval
configurable as Éric Piel suggests) ?

[*1]: http://lwn.net/Articles/244384/


Just a minor nit, but the patch description (not included in the patch itself)
"at least once a day" isn't particularly accurate given that the patch enforces
at least a day [minus a second or so] has elapsed since the last update.  The
description in the above-referenced patch "only once a day" better captures the
situation, I think.

pjrm.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH, resend] relatime: Let relatime update atime at least once per day

[Matthew Garrett]

On Thu, Jan 08, 2009 at 11:29:53PM +1100, Peter Moulder wrote:
> What was the fate of Ingo's previous patch[*1] to implement this (which,
> incidentally, also included updates to documentation and made the interval
> configurable as Éric Piel suggests) ?

Nobody merged it and Ingo got bored of pushing it. I sent it earlier, 
but nobody came up with an especially good argument for the 
configurability and everyone seemed to want to paint the patch pink with 
yellow polka dots, so I gave up and just sent a minimal one in the hope 
that nobody could find anything to complain about.

> Just a minor nit, but the patch description (not included in the patch itself)
> "at least once a day" isn't particularly accurate given that the patch enforces
> at least a day [minus a second or so] has elapsed since the last update.  The
> description in the above-referenced patch "only once a day" better captures the
> situation, I think.

If mtime is changed then atime will be updated on the next access, so 
you can easily have more than one atime update a day.

-- 
Matthew Garrett | mjg59@srcf.ucam.org
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH, resend] relatime: Let relatime update atime at least once per day

[Sitsofe Wheeler]

Matthew Wilcox wrote:
> On Sun, Dec 28, 2008 at 10:24:55PM +0100, ?ric Piel wrote:
>> Yes, it might bring important drawbacks: performance-wise, relatime will
>> become more like atime, making it much less useful. There is also a
>> significant number of desktop computers that are turned on once a day,
>> the boot time may get hindered by those additional writes.
> 
> Huh?  Nobody's ever claimed that atime writes cost a significant amount
> of performance.  The problem that relatime is designed to solve is

I believe Ingo Molnar did - http://kerneltrap.org/node/14148 .