linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V3 0/4] fat: timestamp updates
@ 2018-09-22 20:19 Frank Sorenson
  2018-09-22 20:19 ` [PATCH 1/4] fat: set the s_time_gran for msdos or vfat mounts Frank Sorenson
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Frank Sorenson @ 2018-09-22 20:19 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: hirofumi

vfat/msdos timestamps are stored on-disk with several different
granularities, some of them lower resolution than timespec_trunc()
can provide.  In addition, they are only truncated as they are
written to disk, so the timestamps in-memory for new or modified
files/directories may be different from the same timestamps after
a remount, as the now-truncated times are re-read from the on-disk
format.

These patches allow finer granularity for the timestamps where
possible and add fat-specific ->update_time inode operation and
fat_truncate_time functions to truncate each timestamp correctly,
giving consistent times across remounts.


Frank Sorenson (4):
  fat: set the s_time_gran for msdos or vfat mounts
  fat: create function to calculate timezone offset
  fat: add functions to update and truncate the timestamps appropriately
  fat: change timestamp updates to fat_update_time or fat_truncate_time

 fs/fat/dir.c         |  2 +-
 fs/fat/fat.h         | 14 ++++++++++
 fs/fat/file.c        | 18 ++++++++++---
 fs/fat/inode.c       |  6 ++---
 fs/fat/misc.c        | 74 +++++++++++++++++++++++++++++++++++++++++++++-------
 fs/fat/namei_msdos.c | 17 ++++++------
 fs/fat/namei_vfat.c  | 15 ++++++-----
 7 files changed, 114 insertions(+), 32 deletions(-)

-- 
2.13.6

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/4] fat: set the s_time_gran for msdos or vfat mounts
  2018-09-22 20:19 [PATCH V3 0/4] fat: timestamp updates Frank Sorenson
@ 2018-09-22 20:19 ` Frank Sorenson
  2018-09-22 20:19 ` [PATCH 2/4] fat: create function to calculate timezone offset Frank Sorenson
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Frank Sorenson @ 2018-09-22 20:19 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: hirofumi

For vfat, ctime granularity is 10ms; set the super_block's
s_time_gran to 10ms for vfat and 1 second for msdos mounts.

Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/fat/inode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index d6b81e31f9f5..36071866a324 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -1626,6 +1626,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
 	sb->s_magic = MSDOS_SUPER_MAGIC;
 	sb->s_op = &fat_sops;
 	sb->s_export_op = &fat_export_ops;
+	sb->s_time_gran = isvfat ? 10000000 : 1000000000;
 	mutex_init(&sbi->nfs_build_inode_lock);
 	ratelimit_state_init(&sbi->ratelimit, DEFAULT_RATELIMIT_INTERVAL,
 			     DEFAULT_RATELIMIT_BURST);
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/4] fat: create function to calculate timezone offset
  2018-09-22 20:19 [PATCH V3 0/4] fat: timestamp updates Frank Sorenson
  2018-09-22 20:19 ` [PATCH 1/4] fat: set the s_time_gran for msdos or vfat mounts Frank Sorenson
@ 2018-09-22 20:19 ` Frank Sorenson
  2018-09-22 20:19 ` [PATCH 3/4] fat: add functions to update and truncate the timestamps appropriately Frank Sorenson
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Frank Sorenson @ 2018-09-22 20:19 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: hirofumi

Add a function to calculate the timezone offset in minutes.

Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/fat/fat.h  | 12 ++++++++++++
 fs/fat/misc.c | 12 ++----------
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index 9d7d2d5da28b..c40f7e69f078 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -412,6 +412,18 @@ void fat_msg(struct super_block *sb, const char *level, const char *fmt, ...);
 	 } while (0)
 extern int fat_clusters_flush(struct super_block *sb);
 extern int fat_chain_add(struct inode *inode, int new_dclus, int nr_cluster);
