LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] fs: add some missing ctime updates
@ 2023-06-09 12:50 Jeff Layton
  2023-06-09 12:50 ` [PATCH 1/9] ibmvmc: update ctime in conjunction with mtime on write Jeff Layton
                   ` (9 more replies)
  0 siblings, 10 replies; 18+ messages in thread
From: Jeff Layton @ 2023-06-09 12:50 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Bob Peterson, Andreas Gruenbacher, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, John Johansen,
	Paul Moore, James Morris, Serge E. Hallyn, Ruihan Li,
	Suren Baghdasaryan, Sebastian Reichel, Wolfram Sang, linux-kernel,
	linux-usb, autofs, linux-efi, linux-fsdevel, cluster-devel,
	linux-cifs, samba-technical, apparmor, linux-security-module

While working on a patch series to change how we handle the ctime, I
found a number of places that update the mtime without a corresponding
ctime update. POSIX requires that when the mtime is updated that the
ctime also be updated.

Note that these are largely untested other than for compilation, so
please review carefully. These are a preliminary set for the upcoming
rework of how we handle the ctime.

None of these seem to be very crucial, but it would be nice if
various maintainers could pick these up for v6.5. Please let me know if
you do.

Jeff Layton (9):
  ibmvmc: update ctime in conjunction with mtime on write
  usb: update the ctime as well when updating mtime after an ioctl
  autofs: set ctime as well when mtime changes on a dir
  bfs: update ctime in addition to mtime when adding entries
  efivarfs: update ctime when mtime changes on a write
  exfat: ensure that ctime is updated whenever the mtime is
  gfs2: update ctime when quota is updated
  apparmor: update ctime whenever the mtime changes on an inode
  cifs: update the ctime on a partial page write

 drivers/misc/ibmvmc.c             |  2 +-
 drivers/usb/core/devio.c          | 16 ++++++++--------
 fs/autofs/root.c                  |  6 +++---
 fs/bfs/dir.c                      |  2 +-
 fs/efivarfs/file.c                |  2 +-
 fs/exfat/namei.c                  |  8 ++++----
 fs/gfs2/quota.c                   |  2 +-
 fs/smb/client/file.c              |  2 +-
 security/apparmor/apparmorfs.c    |  7 +++++--
 security/apparmor/policy_unpack.c | 11 +++++++----
 10 files changed, 32 insertions(+), 26 deletions(-)

-- 
2.40.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/9] ibmvmc: update ctime in conjunction with mtime on write
  2023-06-09 12:50 [PATCH 0/9] fs: add some missing ctime updates Jeff Layton
@ 2023-06-09 12:50 ` Jeff Layton
  2023-06-09 12:50 ` [PATCH 2/9] usb: update the ctime as well when updating mtime after an ioctl Jeff Layton
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Jeff Layton @ 2023-06-09 12:50 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Bob Peterson, Andreas Gruenbacher, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, John Johansen,
	Paul Moore, James Morris, Serge E. Hallyn, Ruihan Li,
	Suren Baghdasaryan, Sebastian Reichel, Wolfram Sang, linux-kernel,
	linux-usb, autofs, linux-efi, linux-fsdevel, cluster-devel,
	linux-cifs, samba-technical, apparmor, linux-security-module

