relatime: Let relatime update atime at least once per day

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 <[email protected]>

Reviewed-by Matthew Wilcox <[email protected]>

Acked-by Valerie Aurora Henson <[email protected]>

Acked-by Alan Cox <[email protected]>

Acked-by Ingo Molnar <[email protected]>

---

fs/inode.c +18 -0

index 0487ddb..057c92b 100644
@@ -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:

fs/inode.c +10 -9

+	/*
+	 * 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;
 

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?

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.

Matthew Garrett schreef:

Quoted 5 lines

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).

Quoted 6 lines

Matthew Garrett schreef:

Quoted 5 lines

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.

Quoted 6 lines

:
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.

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.

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).

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.

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) ?

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.