FAILED: patch "[PATCH] erofs: derive atime instead of leaving it empty" failed to apply to 4.19-stable tree

PATCH In review [email protected] 2 participants 3 messages Nov 16, 2020

Subsystems (2)

Stable branch EROFS file system

lore

Share & tools

lore

[email protected]

author maintainer

2020-11-16 15:46 UTC #0 lore raw

The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From d3938ee23e97bfcac2e0eb6b356875da73d700df Mon Sep 17 00:00:00 2001
From: Gao Xiang <[email protected]>
Date: Sun, 1 Nov 2020 03:51:02 +0800
Subject: [PATCH] erofs: derive atime instead of leaving it empty

EROFS has _only one_ ondisk timestamp (ctime is currently documented and recorded, we might also record mtime instead with a new compat feature if needed) for each extended inode since EROFS isn't mainly for archival purposes so no need to keep all timestamps on disk especially for Android scenarios due to security concerns. Also, romfs/cramfs don't have their own on-disk timestamp, and squashfs only records mtime instead.

Let's also derive access time from ondisk timestamp rather than leaving it empty, and if mtime/atime for each file are really needed for specific scenarios as well, we can also use xattrs to record them then.

Link https://lore.kernel.org/r/[email protected] open here

[ Gao Xiang: It'd be better to backport for user-friendly concern. ]

Fixes 431339ba9042 ("staging: erofs: add inode operations")

Cc stable <[email protected]> # 4.19+

Reported-by nl6720 <[email protected]>

Reviewed-by Chao Yu <[email protected]>

Signed-off-by Gao Xiang <[email protected]>

fs/erofs/inode.c +11 ~~-10~~

index 139d0bed42f8..3e21c0e8adae 100644
@@ -107,11 +107,9 @@ static struct page *erofs_read_inode(struct inode *inode,
 		i_gid_write(inode, le32_to_cpu(die->i_gid));
 		set_nlink(inode, le32_to_cpu(die->i_nlink));
 
-		/* ns timestamp */
-		inode->i_mtime.tv_sec = inode->i_ctime.tv_sec =
-			le64_to_cpu(die->i_ctime);
-		inode->i_mtime.tv_nsec = inode->i_ctime.tv_nsec =
-			le32_to_cpu(die->i_ctime_nsec);
+		/* extended inode has its own timestamp */
+		inode->i_ctime.tv_sec = le64_to_cpu(die->i_ctime);
+		inode->i_ctime.tv_nsec = le32_to_cpu(die->i_ctime_nsec);
 
 		inode->i_size = le64_to_cpu(die->i_size);

@@ -149,11 +147,9 @@ static struct page *erofs_read_inode(struct inode *inode,
 		i_gid_write(inode, le16_to_cpu(dic->i_gid));
 		set_nlink(inode, le16_to_cpu(dic->i_nlink));
 
-		/* use build time to derive all file time */
-		inode->i_mtime.tv_sec = inode->i_ctime.tv_sec =
-			sbi->build_time;
-		inode->i_mtime.tv_nsec = inode->i_ctime.tv_nsec =
-			sbi->build_time_nsec;
+		/* use build time for compact inodes */
+		inode->i_ctime.tv_sec = sbi->build_time;
+		inode->i_ctime.tv_nsec = sbi->build_time_nsec;
 
 		inode->i_size = le32_to_cpu(dic->i_size);
 		if (erofs_inode_is_data_compressed(vi->datalayout))

@@ -167,6 +163,11 @@ static struct page *erofs_read_inode(struct inode *inode,
 		goto err_out;
 	}
 
+	inode->i_mtime.tv_sec = inode->i_ctime.tv_sec;
+	inode->i_atime.tv_sec = inode->i_ctime.tv_sec;
+	inode->i_mtime.tv_nsec = inode->i_ctime.tv_nsec;
+	inode->i_atime.tv_nsec = inode->i_ctime.tv_nsec;
+
 	if (!nblks)
 		/* measure inode.i_blocks as generic filesystems */
 		inode->i_blocks = roundup(inode->i_size, EROFS_BLKSIZ) >> 9;

Gao Xiang via Linux-erofs

2020-11-16 16:48 UTC #1 lore raw

PATCH 1/1 erofs: derive atime instead of leaving it empty

From: Gao Xiang <[email protected]>

commit d3938ee23e97bfcac2e0eb6b356875da73d700df upstream.

Link https://lore.kernel.org/r/[email protected] open here

[ Gao Xiang: It'd be better to backport for user-friendly concern. ]

Fixes 431339ba9042 ("staging: erofs: add inode operations")

Cc stable <[email protected]> # 4.19+

Reported-by nl6720 <[email protected]>

Reviewed-by Chao Yu <[email protected]>

[ Gao Xiang: Manually backport to 4.19.y due to trivial conflicts. ]

Signed-off-by Gao Xiang <[email protected]>

---
 drivers/staging/erofs/inode.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

drivers/staging/erofs/inode.c +11 ~~-10~~

index 7448744cc515..12a5be95457f 100644
@@ -53,11 +53,9 @@ static int read_inode(struct inode *inode, void *data)
 		i_gid_write(inode, le32_to_cpu(v2->i_gid));
 		set_nlink(inode, le32_to_cpu(v2->i_nlink));
 
-		/* ns timestamp */
-		inode->i_mtime.tv_sec = inode->i_ctime.tv_sec =
-			le64_to_cpu(v2->i_ctime);
-		inode->i_mtime.tv_nsec = inode->i_ctime.tv_nsec =
-			le32_to_cpu(v2->i_ctime_nsec);
+		/* extended inode has its own timestamp */
+		inode->i_ctime.tv_sec = le64_to_cpu(v2->i_ctime);
+		inode->i_ctime.tv_nsec = le32_to_cpu(v2->i_ctime_nsec);
 
 		inode->i_size = le64_to_cpu(v2->i_size);
 	} else if (__inode_version(advise) == EROFS_INODE_LAYOUT_V1) {

@@ -83,11 +81,9 @@ static int read_inode(struct inode *inode, void *data)
 		i_gid_write(inode, le16_to_cpu(v1->i_gid));
 		set_nlink(inode, le16_to_cpu(v1->i_nlink));
 
-		/* use build time to derive all file time */
-		inode->i_mtime.tv_sec = inode->i_ctime.tv_sec =
-			sbi->build_time;
-		inode->i_mtime.tv_nsec = inode->i_ctime.tv_nsec =
-			sbi->build_time_nsec;
+		/* use build time for compact inodes */
+		inode->i_ctime.tv_sec = sbi->build_time;
+		inode->i_ctime.tv_nsec = sbi->build_time_nsec;
 
 		inode->i_size = le32_to_cpu(v1->i_size);
 	} else {

@@ -97,6 +93,11 @@ static int read_inode(struct inode *inode, void *data)
 		return -EIO;
 	}
 
+	inode->i_mtime.tv_sec = inode->i_ctime.tv_sec;
+	inode->i_atime.tv_sec = inode->i_ctime.tv_sec;
+	inode->i_mtime.tv_nsec = inode->i_ctime.tv_nsec;
+	inode->i_atime.tv_nsec = inode->i_ctime.tv_nsec;
+
 	/* measure inode.i_blocks as the generic filesystem */
 	inode->i_blocks = ((inode->i_size - 1) >> 9) + 1;
 	return 0;

Signature · 1 line

2.24.0

Greg KH

author maintainer

2020-11-16 18:32 UTC #2 lore raw

Re: [PATCH 4.19.y] erofs: derive atime instead of leaving it empty

Quoted 25 lines · Gao Xiang

From: Gao Xiang <[email protected]>
commit d3938ee23e97bfcac2e0eb6b356875da73d700df upstream.
EROFS has _only one_ ondisk timestamp (ctime is currently documented and recorded, we might also record mtime instead with a new compat feature if needed) for each extended inode since EROFS isn't mainly for archival purposes so no need to keep all timestamps on disk especially for Android scenarios due to security concerns. Also, romfs/cramfs don't have their own on-disk timestamp, and squashfs only records mtime instead.
Let's also derive access time from ondisk timestamp rather than leaving it empty, and if mtime/atime for each file are really needed for specific scenarios as well, we can also use xattrs to record them then.
Link https://lore.kernel.org/r/[email protected] open here
[ Gao Xiang: It'd be better to backport for user-friendly concern. ]
Fixes 431339ba9042 ("staging: erofs: add inode operations")
Cc stable <[email protected]> # 4.19+
Reported-by nl6720 <[email protected]>
Reviewed-by Chao Yu <[email protected]>
[ Gao Xiang: Manually backport to 4.19.y due to trivial conflicts. ]
Signed-off-by Gao Xiang <[email protected]>

Quoted 3 lines

---
 drivers/staging/erofs/inode.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

Now queued up, thanks for the backport!

greg k-h