When updating the mtime for a write, you must always update the ctime as
well.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 drivers/misc/ibmvmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/ibmvmc.c b/drivers/misc/ibmvmc.c
index cbaf6d35e854..d7c7f0305257 100644
--- a/drivers/misc/ibmvmc.c
+++ b/drivers/misc/ibmvmc.c
@@ -1124,7 +1124,7 @@ static ssize_t ibmvmc_write(struct file *file, const char *buffer,
 		goto out;
 
 	inode = file_inode(file);
-	inode->i_mtime = current_time(inode);
+	inode->i_mtime = inode->i_ctime = current_time(inode);
 	mark_inode_dirty(inode);
 
 	dev_dbg(adapter->dev, "write: file = 0x%lx, count = 0x%lx\n",
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/9] usb: update the ctime as well when updating mtime after an ioctl
  2023-06-09 12:50 [PATCH 0/9] fs: add some missing ctime updates Jeff Layton
  2023-06-09 12:50 ` [PATCH 1/9] ibmvmc: update ctime in conjunction with mtime on write Jeff Layton
@ 2023-06-09 12:50 ` Jeff Layton
  2023-06-09 13:10   ` Greg Kroah-Hartman
  2023-06-09 12:50 ` [PATCH 3/9] autofs: set ctime as well when mtime changes on a dir Jeff Layton
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Jeff Layton @ 2023-06-09 12:50 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Bob Peterson, Andreas Gruenbacher, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, John Johansen,
	Paul Moore, James Morris, Serge E. Hallyn, Ruihan Li,
	Suren Baghdasaryan, Sebastian Reichel, Wolfram Sang, linux-kernel,
	linux-usb, autofs, linux-efi, linux-fsdevel, cluster-devel,
	linux-cifs, samba-technical, apparmor, linux-security-module

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 drivers/usb/core/devio.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index fcf68818e999..1268d313a8df 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -2640,21 +2640,21 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd,
 		snoop(&dev->dev, "%s: CONTROL\n", __func__);
 		ret = proc_control(ps, p);
 		if (ret >= 0)
-			inode->i_mtime = current_time(inode);
+			inode->i_mtime = inode->i_ctime = current_time(inode);
 		break;
 
 	case USBDEVFS_BULK:
 		snoop(&dev->dev, "%s: BULK\n", __func__);
 		ret = proc_bulk(ps, p);
 		if (ret >= 0)
-			inode->i_mtime = current_time(inode);
+			inode->i_mtime = inode->i_ctime = current_time(inode);
 		break;
 
 	case USBDEVFS_RESETEP:
 		snoop(&dev->dev, "%s: RESETEP\n", __func__);
 		ret = proc_resetep(ps, p);
 		if (ret >= 0)
-			inode->i_mtime = current_time(inode);
+			inode->i_mtime = inode->i_ctime = current_time(inode);
 		break;
 
 	case USBDEVFS_RESET:
@@ -2666,7 +2666,7 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd,
 		snoop(&dev->dev, "%s: CLEAR_HALT\n", __func__);
 		ret = proc_clearhalt(ps, p);
 		if (ret >= 0)
-			inode->i_mtime = current_time(inode);
+			inode->i_mtime = inode->i_ctime = current_time(inode);
 		break;
 
 	case USBDEVFS_GETDRIVER:
@@ -2693,7 +2693,7 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd,
 		snoop(&dev->dev, "%s: SUBMITURB\n", __func__);
 		ret = proc_submiturb(ps, p);
 		if (ret >= 0)
-			inode->i_mtime = current_time(inode);
+			inode->i_mtime = inode->i_ctime = current_time(inode);
 		break;
 
 #ifdef CONFIG_COMPAT
@@ -2701,14 +2701,14 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd,
 		snoop(&dev->dev, "%s: CONTROL32\n", __func__);
 		ret = proc_control_compat(ps, p);
 		if (ret >= 0)
-			inode->i_mtime = current_time(inode);
+			inode->i_mtime = inode->i_ctime = current_time(inode);
 		break;
 
 	case USBDEVFS_BULK32:
 		snoop(&dev->dev, "%s: BULK32\n", __func__);
 		ret = proc_bulk_compat(ps, p);
 		if (ret >= 0)
-			inode->i_mtime = current_time(inode);
+			inode->i_mtime = inode->i_ctime = current_time(inode);
 		break;
 
 	case USBDEVFS_DISCSIGNAL32:
@@ -2720,7 +2720,7 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd,
 		snoop(&dev->dev, "%s: SUBMITURB32\n", __func__);
 		ret = proc_submiturb_compat(ps, p);
 		if (ret >= 0)
-			inode->i_mtime = current_time(inode);
+			inode->i_mtime = inode->i_ctime = current_time(inode);
 		break;
 
 	case USBDEVFS_IOCTL32:
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/9] autofs: set ctime as well when mtime changes on a dir
  2023-06-09 12:50 [PATCH 0/9] fs: add some missing ctime updates Jeff Layton
  2023-06-09 12:50 ` [PATCH 1/9] ibmvmc: update ctime in conjunction with mtime on write Jeff Layton
  2023-06-09 12:50 ` [PATCH 2/9] usb: update the ctime as well when updating mtime after an ioctl Jeff Layton
@ 2023-06-09 12:50 ` Jeff Layton
  2023-06-09 12:50 ` [PATCH 4/9] bfs: update ctime in addition to mtime when adding entries Jeff Layton
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Jeff Layton @ 2023-06-09 12:50 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Bob Peterson, Andreas Gruenbacher, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, John Johansen,
	Paul Moore, James Morris, Serge E. Hallyn, Ruihan Li,
	Suren Baghdasaryan, Sebastian Reichel, Wolfram Sang, linux-kernel,
	linux-usb, autofs, linux-efi, linux-fsdevel, cluster-devel,
	linux-cifs, samba-technical, apparmor, linux-security-module

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/autofs/root.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/autofs/root.c b/fs/autofs/root.c
index 6baf90b08e0e..93046c9dc461 100644
--- a/fs/autofs/root.c
+++ b/fs/autofs/root.c
@@ -600,7 +600,7 @@ static int autofs_dir_symlink(struct mnt_idmap *idmap,
 	p_ino = autofs_dentry_ino(dentry->d_parent);
 	p_ino->count++;
 
-	dir->i_mtime = current_time(dir);
+	dir->i_mtime = dir->i_ctime = current_time(dir);
 
 	return 0;
 }
@@ -633,7 +633,7 @@ static int autofs_dir_unlink(struct inode *dir, struct dentry *dentry)
 	d_inode(dentry)->i_size = 0;
 	clear_nlink(d_inode(dentry));
 
-	dir->i_mtime = current_time(dir);
+	dir->i_mtime = dir->i_ctime = current_time(dir);
 
 	spin_lock(&sbi->lookup_lock);
 	__autofs_add_expiring(dentry);
@@ -749,7 +749,7 @@ static int autofs_dir_mkdir(struct mnt_idmap *idmap,
 	p_ino = autofs_dentry_ino(dentry->d_parent);
 	p_ino->count++;
 	inc_nlink(dir);
-	dir->i_mtime = current_time(dir);
+	dir->i_mtime = dir->i_ctime = current_time(dir);
 
 	return 0;
 }
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/9] bfs: update ctime in addition to mtime when adding entries
  2023-06-09 12:50 [PATCH 0/9] fs: add some missing ctime updates Jeff Layton
                   ` (2 preceding siblings ...)
  2023-06-09 12:50 ` [PATCH 3/9] autofs: set ctime as well when mtime changes on a dir Jeff Layton