+
+#define SECS_PER_MIN    60
+#define SECS_PER_HOUR   (60 * 60)
+#define SECS_PER_DAY    (SECS_PER_HOUR * 24)
+
+static inline int fat_tz_offset(struct msdos_sb_info *sbi)
+{
+	return  (sbi->options.tz_set ?
+		- sbi->options.time_offset :
+		sys_tz.tz_minuteswest) * SECS_PER_MIN;
+}
+
 extern void fat_time_fat2unix(struct msdos_sb_info *sbi, struct timespec64 *ts,
 			      __le16 __time, __le16 __date, u8 time_cs);
 extern void fat_time_unix2fat(struct msdos_sb_info *sbi, struct timespec64 *ts,
diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index 573836dcaefc..58580c7e558e 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -170,9 +170,6 @@ int fat_chain_add(struct inode *inode, int new_dclus, int nr_cluster)
  * time:  5 - 10: min	(0 -  59)
  * time: 11 - 15: hour	(0 -  23)
  */
-#define SECS_PER_MIN	60
-#define SECS_PER_HOUR	(60 * 60)
-#define SECS_PER_DAY	(SECS_PER_HOUR * 24)
 /* days between 1.1.70 and 1.1.80 (2 leap days) */
 #define DAYS_DELTA	(365 * 10 + 2)
 /* 120 (2100 - 1980) isn't leap year */
@@ -210,10 +207,7 @@ void fat_time_fat2unix(struct msdos_sb_info *sbi, struct timespec64 *ts,
 		   + days_in_year[month] + day
 		   + DAYS_DELTA) * SECS_PER_DAY;
 
-	if (!sbi->options.tz_set)
-		second += sys_tz.tz_minuteswest * SECS_PER_MIN;
-	else
-		second -= sbi->options.time_offset * SECS_PER_MIN;
+	second += fat_tz_offset(sbi);
 
 	if (time_cs) {
 		ts->tv_sec = second + (time_cs / 100);
@@ -229,9 +223,7 @@ void fat_time_unix2fat(struct msdos_sb_info *sbi, struct timespec64 *ts,
 		       __le16 *time, __le16 *date, u8 *time_cs)
 {
 	struct tm tm;
-	time64_to_tm(ts->tv_sec,
-		   (sbi->options.tz_set ? sbi->options.time_offset :
-		   -sys_tz.tz_minuteswest) * SECS_PER_MIN, &tm);
+	time64_to_tm(ts->tv_sec, -fat_tz_offset(sbi), &tm);
 
 	/*  FAT can only support year between 1980 to 2107 */
 	if (tm.tm_year < 1980 - 1900) {
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 3/4] fat: add functions to update and truncate the timestamps appropriately
  2018-09-22 20:19 [PATCH V3 0/4] fat: timestamp updates Frank Sorenson
  2018-09-22 20:19 ` [PATCH 1/4] fat: set the s_time_gran for msdos or vfat mounts Frank Sorenson
  2018-09-22 20:19 ` [PATCH 2/4] fat: create function to calculate timezone offset Frank Sorenson
@ 2018-09-22 20:19 ` Frank Sorenson
  2018-09-22 20:19 ` [PATCH 4/4] fat: change timestamp updates to fat_update_time or fat_truncate_time Frank Sorenson
  2018-09-23 10:31 ` [PATCH V3 0/4] fat: timestamp updates OGAWA Hirofumi
  4 siblings, 0 replies; 6+ messages in thread
From: Frank Sorenson @ 2018-09-22 20:19 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: hirofumi

Add the fat-specific inode_operation ->update_time() and
fat_truncate_time() function to truncate the inode timestamps
with the appropriate granularity.

Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/fat/fat.h         |  2 ++
 fs/fat/file.c        |  1 +
 fs/fat/misc.c        | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/fat/namei_msdos.c |  1 +
 fs/fat/namei_vfat.c  |  1 +
 5 files changed, 67 insertions(+)

diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index c40f7e69f078..6201c6bdb5f9 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -428,6 +428,8 @@ extern void fat_time_fat2unix(struct msdos_sb_info *sbi, struct timespec64 *ts,
 			      __le16 __time, __le16 __date, u8 time_cs);
 extern void fat_time_unix2fat(struct msdos_sb_info *sbi, struct timespec64 *ts,
 			      __le16 *time, __le16 *date, u8 *time_cs);
+extern int fat_truncate_time(struct inode *inode, struct timespec64 *now, int flags);
+extern int fat_update_time(struct inode *inode, struct timespec64 *now, int flags);
 extern int fat_sync_bhs(struct buffer_head **bhs, int nr_bhs);
 
 int fat_cache_init(void);
diff --git a/fs/fat/file.c b/fs/fat/file.c
index 4f3d72fb1e60..19b6b0566411 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -552,4 +552,5 @@ EXPORT_SYMBOL_GPL(fat_setattr);
 const struct inode_operations fat_file_inode_operations = {
 	.setattr	= fat_setattr,
 	.getattr	= fat_getattr,
+	.update_time	= fat_update_time,
 };
diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index 58580c7e558e..42c4fb063287 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -255,6 +255,68 @@ void fat_time_unix2fat(struct msdos_sb_info *sbi, struct timespec64 *ts,
 }
 EXPORT_SYMBOL_GPL(fat_time_unix2fat);
 
+/*
+ * truncate the various times with appropriate granularity:
+ *   root inode:
+ *     all times always 0
+ *   all other inodes:
+ *     mtime - 2 seconds
+ *     ctime
+ *       msdos - 2 seconds
+ *       vfat  - 10 milliseconds
+ *     atime - 24 hours (00:00:00 in local timezone)
+ */
+noinline int fat_truncate_time(struct inode *inode, struct timespec64 *now, int flags)
+{
+	struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
+	struct timespec64 ts;
+
+	if (inode->i_ino == MSDOS_ROOT_INO)
+		return 0;
+
+	if (now == NULL) {
+		now = &ts;
+		ts = current_time(inode);
+	}
+
+	if (flags & S_ATIME) {
+		int offset = fat_tz_offset(sbi);
+		long seconds;
+
+		seconds = now->tv_sec;
+		seconds -= (now->tv_sec - offset) % 86400;
+
+		inode->i_atime = (struct timespec64){ seconds, 0 };
+	}
+	if (flags & S_CTIME) {
+		if (sbi->options.isvfat)
+			inode->i_ctime = *now;
+		else
+			inode->i_ctime = (struct timespec64){ now->tv_sec & ~1, 0 };
+	}
+	if (flags & S_MTIME)
+		inode->i_mtime = (struct timespec64){ now->tv_sec & ~1, 0 };
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fat_truncate_time);
+
+int fat_update_time(struct inode *inode, struct timespec64 *now, int flags)
+{
+	int iflags = I_DIRTY_TIME;
+
+	if (inode->i_ino == MSDOS_ROOT_INO)
+		return 0;
+	fat_truncate_time(inode, now, flags);
+
+	if ((flags & (S_ATIME | S_CTIME | S_MTIME)) &&
+	    !(inode->i_sb->s_flags & SB_LAZYTIME))
+		iflags |= I_DIRTY_SYNC;
+
+	__mark_inode_dirty(inode, iflags);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fat_update_time);
+
 int fat_sync_bhs(struct buffer_head **bhs, int nr_bhs)
 {
 	int i, err = 0;
diff --git a/fs/fat/namei_msdos.c b/fs/fat/namei_msdos.c
index efb8c40c9d27..effbdd5fbf6e 100644
--- a/fs/fat/namei_msdos.c
+++ b/fs/fat/namei_msdos.c
@@ -637,6 +637,7 @@ static const struct inode_operations msdos_dir_inode_operations = {
 	.rename		= msdos_rename,
 	.setattr	= fat_setattr,
 	.getattr	= fat_getattr,
+	.update_time	= fat_update_time,
 };
 
 static void setup(struct super_block *sb)
diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
index 82cd1e69cbdf..1daa57cf4bf3 100644
--- a/fs/fat/namei_vfat.c
+++ b/fs/fat/namei_vfat.c
@@ -1032,6 +1032,7 @@ static const struct inode_operations vfat_dir_inode_operations = {
 	.rename		= vfat_rename,
 	.setattr	= fat_setattr,
 	.getattr	= fat_getattr,
+	.update_time	= fat_update_time,
 };
 
 static void setup(struct super_block *sb)
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 4/4] fat: change timestamp updates to fat_update_time or fat_truncate_time
  2018-09-22 20:19 [PATCH V3 0/4] fat: timestamp updates Frank Sorenson
                   ` (2 preceding siblings ...)
  2018-09-22 20:19 ` [PATCH 3/4] fat: add functions to update and truncate the timestamps appropriately Frank Sorenson
@ 2018-09-22 20:19 ` Frank Sorenson
  2018-09-23 10:31 ` [PATCH V3 0/4] fat: timestamp updates OGAWA Hirofumi
  4 siblings, 0 replies; 6+ messages in thread
From: Frank Sorenson @ 2018-09-22 20:19 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: hirofumi

Make the inode timestamp updates use fat_update_time or
fat_truncate_time, depending on whether to call
mark_inode_dirty.

Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/fat/dir.c         |  2 +-
 fs/fat/file.c        | 17 ++++++++++++++---
 fs/fat/inode.c       |  5 ++---
 fs/fat/namei_msdos.c | 16 ++++++++--------
 fs/fat/namei_vfat.c  | 14 +++++++-------
 5 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 7f5f3699fc6c..1f9bad2c06c7 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -1071,7 +1071,7 @@ int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo)
 		}
 	}
 
-	dir->i_mtime = dir->i_atime = current_time(dir);
+	fat_truncate_time(dir, NULL, S_ATIME|S_MTIME);
 	if (IS_DIRSYNC(dir))
 		(void)fat_sync_inode(dir);
 	else
diff --git a/fs/fat/file.c b/fs/fat/file.c
index 19b6b0566411..cf0d5aea6e43 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -227,8 +227,7 @@ static int fat_cont_expand(struct inode *inode, loff_t size)
 	if (err)
 		goto out;
 
-	inode->i_ctime = inode->i_mtime = current_time(inode);
-	mark_inode_dirty(inode);
+	fat_update_time(inode, NULL, S_CTIME|S_MTIME);
 	if (IS_SYNC(inode)) {
 		int err2;
 
@@ -330,7 +329,7 @@ static int fat_free(struct inode *inode, int skip)
 		MSDOS_I(inode)->i_logstart = 0;
 	}
 	MSDOS_I(inode)->i_attrs |= ATTR_ARCH;
-	inode->i_ctime = inode->i_mtime = current_time(inode);
+	fat_truncate_time(inode, NULL, S_CTIME|S_MTIME);
 	if (wait) {
 		err = fat_sync_inode(inode);
 		if (err) {
@@ -542,6 +541,18 @@ int fat_setattr(struct dentry *dentry, struct iattr *attr)
 		up_write(&MSDOS_I(inode)->truncate_lock);
 	}
 
+	/*
+	 * setattr_copy can't truncate these appropriately, so we'll
+	 * copy them ourselves
+	 */
+	if (attr->ia_valid & ATTR_ATIME)
+		fat_truncate_time(inode, &attr->ia_atime, S_ATIME);
+	if (attr->ia_valid & ATTR_CTIME)
+		fat_truncate_time(inode, &attr->ia_ctime, S_CTIME);
+	if (attr->ia_valid & ATTR_MTIME)
+		fat_truncate_time(inode, &attr->ia_mtime, S_MTIME);
+	attr->ia_valid &= ~(ATTR_ATIME|ATTR_CTIME|ATTR_MTIME);
+
 	setattr_copy(inode, attr);
 	mark_inode_dirty(inode);
 out:
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 36071866a324..a803008d8100 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -244,9 +244,8 @@ static int fat_write_end(struct file *file, struct address_space *mapping,
 	if (err < len)
 		fat_write_failed(mapping, pos + len);
 	if (!(err < 0) && !(MSDOS_I(inode)->i_attrs & ATTR_ARCH)) {
-		inode->i_mtime = inode->i_ctime = current_time(inode);
 		MSDOS_I(inode)->i_attrs |= ATTR_ARCH;
-		mark_inode_dirty(inode);
+		fat_update_time(inode, NULL, S_CTIME|S_MTIME);
 	}
 	return err;
 }
@@ -564,7 +563,7 @@ int fat_fill_inode(struct inode *inode, struct msdos_dir_entry *de)
 				  de->cdate, de->ctime_cs);
 		fat_time_fat2unix(sbi, &inode->i_atime, 0, de->adate, 0);
 	} else
-		inode->i_ctime = inode->i_atime = inode->i_mtime;
+		fat_update_time(inode, &inode->i_mtime, S_ATIME|S_CTIME);
 
 	return 0;
 }
diff --git a/fs/fat/namei_msdos.c b/fs/fat/namei_msdos.c
index effbdd5fbf6e..6506290a3e99 100644
--- a/fs/fat/namei_msdos.c
+++ b/fs/fat/namei_msdos.c
@@ -250,7 +250,7 @@ static int msdos_add_entry(struct inode *dir, const unsigned char *name,
 	if (err)
 		return err;
 
-	dir->i_ctime = dir->i_mtime = *ts;
+	fat_truncate_time(dir, ts, S_CTIME|S_MTIME);
 	if (IS_DIRSYNC(dir))
 		(void)fat_sync_inode(dir);
 	else
@@ -294,7 +294,7 @@ static int msdos_create(struct inode *dir, struct dentry *dentry, umode_t mode,
 		err = PTR_ERR(inode);
 		goto out;
 	}
-	inode->i_mtime = inode->i_atime = inode->i_ctime = ts;
+	fat_truncate_time(inode, &ts, S_ATIME|S_CTIME|S_MTIME);
 	/* timestamp is already written, so mark_inode_dirty() is unneeded. */
 
 	d_instantiate(dentry, inode);
@@ -327,7 +327,7 @@ static int msdos_rmdir(struct inode *dir, struct dentry *dentry)
 	drop_nlink(dir);
 
 	clear_nlink(inode);
-	inode->i_ctime = current_time(inode);
+	fat_update_time(inode, NULL, S_CTIME);
 	fat_detach(inode);
 out:
 	mutex_unlock(&MSDOS_SB(sb)->s_lock);
@@ -380,7 +380,7 @@ static int msdos_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 		goto out;
 	}
 	set_nlink(inode, 2);
-	inode->i_mtime = inode->i_atime = inode->i_ctime = ts;
+	fat_truncate_time(inode, &ts, S_ATIME|S_CTIME|S_MTIME);
 	/* timestamp is already written, so mark_inode_dirty() is unneeded. */
 
 	d_instantiate(dentry, inode);
@@ -413,7 +413,7 @@ static int msdos_unlink(struct inode *dir, struct dentry *dentry)
 	if (err)
 		goto out;
 	clear_nlink(inode);
-	inode->i_ctime = current_time(inode);
+	fat_update_time(inode, NULL, S_CTIME);
 	fat_detach(inode);
 out:
 	mutex_unlock(&MSDOS_SB(sb)->s_lock);
@@ -478,7 +478,7 @@ static int do_msdos_rename(struct inode *old_dir, unsigned char *old_name,
 				mark_inode_dirty(old_inode);
 
 			inode_inc_iversion(old_dir);
-			old_dir->i_ctime = old_dir->i_mtime = current_time(old_dir);
+			fat_truncate_time(old_dir, NULL, S_CTIME|S_MTIME);
 			if (IS_DIRSYNC(old_dir))
 				(void)fat_sync_inode(old_dir);
 			else
@@ -538,7 +538,7 @@ static int do_msdos_rename(struct inode *old_dir, unsigned char *old_name,
 	if (err)
 		goto error_dotdot;
 	inode_inc_iversion(old_dir);
-	old_dir->i_ctime = old_dir->i_mtime = ts;
+	fat_truncate_time(old_dir, &ts, S_CTIME|S_MTIME);
 	if (IS_DIRSYNC(old_dir))
 		(void)fat_sync_inode(old_dir);
 	else
@@ -548,7 +548,7 @@ static int do_msdos_rename(struct inode *old_dir, unsigned char *old_name,
 		drop_nlink(new_inode);
 		if (is_dir)
 			drop_nlink(new_inode);
-		new_inode->i_ctime = ts;
+		fat_truncate_time(new_inode, &ts, S_CTIME);
 	}
 out:
 	brelse(sinfo.bh);
diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
index 1daa57cf4bf3..e384f2b7d088 100644
--- a/fs/fat/namei_vfat.c
+++ b/fs/fat/namei_vfat.c
@@ -678,7 +678,7 @@ static int vfat_add_entry(struct inode *dir, const struct qstr *qname,
 		goto cleanup;
 
 	/* update timestamp */
-	dir->i_ctime = dir->i_mtime = dir->i_atime = *ts;
+	fat_truncate_time(dir, ts, S_CTIME|S_MTIME);
 	if (IS_DIRSYNC(dir))
 		(void)fat_sync_inode(dir);
 	else
@@ -779,7 +779,7 @@ static int vfat_create(struct inode *dir, struct dentry *dentry, umode_t mode,
 		goto out;
 	}
 	inode_inc_iversion(inode);
-	inode->i_mtime = inode->i_atime = inode->i_ctime = ts;
+	fat_truncate_time(inode, &ts, S_ATIME|S_CTIME|S_MTIME);
 	/* timestamp is already written, so mark_inode_dirty() is unneeded. */
 
 	d_instantiate(dentry, inode);
@@ -810,7 +810,7 @@ static int vfat_rmdir(struct inode *dir, struct dentry *dentry)
 	drop_nlink(dir);
 
 	clear_nlink(inode);
-	inode->i_mtime = inode->i_atime = current_time(inode);
+	fat_update_time(inode, NULL, S_ATIME|S_MTIME);
 	fat_detach(inode);
 	vfat_d_version_set(dentry, inode_query_iversion(dir));
 out:
@@ -836,7 +836,7 @@ static int vfat_unlink(struct inode *dir, struct dentry *dentry)
 	if (err)
 		goto out;
 	clear_nlink(inode);
-	inode->i_mtime = inode->i_atime = current_time(inode);
+	fat_update_time(inode, NULL, S_ATIME|S_MTIME);
 	fat_detach(inode);
 	vfat_d_version_set(dentry, inode_query_iversion(dir));
 out:
@@ -876,7 +876,7 @@ static int vfat_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	}
 	inode_inc_iversion(inode);
 	set_nlink(inode, 2);
-	inode->i_mtime = inode->i_atime = inode->i_ctime = ts;
+	fat_truncate_time(inode, &ts, S_ATIME|S_CTIME|S_MTIME);
 	/* timestamp is already written, so mark_inode_dirty() is unneeded. */
 
 	d_instantiate(dentry, inode);
@@ -969,7 +969,7 @@ static int vfat_rename(struct inode *old_dir, struct dentry *old_dentry,
 	if (err)
 		goto error_dotdot;
 	inode_inc_iversion(old_dir);
-	old_dir->i_ctime = old_dir->i_mtime = ts;
+	fat_truncate_time(old_dir, &ts, S_CTIME|S_MTIME);
 	if (IS_DIRSYNC(old_dir))
 		(void)fat_sync_inode(old_dir);
 	else
@@ -979,7 +979,7 @@ static int vfat_rename(struct inode *old_dir, struct dentry *old_dentry,
 		drop_nlink(new_inode);
 		if (is_dir)
 			drop_nlink(new_inode);
-		new_inode->i_ctime = ts;
+		fat_truncate_time(new_inode, &ts, S_CTIME);
 	}
 out:
 	brelse(sinfo.bh);
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH V3 0/4] fat: timestamp updates
  2018-09-22 20:19 [PATCH V3 0/4] fat: timestamp updates Frank Sorenson
                   ` (3 preceding siblings ...)
  2018-09-22 20:19 ` [PATCH 4/4] fat: change timestamp updates to fat_update_time or fat_truncate_time Frank Sorenson
@ 2018-09-23 10:31 ` OGAWA Hirofumi
  4 siblings, 0 replies; 6+ messages in thread
From: OGAWA Hirofumi @ 2018-09-23 10:31 UTC (permalink / raw)
  To: Frank Sorenson; +Cc: linux-fsdevel

Frank Sorenson <sorenson@redhat.com> writes:

> vfat/msdos timestamps are stored on-disk with several different
> granularities, some of them lower resolution than timespec_trunc()
> can provide.  In addition, they are only truncated as they are
> written to disk, so the timestamps in-memory for new or modified
> files/directories may be different from the same timestamps after
> a remount, as the now-truncated times are re-read from the on-disk
> format.
>
> These patches allow finer granularity for the timestamps where
> possible and add fat-specific ->update_time inode operation and
> fat_truncate_time functions to truncate each timestamp correctly,
> giving consistent times across remounts.

Looks like basically very good, however there are some issues.  Those
small stuff would be easier to tell as patch, instead of commenting
inline.

Please check the following patch and comment of it. Only noticeable one
is atime bug (6 in comment), looks like it is only missed to test well,
and need to re-test (in my quick test, looks works with this patch).

Other stuff are tweaks and optimizations basically.

Thanks.


[PATCH] fat: Fix/cleanup FAT timestamp works


1) Move SECS_PER_* family to misc.c, only used in misc.c

2) Don't use fat_update_time() to replace mark_inode_dirty(),
mark_inode_dirty_sync() and mark_inode_dirty() are different.

3) Set sb->s_time_gran, we use this only for ctime in vfat mode. So
nothing benefit to add additional overhead to get current time.