@ 2023-06-09 12:50 ` Jeff Layton
  2023-06-09 12:50 ` [PATCH 5/9] efivarfs: update ctime when mtime changes on a write Jeff Layton
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Jeff Layton @ 2023-06-09 12:50 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Bob Peterson, Andreas Gruenbacher, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, John Johansen,
	Paul Moore, James Morris, Serge E. Hallyn, Ruihan Li,
	Suren Baghdasaryan, Sebastian Reichel, Wolfram Sang, linux-kernel,
	linux-usb, autofs, linux-efi, linux-fsdevel, cluster-devel,
	linux-cifs, samba-technical, apparmor, linux-security-module

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/bfs/dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/bfs/dir.c b/fs/bfs/dir.c
index 040d5140e426..d2e8a2a56b05 100644
--- a/fs/bfs/dir.c
+++ b/fs/bfs/dir.c
@@ -294,7 +294,7 @@ static int bfs_add_entry(struct inode *dir, const struct qstr *child, int ino)
 					dir->i_size += BFS_DIRENT_SIZE;
 					dir->i_ctime = current_time(dir);
 				}
-				dir->i_mtime = current_time(dir);
+				dir->i_mtime = dir->i_ctime = current_time(dir);
 				mark_inode_dirty(dir);
 				de->ino = cpu_to_le16((u16)ino);
 				for (i = 0; i < BFS_NAMELEN; i++)
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/9] efivarfs: update ctime when mtime changes on a write
  2023-06-09 12:50 [PATCH 0/9] fs: add some missing ctime updates Jeff Layton
                   ` (3 preceding siblings ...)
  2023-06-09 12:50 ` [PATCH 4/9] bfs: update ctime in addition to mtime when adding entries Jeff Layton
@ 2023-06-09 12:50 ` Jeff Layton
  2023-06-09 12:50 ` [PATCH 6/9] exfat: ensure that ctime is updated whenever the mtime is Jeff Layton
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Jeff Layton @ 2023-06-09 12:50 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Bob Peterson, Andreas Gruenbacher, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, John Johansen,
	Paul Moore, James Morris, Serge E. Hallyn, Ruihan Li,
	Suren Baghdasaryan, Sebastian Reichel, Wolfram Sang, linux-kernel,
	linux-usb, autofs, linux-efi, linux-fsdevel, cluster-devel,
	linux-cifs, samba-technical, apparmor, linux-security-module

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/efivarfs/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/efivarfs/file.c b/fs/efivarfs/file.c
index d57ee15874f9..375576111dc3 100644
--- a/fs/efivarfs/file.c
+++ b/fs/efivarfs/file.c
@@ -51,7 +51,7 @@ static ssize_t efivarfs_file_write(struct file *file,
 	} else {
 		inode_lock(inode);
 		i_size_write(inode, datasize + sizeof(attributes));
-		inode->i_mtime = current_time(inode);
+		inode->i_mtime = inode->i_ctime = current_time(inode);
 		inode_unlock(inode);
 	}
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6/9] exfat: ensure that ctime is updated whenever the mtime is
  2023-06-09 12:50 [PATCH 0/9] fs: add some missing ctime updates Jeff Layton
                   ` (4 preceding siblings ...)
  2023-06-09 12:50 ` [PATCH 5/9] efivarfs: update ctime when mtime changes on a write Jeff Layton
@ 2023-06-09 12:50 ` Jeff Layton
  2023-06-09 12:50 ` [PATCH 7/9] gfs2: update ctime when quota is updated Jeff Layton
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Jeff Layton @ 2023-06-09 12:50 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Bob Peterson, Andreas Gruenbacher, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, John Johansen,
	Paul Moore, James Morris, Serge E. Hallyn, Ruihan Li,
	Suren Baghdasaryan, Sebastian Reichel, Wolfram Sang, linux-kernel,
	linux-usb, autofs, linux-efi, linux-fsdevel, cluster-devel,
	linux-cifs, samba-technical, apparmor, linux-security-module

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/exfat/namei.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index e0ff9d156f6f..d9b46fa36bff 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -817,7 +817,7 @@ static int exfat_unlink(struct inode *dir, struct dentry *dentry)
 	ei->dir.dir = DIR_DELETED;
 
 	inode_inc_iversion(dir);
-	dir->i_mtime = dir->i_atime = current_time(dir);
+	dir->i_mtime = dir->i_atime = dir->i_ctime = current_time(dir);
 	exfat_truncate_atime(&dir->i_atime);
 	if (IS_DIRSYNC(dir))
 		exfat_sync_inode(dir);
@@ -825,7 +825,7 @@ static int exfat_unlink(struct inode *dir, struct dentry *dentry)
 		mark_inode_dirty(dir);
 
 	clear_nlink(inode);
-	inode->i_mtime = inode->i_atime = current_time(inode);
+	inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
 	exfat_truncate_atime(&inode->i_atime);
 	exfat_unhash_inode(inode);
 	exfat_d_version_set(dentry, inode_query_iversion(dir));
@@ -979,7 +979,7 @@ static int exfat_rmdir(struct inode *dir, struct dentry *dentry)
 	ei->dir.dir = DIR_DELETED;
 
 	inode_inc_iversion(dir);
-	dir->i_mtime = dir->i_atime = current_time(dir);
+	dir->i_mtime = dir->i_atime = dir->i_ctime = current_time(dir);
 	exfat_truncate_atime(&dir->i_atime);
 	if (IS_DIRSYNC(dir))
 		exfat_sync_inode(dir);
@@ -988,7 +988,7 @@ static int exfat_rmdir(struct inode *dir, struct dentry *dentry)
 	drop_nlink(dir);
 
 	clear_nlink(inode);
-	inode->i_mtime = inode->i_atime = current_time(inode);
+	inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
 	exfat_truncate_atime(&inode->i_atime);
 	exfat_unhash_inode(inode);
 	exfat_d_version_set(dentry, inode_query_iversion(dir));
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 7/9] gfs2: update ctime when quota is updated
  2023-06-09 12:50 [PATCH 0/9] fs: add some missing ctime updates Jeff Layton
                   ` (5 preceding siblings ...)
  2023-06-09 12:50 ` [PATCH 6/9] exfat: ensure that ctime is updated whenever the mtime is Jeff Layton
@ 2023-06-09 12:50 ` Jeff Layton
  2023-06-09 16:44   ` Andreas Gruenbacher
  2023-06-09 12:50 ` [PATCH 8/9] apparmor: update ctime whenever the mtime changes on an inode Jeff Layton
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Jeff Layton @ 2023-06-09 12:50 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Bob Peterson, Andreas Gruenbacher, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, John Johansen,
	Paul Moore, James Morris, Serge E. Hallyn, Ruihan Li,
	Suren Baghdasaryan, Sebastian Reichel, Wolfram Sang, linux-kernel,
	linux-usb, autofs, linux-efi, linux-fsdevel, cluster-devel,
	linux-cifs, samba-technical, apparmor, linux-security-module

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/gfs2/quota.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 1ed17226d9ed..6d283e071b90 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -869,7 +869,7 @@ static int gfs2_adjust_quota(struct gfs2_inode *ip, loff_t loc,
 		size = loc + sizeof(struct gfs2_quota);
 		if (size > inode->i_size)
 			i_size_write(inode, size);
-		inode->i_mtime = inode->i_atime = current_time(inode);
+		inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
 		mark_inode_dirty(inode);
 		set_bit(QDF_REFRESH, &qd->qd_flags);
 	}
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 8/9] apparmor: update ctime whenever the mtime changes on an inode
  2023-06-09 12:50 [PATCH 0/9] fs: add some missing ctime updates Jeff Layton
                   ` (6 preceding siblings ...)
  2023-06-09 12:50 ` [PATCH 7/9] gfs2: update ctime when quota is updated Jeff Layton
@ 2023-06-09 12:50 ` Jeff Layton
  2023-06-09 12:50 ` [PATCH 9/9] cifs: update the ctime on a partial page write Jeff Layton
  2023-06-09 13:10 ` [PATCH 0/9] fs: add some missing ctime updates Greg Kroah-Hartman
  9 siblings, 0 replies; 18+ messages in thread
From: Jeff Layton @ 2023-06-09 12:50 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Bob Peterson, Andreas Gruenbacher, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, John Johansen,
	Paul Moore, James Morris, Serge E. Hallyn, Ruihan Li,
	Suren Baghdasaryan, Sebastian Reichel, Wolfram Sang, linux-kernel,
	linux-usb, autofs, linux-efi, linux-fsdevel, cluster-devel,
	linux-cifs, samba-technical, apparmor, linux-security-module

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 security/apparmor/apparmorfs.c    |  7 +++++--
 security/apparmor/policy_unpack.c | 11 +++++++----
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index db7a51acf9db..c06053718836 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -1554,8 +1554,11 @@ void __aafs_profile_migrate_dents(struct aa_profile *old,
 
 	for (i = 0; i < AAFS_PROF_SIZEOF; i++) {
 		new->dents[i] = old->dents[i];
-		if (new->dents[i])
-			new->dents[i]->d_inode->i_mtime = current_time(new->dents[i]->d_inode);
+		if (new->dents[i]) {
+			struct inode *inode = d_inode(new->dents[i]);
+
+			inode->i_mtime = inode->i_ctime = current_time(inode);
+		}
 		old->dents[i] = NULL;
 	}
 }
diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index cf2ceec40b28..48a97c1800b9 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -86,10 +86,13 @@ void __aa_loaddata_update(struct aa_loaddata *data, long revision)
 
 	data->revision = revision;
 	if ((data->dents[AAFS_LOADDATA_REVISION])) {
-		d_inode(data->dents[AAFS_LOADDATA_DIR])->i_mtime =
-			current_time(d_inode(data->dents[AAFS_LOADDATA_DIR]));
-		d_inode(data->dents[AAFS_LOADDATA_REVISION])->i_mtime =
-			current_time(d_inode(data->dents[AAFS_LOADDATA_REVISION]));
+		struct inode *inode;
+
+		inode = d_inode(data->dents[AAFS_LOADDATA_DIR]);
+		inode->i_mtime = inode->i_ctime = current_time(inode);
+
+		inode = d_inode(data->dents[AAFS_LOADDATA_REVISION]);
+		inode->i_mtime = inode->i_ctime = current_time(inode);
 	}
 }
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 9/9] cifs: update the ctime on a partial page write
  2023-06-09 12:50 [PATCH 0/9] fs: add some missing ctime updates Jeff Layton
                   ` (7 preceding siblings ...)
  2023-06-09 12:50 ` [PATCH 8/9] apparmor: update ctime whenever the mtime changes on an inode Jeff Layton
@ 2023-06-09 12:50 ` Jeff Layton
  2023-06-09 13:10 ` [PATCH 0/9] fs: add some missing ctime updates Greg Kroah-Hartman
  9 siblings, 0 replies; 18+ messages in thread
From: Jeff Layton @ 2023-06-09 12:50 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Bob Peterson, Andreas Gruenbacher, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, John Johansen,
	Paul Moore, James Morris, Serge E. Hallyn, Ruihan Li,
	Suren Baghdasaryan, Sebastian Reichel, Wolfram Sang, linux-kernel,
	linux-usb, autofs, linux-efi, linux-fsdevel, cluster-devel,
	linux-cifs, samba-technical, apparmor, linux-security-module

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/smb/client/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index df88b8c04d03..a00038a326cf 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -2596,7 +2596,7 @@ static int cifs_partialpagewrite(struct page *page, unsigned from, unsigned to)
 					   write_data, to - from, &offset);
 		cifsFileInfo_put(open_file);
 		/* Does mm or vfs already set times? */