Instead, just truncate ctime manually.

4) Add i_version support to update_time(). FAT doesn't support
i_version, but user still can set it.

5) Make sure to use 64bits wide to mask 2secs (i.e. ~1ULL, instead ~1)

6) Fix atime bug

    - on 32bit arch, have to use div64 stuff
    - missing to back to unix time after converting localtime

7) Remove needless noinline. gcc should be able to choice to inline it
or not, without any issue.

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 fs/fat/fat.h         |   12 ------------
 fs/fat/file.c        |    3 ++-
 fs/fat/inode.c       |   11 ++++++++---
 fs/fat/misc.c        |   45 ++++++++++++++++++++++++++++++++++-----------
 fs/fat/namei_msdos.c |    4 ++--
 fs/fat/namei_vfat.c  |    4 ++--
 6 files changed, 48 insertions(+), 31 deletions(-)

diff -puN fs/fat/fat.h~fat-timestamp-fix-tweaks fs/fat/fat.h
--- linux/fs/fat/fat.h~fat-timestamp-fix-tweaks	2018-09-23 19:00:41.573501694 +0900
+++ linux-hirofumi/fs/fat/fat.h	2018-09-23 19:16:10.576363930 +0900
@@ -412,18 +412,6 @@ void fat_msg(struct super_block *sb, con
 	 } while (0)
 extern int fat_clusters_flush(struct super_block *sb);
 extern int fat_chain_add(struct inode *inode, int new_dclus, int nr_cluster);