-		inode->i_atime = inode->i_mtime = current_time(inode);
+		inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
 		if ((bytes_written > 0) && (offset))
 			rc = 0;
 		else if (bytes_written < 0)
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/9] usb: update the ctime as well when updating mtime after an ioctl
  2023-06-09 12:50 ` [PATCH 2/9] usb: update the ctime as well when updating mtime after an ioctl Jeff Layton
@ 2023-06-09 13:10   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 18+ messages in thread
From: Greg Kroah-Hartman @ 2023-06-09 13:10 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Ian Kent, Tigran A. Aivazian, Jeremy Kerr,
	Ard Biesheuvel, Namjae Jeon, Sungjong Seo, Bob Peterson,
	Andreas Gruenbacher, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, John Johansen,
	Paul Moore, James Morris, Serge E. Hallyn, Ruihan Li,
	Suren Baghdasaryan, Sebastian Reichel, Wolfram Sang, linux-kernel,
	linux-usb, autofs, linux-efi, linux-fsdevel, cluster-devel,
	linux-cifs, samba-technical, apparmor, linux-security-module

On Fri, Jun 09, 2023 at 08:50:16AM -0400, Jeff Layton wrote:
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

Sorry, but I can't take commits without any changelog text at all (nor
would you want me to...)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/9] fs: add some missing ctime updates
  2023-06-09 12:50 [PATCH 0/9] fs: add some missing ctime updates Jeff Layton
                   ` (8 preceding siblings ...)
  2023-06-09 12:50 ` [PATCH 9/9] cifs: update the ctime on a partial page write Jeff Layton
@ 2023-06-09 13:10 ` Greg Kroah-Hartman
  2023-06-09 13:27   ` Jeff Layton
  9 siblings, 1 reply; 18+ messages in thread
From: Greg Kroah-Hartman @ 2023-06-09 13:10 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Ian Kent, Tigran A. Aivazian, Jeremy Kerr,
	Ard Biesheuvel, Namjae Jeon, Sungjong Seo, Bob Peterson,
	Andreas Gruenbacher, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, John Johansen,
	Paul Moore, James Morris, Serge E. Hallyn, Ruihan Li,
	Suren Baghdasaryan, Sebastian Reichel, Wolfram Sang, linux-kernel,
	linux-usb, autofs, linux-efi, linux-fsdevel, cluster-devel,
	linux-cifs, samba-technical, apparmor, linux-security-module

On Fri, Jun 09, 2023 at 08:50:14AM -0400, Jeff Layton wrote:
> While working on a patch series to change how we handle the ctime, I
> found a number of places that update the mtime without a corresponding
> ctime update. POSIX requires that when the mtime is updated that the
> ctime also be updated.
> 
> Note that these are largely untested other than for compilation, so
> please review carefully. These are a preliminary set for the upcoming
> rework of how we handle the ctime.
> 
> None of these seem to be very crucial, but it would be nice if
> various maintainers could pick these up for v6.5. Please let me know if
> you do.
> 
> Jeff Layton (9):
>   ibmvmc: update ctime in conjunction with mtime on write
>   usb: update the ctime as well when updating mtime after an ioctl
>   autofs: set ctime as well when mtime changes on a dir
>   bfs: update ctime in addition to mtime when adding entries
>   efivarfs: update ctime when mtime changes on a write
>   exfat: ensure that ctime is updated whenever the mtime is
>   gfs2: update ctime when quota is updated
>   apparmor: update ctime whenever the mtime changes on an inode
>   cifs: update the ctime on a partial page write
> 
>  drivers/misc/ibmvmc.c             |  2 +-
>  drivers/usb/core/devio.c          | 16 ++++++++--------
>  fs/autofs/root.c                  |  6 +++---
>  fs/bfs/dir.c                      |  2 +-
>  fs/efivarfs/file.c                |  2 +-
>  fs/exfat/namei.c                  |  8 ++++----
>  fs/gfs2/quota.c                   |  2 +-
>  fs/smb/client/file.c              |  2 +-
>  security/apparmor/apparmorfs.c    |  7 +++++--
>  security/apparmor/policy_unpack.c | 11 +++++++----
>  10 files changed, 32 insertions(+), 26 deletions(-)
> 
> -- 
> 2.40.1
> 

All of these need commit log messages, didn't checkpatch warn you about
that?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/9] fs: add some missing ctime updates
  2023-06-09 13:10 ` [PATCH 0/9] fs: add some missing ctime updates Greg Kroah-Hartman
@ 2023-06-09 13:27   ` Jeff Layton
  0 siblings, 0 replies; 18+ messages in thread
From: Jeff Layton @ 2023-06-09 13:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Ian Kent, Tigran A. Aivazian, Jeremy Kerr,
	Ard Biesheuvel, Namjae Jeon, Sungjong Seo, Bob Peterson,
	Andreas Gruenbacher, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, John Johansen,
	Paul Moore, James Morris, Serge E. Hallyn, Ruihan Li,
	Suren Baghdasaryan, Sebastian Reichel, Wolfram Sang, linux-kernel,
	linux-usb, autofs, linux-efi, linux-fsdevel, cluster-devel,
	linux-cifs, samba-technical, apparmor, linux-security-module

On Fri, 2023-06-09 at 15:10 +0200, Greg Kroah-Hartman wrote:
> On Fri, Jun 09, 2023 at 08:50:14AM -0400, Jeff Layton wrote:
> > While working on a patch series to change how we handle the ctime, I
> > found a number of places that update the mtime without a corresponding
> > ctime update. POSIX requires that when the mtime is updated that the
> > ctime also be updated.
> > 
> > Note that these are largely untested other than for compilation, so
> > please review carefully. These are a preliminary set for the upcoming
> > rework of how we handle the ctime.
> > 
> > None of these seem to be very crucial, but it would be nice if
> > various maintainers could pick these up for v6.5. Please let me know if
> > you do.
> > 
> > Jeff Layton (9):
> >   ibmvmc: update ctime in conjunction with mtime on write
> >   usb: update the ctime as well when updating mtime after an ioctl
> >   autofs: set ctime as well when mtime changes on a dir
> >   bfs: update ctime in addition to mtime when adding entries
> >   efivarfs: update ctime when mtime changes on a write
> >   exfat: ensure that ctime is updated whenever the mtime is
> >   gfs2: update ctime when quota is updated
> >   apparmor: update ctime whenever the mtime changes on an inode
> >   cifs: update the ctime on a partial page write
> > 
> >  drivers/misc/ibmvmc.c             |  2 +-
> >  drivers/usb/core/devio.c          | 16 ++++++++--------
> >  fs/autofs/root.c                  |  6 +++---
> >  fs/bfs/dir.c                      |  2 +-
> >  fs/efivarfs/file.c                |  2 +-
> >  fs/exfat/namei.c                  |  8 ++++----
> >  fs/gfs2/quota.c                   |  2 +-
> >  fs/smb/client/file.c              |  2 +-
> >  security/apparmor/apparmorfs.c    |  7 +++++--
> >  security/apparmor/policy_unpack.c | 11 +++++++----
> >  10 files changed, 32 insertions(+), 26 deletions(-)
> > 
> > -- 
> > 2.40.1
> > 
> 
> All of these need commit log messages, didn't checkpatch warn you about
> that?

It did, once I ran it. ;)

I'll repost the set with more elaborate changelogs.
-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 7/9] gfs2: update ctime when quota is updated
  2023-06-09 12:50 ` [PATCH 7/9] gfs2: update ctime when quota is updated Jeff Layton
@ 2023-06-09 16:44   ` Andreas Gruenbacher
  2023-06-12 10:36     ` Jeff Layton
  0 siblings, 1 reply; 18+ messages in thread
From: Andreas Gruenbacher @ 2023-06-09 16:44 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Bob Peterson, Steve French, Paulo Alcantara, Ronnie Sahlberg,
	Shyam Prasad N, Tom Talpey, John Johansen, Paul Moore,
	James Morris, Serge E. Hallyn, Ruihan Li, Suren Baghdasaryan,
	Sebastian Reichel, Wolfram Sang, linux-kernel, linux-usb, autofs,
	linux-efi, linux-fsdevel, cluster-devel, linux-cifs,
	samba-technical, apparmor, linux-security-module

Jeff,

On Fri, Jun 9, 2023 at 2:50 PM Jeff Layton <jlayton@kernel.org> wrote:
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/gfs2/quota.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> index 1ed17226d9ed..6d283e071b90 100644
> --- a/fs/gfs2/quota.c
> +++ b/fs/gfs2/quota.c
> @@ -869,7 +869,7 @@ static int gfs2_adjust_quota(struct gfs2_inode *ip, loff_t loc,
>                 size = loc + sizeof(struct gfs2_quota);
>                 if (size > inode->i_size)
>                         i_size_write(inode, size);
> -               inode->i_mtime = inode->i_atime = current_time(inode);
> +               inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);

I don't think we need to worry about the ctime of the quota inode as
that inode is internal to the filesystem only.

>                 mark_inode_dirty(inode);
>                 set_bit(QDF_REFRESH, &qd->qd_flags);
>         }
> --
> 2.40.1
>

Thanks,
Andreas


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 7/9] gfs2: update ctime when quota is updated
  2023-06-09 16:44   ` Andreas Gruenbacher
@ 2023-06-12 10:36     ` Jeff Layton
  2023-07-05 20:25       ` Andreas Gruenbacher
  0 siblings, 1 reply; 18+ messages in thread
From: Jeff Layton @ 2023-06-12 10:36 UTC (permalink / raw)
  To: Andreas Gruenbacher
  Cc: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Bob Peterson, Steve French, Paulo Alcantara, Ronnie Sahlberg,
	Shyam Prasad N, Tom Talpey, John Johansen, Paul Moore,
	James Morris, Serge E. Hallyn, Ruihan Li, Suren Baghdasaryan,
	Sebastian Reichel, Wolfram Sang, linux-kernel, linux-usb, autofs,
	linux-efi, linux-fsdevel, cluster-devel, linux-cifs,
	samba-technical, apparmor, linux-security-module

On Fri, 2023-06-09 at 18:44 +0200, Andreas Gruenbacher wrote:
> Jeff,
> 
> On Fri, Jun 9, 2023 at 2:50 PM Jeff Layton <jlayton@kernel.org> wrote:
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/gfs2/quota.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> > index 1ed17226d9ed..6d283e071b90 100644
> > --- a/fs/gfs2/quota.c
> > +++ b/fs/gfs2/quota.c
> > @@ -869,7 +869,7 @@ static int gfs2_adjust_quota(struct gfs2_inode *ip, loff_t loc,
> >                 size = loc + sizeof(struct gfs2_quota);
> >                 if (size > inode->i_size)
> >                         i_size_write(inode, size);
> > -               inode->i_mtime = inode->i_atime = current_time(inode);
> > +               inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
> 
> I don't think we need to worry about the ctime of the quota inode as
> that inode is internal to the filesystem only.
> 

Thanks Andreas.  I'll plan to drop this patch from the series for now.

Does updating the mtime and atime here serve any purpose, or should
those also be removed? If you plan to keep the a/mtime updates then I'd
still suggest updating the ctime for consistency's sake. It shouldn't
cost anything extra to do so since you're dirtying the inode below
anyway.

Thanks!

> >                 mark_inode_dirty(inode);
> >                 set_bit(QDF_REFRESH, &qd->qd_flags);
> >         }
> > --
> > 2.40.1
> > 
> 
> Thanks,
> Andreas
> 

-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 7/9] gfs2: update ctime when quota is updated
  2023-06-12 10:36     ` Jeff Layton