-
-#define SECS_PER_MIN    60
-#define SECS_PER_HOUR   (60 * 60)
-#define SECS_PER_DAY    (SECS_PER_HOUR * 24)
-
-static inline int fat_tz_offset(struct msdos_sb_info *sbi)
-{
-	return  (sbi->options.tz_set ?
-		- sbi->options.time_offset :
-		sys_tz.tz_minuteswest) * SECS_PER_MIN;
-}
-
 extern void fat_time_fat2unix(struct msdos_sb_info *sbi, struct timespec64 *ts,
 			      __le16 __time, __le16 __date, u8 time_cs);
 extern void fat_time_unix2fat(struct msdos_sb_info *sbi, struct timespec64 *ts,
diff -puN fs/fat/file.c~fat-timestamp-fix-tweaks fs/fat/file.c
--- linux/fs/fat/file.c~fat-timestamp-fix-tweaks	2018-09-23 19:00:41.574501693 +0900
+++ linux-hirofumi/fs/fat/file.c	2018-09-23 19:16:10.576363930 +0900
@@ -227,7 +227,8 @@ static int fat_cont_expand(struct inode
 	if (err)
 		goto out;
 
-	fat_update_time(inode, NULL, S_CTIME|S_MTIME);
+	fat_truncate_time(inode, NULL, S_CTIME|S_MTIME);
+	mark_inode_dirty(inode);
 	if (IS_SYNC(inode)) {
 		int err2;
 
diff -puN fs/fat/inode.c~fat-timestamp-fix-tweaks fs/fat/inode.c
--- linux/fs/fat/inode.c~fat-timestamp-fix-tweaks	2018-09-23 19:00:41.575501692 +0900
+++ linux-hirofumi/fs/fat/inode.c	2018-09-23 19:16:10.576363930 +0900
@@ -244,8 +244,9 @@ static int fat_write_end(struct file *fi
 	if (err < len)
 		fat_write_failed(mapping, pos + len);
 	if (!(err < 0) && !(MSDOS_I(inode)->i_attrs & ATTR_ARCH)) {
+		fat_truncate_time(inode, NULL, S_CTIME|S_MTIME);
 		MSDOS_I(inode)->i_attrs |= ATTR_ARCH;
-		fat_update_time(inode, NULL, S_CTIME|S_MTIME);
+		mark_inode_dirty(inode);
 	}
 	return err;
 }
@@ -563,7 +564,7 @@ int fat_fill_inode(struct inode *inode,
 				  de->cdate, de->ctime_cs);
 		fat_time_fat2unix(sbi, &inode->i_atime, 0, de->adate, 0);
 	} else
-		fat_update_time(inode, &inode->i_mtime, S_ATIME|S_CTIME);
+		fat_truncate_time(inode, &inode->i_mtime, S_ATIME|S_CTIME);
 
 	return 0;
 }
@@ -1625,7 +1626,11 @@ int fat_fill_super(struct super_block *s
 	sb->s_magic = MSDOS_SUPER_MAGIC;
 	sb->s_op = &fat_sops;
 	sb->s_export_op = &fat_export_ops;
-	sb->s_time_gran = isvfat ? 10000000 : 1000000000;
+	/*
+	 * timestamp is complex and truncated by fat itself, so we set
+	 * 1 here to be fast.
+	 */
+	sb->s_time_gran = 1;
 	mutex_init(&sbi->nfs_build_inode_lock);
 	ratelimit_state_init(&sbi->ratelimit, DEFAULT_RATELIMIT_INTERVAL,
 			     DEFAULT_RATELIMIT_BURST);
diff -puN fs/fat/misc.c~fat-timestamp-fix-tweaks fs/fat/misc.c
--- linux/fs/fat/misc.c~fat-timestamp-fix-tweaks	2018-09-23 19:00:41.576501690 +0900
+++ linux-hirofumi/fs/fat/misc.c	2018-09-23 19:16:10.542363973 +0900
@@ -7,6 +7,7 @@
  */
 
 #include "fat.h"
+#include <linux/iversion.h>
 
 /*
  * fat_fs_error reports a file system problem that might indicate fa data
@@ -170,6 +171,9 @@ int fat_chain_add(struct inode *inode, i
  * time:  5 - 10: min	(0 -  59)
  * time: 11 - 15: hour	(0 -  23)
  */
+#define SECS_PER_MIN	60
+#define SECS_PER_HOUR	(60 * 60)
+#define SECS_PER_DAY	(SECS_PER_HOUR * 24)
 /* days between 1.1.70 and 1.1.80 (2 leap days) */
 #define DAYS_DELTA	(365 * 10 + 2)
 /* 120 (2100 - 1980) isn't leap year */
@@ -182,6 +186,13 @@ static long days_in_year[] = {
 	0,   0,  31,  59,  90, 120, 151, 181, 212, 243, 273, 304, 334, 0, 0, 0,
 };
 
+static inline int fat_tz_offset(struct msdos_sb_info *sbi)
+{
+	return (sbi->options.tz_set ?
+		-sbi->options.time_offset :
+		sys_tz.tz_minuteswest) * SECS_PER_MIN;
+}
+
 /* Convert a FAT time/date pair to a UNIX date (seconds since 1 1 70). */
 void fat_time_fat2unix(struct msdos_sb_info *sbi, struct timespec64 *ts,
 		       __le16 __time, __le16 __date, u8 time_cs)
@@ -255,6 +266,11 @@ void fat_time_unix2fat(struct msdos_sb_i
 }
 EXPORT_SYMBOL_GPL(fat_time_unix2fat);
 
+static inline struct timespec64 fat_timespec64_trunc_2secs(struct timespec64 ts)
+{
+	return (struct timespec64){ ts.tv_sec & ~1ULL, 0 };
+}
+
 /*
  * truncate the various times with appropriate granularity:
  *   root inode:
@@ -266,7 +282,7 @@ EXPORT_SYMBOL_GPL(fat_time_unix2fat);
  *       vfat  - 10 milliseconds
  *     atime - 24 hours (00:00:00 in local timezone)
  */
-noinline int fat_truncate_time(struct inode *inode, struct timespec64 *now, int flags)
+int fat_truncate_time(struct inode *inode, struct timespec64 *now, int flags)
 {
 	struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
 	struct timespec64 ts;
@@ -280,22 +296,24 @@ noinline int fat_truncate_time(struct in
 	}
 
 	if (flags & S_ATIME) {
-		int offset = fat_tz_offset(sbi);
-		long seconds;
-
-		seconds = now->tv_sec;
-		seconds -= (now->tv_sec - offset) % 86400;
+		/* To localtime */
+		time64_t seconds = now->tv_sec - fat_tz_offset(sbi);
+		s32 remainder;
+		div_s64_rem(seconds, SECS_PER_DAY, &remainder);
+		/* To day boundary and back to unix time */
+		seconds = seconds + fat_tz_offset(sbi) - remainder;
 
 		inode->i_atime = (struct timespec64){ seconds, 0 };
 	}
 	if (flags & S_CTIME) {
 		if (sbi->options.isvfat)
-			inode->i_ctime = *now;
+			inode->i_ctime = timespec64_trunc(*now, 10000000);
 		else
-			inode->i_ctime = (struct timespec64){ now->tv_sec & ~1, 0 };
+			inode->i_ctime = fat_timespec64_trunc_2secs(*now);
 	}
 	if (flags & S_MTIME)
-		inode->i_mtime = (struct timespec64){ now->tv_sec & ~1, 0 };
+		inode->i_mtime = fat_timespec64_trunc_2secs(*now);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(fat_truncate_time);
@@ -303,15 +321,20 @@ EXPORT_SYMBOL_GPL(fat_truncate_time);
 int fat_update_time(struct inode *inode, struct timespec64 *now, int flags)
 {
 	int iflags = I_DIRTY_TIME;
+	bool dirty = false;
 
 	if (inode->i_ino == MSDOS_ROOT_INO)
 		return 0;
-	fat_truncate_time(inode, now, flags);
 
+	fat_truncate_time(inode, now, flags);
+	if (flags & S_VERSION)
+		dirty = inode_maybe_inc_iversion(inode, false);
 	if ((flags & (S_ATIME | S_CTIME | S_MTIME)) &&
 	    !(inode->i_sb->s_flags & SB_LAZYTIME))
-		iflags |= I_DIRTY_SYNC;
+		dirty = true;
 
+	if (dirty)
+		iflags |= I_DIRTY_SYNC;
 	__mark_inode_dirty(inode, iflags);
 	return 0;
 }
diff -puN fs/fat/namei_msdos.c~fat-timestamp-fix-tweaks fs/fat/namei_msdos.c
--- linux/fs/fat/namei_msdos.c~fat-timestamp-fix-tweaks	2018-09-23 19:00:41.576501690 +0900
+++ linux-hirofumi/fs/fat/namei_msdos.c	2018-09-23 19:16:10.492364035 +0900
@@ -327,7 +327,7 @@ static int msdos_rmdir(struct inode *dir
 	drop_nlink(dir);
 
 	clear_nlink(inode);
-	fat_update_time(inode, NULL, S_CTIME);
+	fat_truncate_time(inode, NULL, S_CTIME);
 	fat_detach(inode);
 out:
 	mutex_unlock(&MSDOS_SB(sb)->s_lock);
@@ -413,7 +413,7 @@ static int msdos_unlink(struct inode *di
 	if (err)
 		goto out;
 	clear_nlink(inode);
-	fat_update_time(inode, NULL, S_CTIME);
+	fat_truncate_time(inode, NULL, S_CTIME);
 	fat_detach(inode);
 out:
 	mutex_unlock(&MSDOS_SB(sb)->s_lock);
diff -puN fs/fat/namei_vfat.c~fat-timestamp-fix-tweaks fs/fat/namei_vfat.c
--- linux/fs/fat/namei_vfat.c~fat-timestamp-fix-tweaks	2018-09-23 19:00:41.577501689 +0900
+++ linux-hirofumi/fs/fat/namei_vfat.c	2018-09-23 19:16:10.492364035 +0900
@@ -810,7 +810,7 @@ static int vfat_rmdir(struct inode *dir,
 	drop_nlink(dir);
 
 	clear_nlink(inode);
-	fat_update_time(inode, NULL, S_ATIME|S_MTIME);
+	fat_truncate_time(inode, NULL, S_ATIME|S_MTIME);
 	fat_detach(inode);
 	vfat_d_version_set(dentry, inode_query_iversion(dir));
 out:
@@ -836,7 +836,7 @@ static int vfat_unlink(struct inode *dir
 	if (err)
 		goto out;
 	clear_nlink(inode);
-	fat_update_time(inode, NULL, S_ATIME|S_MTIME);
+	fat_truncate_time(inode, NULL, S_ATIME|S_MTIME);
 	fat_detach(inode);
 	vfat_d_version_set(dentry, inode_query_iversion(dir));
 out:
_

-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-09-23 16:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-22 20:19 [PATCH V3 0/4] fat: timestamp updates Frank Sorenson
2018-09-22 20:19 ` [PATCH 1/4] fat: set the s_time_gran for msdos or vfat mounts Frank Sorenson
2018-09-22 20:19 ` [PATCH 2/4] fat: create function to calculate timezone offset Frank Sorenson
2018-09-22 20:19 ` [PATCH 3/4] fat: add functions to update and truncate the timestamps appropriately Frank Sorenson
2018-09-22 20:19 ` [PATCH 4/4] fat: change timestamp updates to fat_update_time or fat_truncate_time Frank Sorenson
2018-09-23 10:31 ` [PATCH V3 0/4] fat: timestamp updates OGAWA Hirofumi

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 NNTP newsgroup(s).