@ 2023-07-05 20:25       ` Andreas Gruenbacher
  2023-07-05 21:48         ` Jeff Layton
  0 siblings, 1 reply; 18+ messages in thread
From: Andreas Gruenbacher @ 2023-07-05 20:25 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Bob Peterson, Steve French, Paulo Alcantara, Ronnie Sahlberg,
	Shyam Prasad N, Tom Talpey, John Johansen, Paul Moore,
	James Morris, Serge E. Hallyn, Ruihan Li, Suren Baghdasaryan,
	Sebastian Reichel, Wolfram Sang, linux-kernel, linux-usb, autofs,
	linux-efi, linux-fsdevel, cluster-devel, linux-cifs,
	samba-technical, apparmor, linux-security-module

On Mon, Jun 12, 2023 at 12:36 PM Jeff Layton <jlayton@kernel.org> wrote:
> On Fri, 2023-06-09 at 18:44 +0200, Andreas Gruenbacher wrote:
> > Jeff,
> >
> > On Fri, Jun 9, 2023 at 2:50 PM Jeff Layton <jlayton@kernel.org> wrote:
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > ---
> > >  fs/gfs2/quota.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> > > index 1ed17226d9ed..6d283e071b90 100644
> > > --- a/fs/gfs2/quota.c
> > > +++ b/fs/gfs2/quota.c
> > > @@ -869,7 +869,7 @@ static int gfs2_adjust_quota(struct gfs2_inode *ip, loff_t loc,
> > >                 size = loc + sizeof(struct gfs2_quota);
> > >                 if (size > inode->i_size)
> > >                         i_size_write(inode, size);
> > > -               inode->i_mtime = inode->i_atime = current_time(inode);
> > > +               inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
> >
> > I don't think we need to worry about the ctime of the quota inode as
> > that inode is internal to the filesystem only.
> >
>
> Thanks Andreas.  I'll plan to drop this patch from the series for now.
>
> Does updating the mtime and atime here serve any purpose, or should
> those also be removed? If you plan to keep the a/mtime updates then I'd
> still suggest updating the ctime for consistency's sake. It shouldn't
> cost anything extra to do so since you're dirtying the inode below
> anyway.

Yes, good point actually, we should keep things consistent for simplicity.

Would you add this back in if you do another posting?

Thanks,
Andreas

> Thanks!
>
> > >                 mark_inode_dirty(inode);
> > >                 set_bit(QDF_REFRESH, &qd->qd_flags);
> > >         }
> > > --
> > > 2.40.1
> > >
> >
> > Thanks,
> > Andreas
> >
>
> --
> Jeff Layton <jlayton@kernel.org>
>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 7/9] gfs2: update ctime when quota is updated
  2023-07-05 20:25       ` Andreas Gruenbacher
@ 2023-07-05 21:48         ` Jeff Layton
  2023-07-05 23:19           ` Andreas Grünbacher
  0 siblings, 1 reply; 18+ messages in thread
From: Jeff Layton @ 2023-07-05 21:48 UTC (permalink / raw)
  To: Andreas Gruenbacher
  Cc: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Bob Peterson, Steve French, Paulo Alcantara, Ronnie Sahlberg,
	Shyam Prasad N, Tom Talpey, John Johansen, Paul Moore,
	James Morris, Serge E. Hallyn, Ruihan Li, Suren Baghdasaryan,
	Sebastian Reichel, Wolfram Sang, linux-kernel, linux-usb, autofs,
	linux-efi, linux-fsdevel, cluster-devel, linux-cifs,
	samba-technical, apparmor, linux-security-module

On Wed, 2023-07-05 at 22:25 +0200, Andreas Gruenbacher wrote:
> On Mon, Jun 12, 2023 at 12:36 PM Jeff Layton <jlayton@kernel.org> wrote:
> > On Fri, 2023-06-09 at 18:44 +0200, Andreas Gruenbacher wrote:
> > > Jeff,
> > > 
> > > On Fri, Jun 9, 2023 at 2:50 PM Jeff Layton <jlayton@kernel.org> wrote:
> > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > ---
> > > >  fs/gfs2/quota.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> > > > index 1ed17226d9ed..6d283e071b90 100644
> > > > --- a/fs/gfs2/quota.c
> > > > +++ b/fs/gfs2/quota.c
> > > > @@ -869,7 +869,7 @@ static int gfs2_adjust_quota(struct gfs2_inode *ip, loff_t loc,
> > > >                 size = loc + sizeof(struct gfs2_quota);
> > > >                 if (size > inode->i_size)
> > > >                         i_size_write(inode, size);
> > > > -               inode->i_mtime = inode->i_atime = current_time(inode);
> > > > +               inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
> > > 
> > > I don't think we need to worry about the ctime of the quota inode as
> > > that inode is internal to the filesystem only.
> > > 
> > 
> > Thanks Andreas.  I'll plan to drop this patch from the series for now.
> > 
> > Does updating the mtime and atime here serve any purpose, or should
> > those also be removed? If you plan to keep the a/mtime updates then I'd
> > still suggest updating the ctime for consistency's sake. It shouldn't
> > cost anything extra to do so since you're dirtying the inode below
> > anyway.
> 
> Yes, good point actually, we should keep things consistent for simplicity.
> 
> Would you add this back in if you do another posting?
> 

I just re-posted the other patches in this as part of the ctime accessor
conversion. If I post again though, I can resurrect the gfs2 patch. If
not, we can do a follow-on fix later.

Since we're discussing it, it may be more correct to remove the atime
update there. gfs2_adjust_quota sounds like a "modify" operation, not a
"read", so I don't see a reason to update the atime.

In general, the only time you only want to set the atime, ctime and
mtime in lockstep is when the inode is brand new.
-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 7/9] gfs2: update ctime when quota is updated
  2023-07-05 21:48         ` Jeff Layton
@ 2023-07-05 23:19           ` Andreas Grünbacher
  0 siblings, 0 replies; 18+ messages in thread
From: Andreas Grünbacher @ 2023-07-05 23:19 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Andreas Gruenbacher, Christian Brauner, Al Viro, Brad Warrum,
	Ritu Agarwal, Arnd Bergmann, Greg Kroah-Hartman, Ian Kent,
	Tigran A. Aivazian, Jeremy Kerr, Ard Biesheuvel, Namjae Jeon,
	Sungjong Seo, Bob Peterson, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, John Johansen,
	Paul Moore, James Morris, Serge E. Hallyn, Ruihan Li,
	Suren Baghdasaryan, Sebastian Reichel, Wolfram Sang, linux-kernel,
	linux-usb, autofs, linux-efi, linux-fsdevel, cluster-devel,
	linux-cifs, samba-technical, apparmor, linux-security-module

Am Mi., 5. Juli 2023 um 23:51 Uhr schrieb Jeff Layton <jlayton@kernel.org>:
>
> On Wed, 2023-07-05 at 22:25 +0200, Andreas Gruenbacher wrote:
> > On Mon, Jun 12, 2023 at 12:36 PM Jeff Layton <jlayton@kernel.org> wrote:
> > > On Fri, 2023-06-09 at 18:44 +0200, Andreas Gruenbacher wrote:
> > > > Jeff,
> > > >
> > > > On Fri, Jun 9, 2023 at 2:50 PM Jeff Layton <jlayton@kernel.org> wrote:
> > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > > ---
> > > > >  fs/gfs2/quota.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> > > > > index 1ed17226d9ed..6d283e071b90 100644
> > > > > --- a/fs/gfs2/quota.c
> > > > > +++ b/fs/gfs2/quota.c
> > > > > @@ -869,7 +869,7 @@ static int gfs2_adjust_quota(struct gfs2_inode *ip, loff_t loc,
> > > > >                 size = loc + sizeof(struct gfs2_quota);
> > > > >                 if (size > inode->i_size)
> > > > >                         i_size_write(inode, size);
> > > > > -               inode->i_mtime = inode->i_atime = current_time(inode);
> > > > > +               inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
> > > >
> > > > I don't think we need to worry about the ctime of the quota inode as
> > > > that inode is internal to the filesystem only.
> > > >
> > >
> > > Thanks Andreas.  I'll plan to drop this patch from the series for now.
> > >
> > > Does updating the mtime and atime here serve any purpose, or should
> > > those also be removed? If you plan to keep the a/mtime updates then I'd
> > > still suggest updating the ctime for consistency's sake. It shouldn't
> > > cost anything extra to do so since you're dirtying the inode below
> > > anyway.
> >
> > Yes, good point actually, we should keep things consistent for simplicity.
> >
> > Would you add this back in if you do another posting?
> >
>
> I just re-posted the other patches in this as part of the ctime accessor
> conversion. If I post again though, I can resurrect the gfs2 patch. If
> not, we can do a follow-on fix later.

Sure, not a big deal.

> Since we're discussing it, it may be more correct to remove the atime
> update there. gfs2_adjust_quota sounds like a "modify" operation, not a
> "read", so I don't see a reason to update the atime.
>
> In general, the only time you only want to set the atime, ctime and
> mtime in lockstep is when the inode is brand new.

Yes, that makes sense, too.

Thanks,
Andreas

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2023-07-05 23:20 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-09 12:50 [PATCH 0/9] fs: add some missing ctime updates Jeff Layton
2023-06-09 12:50 ` [PATCH 1/9] ibmvmc: update ctime in conjunction with mtime on write Jeff Layton
2023-06-09 12:50 ` [PATCH 2/9] usb: update the ctime as well when updating mtime after an ioctl Jeff Layton
2023-06-09 13:10   ` Greg Kroah-Hartman
2023-06-09 12:50 ` [PATCH 3/9] autofs: set ctime as well when mtime changes on a dir Jeff Layton
2023-06-09 12:50 ` [PATCH 4/9] bfs: update ctime in addition to mtime when adding entries Jeff Layton
2023-06-09 12:50 ` [PATCH 5/9] efivarfs: update ctime when mtime changes on a write Jeff Layton
2023-06-09 12:50 ` [PATCH 6/9] exfat: ensure that ctime is updated whenever the mtime is Jeff Layton
2023-06-09 12:50 ` [PATCH 7/9] gfs2: update ctime when quota is updated Jeff Layton
2023-06-09 16:44   ` Andreas Gruenbacher
2023-06-12 10:36     ` Jeff Layton
2023-07-05 20:25       ` Andreas Gruenbacher
2023-07-05 21:48         ` Jeff Layton
2023-07-05 23:19           ` Andreas Grünbacher
2023-06-09 12:50 ` [PATCH 8/9] apparmor: update ctime whenever the mtime changes on an inode Jeff Layton
2023-06-09 12:50 ` [PATCH 9/9] cifs: update the ctime on a partial page write Jeff Layton
2023-06-09 13:10 ` [PATCH 0/9] fs: add some missing ctime updates Greg Kroah-Hartman
2023-06-09 13:27   ` Jeff Layton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).