[PATCH v4 1/3] fat: split fat_truncate_time() into separate functions

[PATCH v4 1/3] fat: split fat_truncate_time() into separate functions

[Chung-Chiang Cheng]

Separate fat_truncate_time() to each timestamps for later creation time
work.

This patch does not introduce any functional changes, it's merely
refactoring change.

Signed-off-by: Chung-Chiang Cheng <cccheng@synology.com>
---
 fs/fat/fat.h  |  6 +++++
 fs/fat/misc.c | 72 ++++++++++++++++++++++++++++++++-------------------
 2 files changed, 52 insertions(+), 26 deletions(-)

diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index 02d4d4234956..508b4f2a1ffb 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -446,6 +446,12 @@ 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 void fat_truncate_atime(struct msdos_sb_info *sbi, struct timespec64 *ts,
+			       struct timespec64 *atime);
+extern void fat_truncate_crtime(struct msdos_sb_info *sbi, struct timespec64 *ts,
+				struct timespec64 *crtime);
+extern void fat_truncate_mtime(struct msdos_sb_info *sbi, struct timespec64 *ts,
+			       struct timespec64 *mtime);
 extern int fat_truncate_time(struct inode *inode, struct timespec64 *now,
 			     int flags);
 extern int fat_update_time(struct inode *inode, struct timespec64 *now,
diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index 91ca3c304211..c87df64f8b2b 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -282,16 +282,49 @@ static inline struct timespec64 fat_timespec64_trunc_10ms(struct timespec64 ts)
 	return ts;
 }
 
+/*
+ * truncate atime to 24 hour granularity (00:00:00 in local timezone)
+ */
+void fat_truncate_atime(struct msdos_sb_info *sbi, struct timespec64 *ts,
+			struct timespec64 *atime)
+{
+	/* to localtime */
+	time64_t seconds = ts->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;
+
+	*atime = (struct timespec64){ seconds, 0 };
+}
+
+/*
+ * truncate creation time with appropriate granularity:
+ *   msdos - 2 seconds
+ *   vfat  - 10 milliseconds
+ */
+void fat_truncate_crtime(struct msdos_sb_info *sbi, struct timespec64 *ts,
+			 struct timespec64 *crtime)
+{
+	if (sbi->options.isvfat)
+		*crtime = fat_timespec64_trunc_10ms(*ts);
+	else
+		*crtime = fat_timespec64_trunc_2secs(*ts);
+}
+
+/*
+ * truncate mtime to 2 second granularity
+ */
+void fat_truncate_mtime(struct msdos_sb_info *sbi, struct timespec64 *ts,
+			struct timespec64 *mtime)
+{
+	*mtime = fat_timespec64_trunc_2secs(*ts);
+}
+
 /*
  * 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)
+ *   all times in root node are always 0
  */
 int fat_truncate_time(struct inode *inode, struct timespec64 *now, int flags)
 {
@@ -306,25 +339,12 @@ int fat_truncate_time(struct inode *inode, struct timespec64 *now, int flags)
 		ts = current_time(inode);
 	}
 
-	if (flags & S_ATIME) {
-		/* 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 = fat_timespec64_trunc_10ms(*now);
-		else
-			inode->i_ctime = fat_timespec64_trunc_2secs(*now);
-	}
+	if (flags & S_ATIME)
+		fat_truncate_atime(sbi, now, &inode->i_atime);
+	if (flags & S_CTIME)
+		fat_truncate_crtime(sbi, now, &inode->i_ctime);
 	if (flags & S_MTIME)
-		inode->i_mtime = fat_timespec64_trunc_2secs(*now);
+		fat_truncate_mtime(sbi, now, &inode->i_mtime);
 
 	return 0;
 }
-- 
2.34.1

[PATCH v4 2/3] fat: ignore ctime updates, and keep ctime identical to mtime in memory

[Chung-Chiang Cheng]

FAT supports creation time but not change time, and there was no
corresponding timestamp for creation time in previous VFS. The
original implementation took the compromise of saving the in-memory
change time into the on-disk creation time field, but this would lead
to compatibility issues with non-linux systems.

To address this issue, this patch changes the behavior of ctime. It
will no longer be loaded and stored from the creation time on disk.
Instead of that, it'll be consistent with the in-memory mtime and share
the same on-disk field. All updates to mtime will also be applied to
ctime in memory, while all updates to ctime will be ignored.

Signed-off-by: Chung-Chiang Cheng <cccheng@synology.com>
---
 fs/fat/inode.c | 11 ++++-------
 fs/fat/misc.c  | 12 +++++++++---
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index bf6051bdf1d1..f2ac55cd4ea4 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -567,12 +567,11 @@ int fat_fill_inode(struct inode *inode, struct msdos_dir_entry *de)
 			   & ~((loff_t)sbi->cluster_size - 1)) >> 9;
 
 	fat_time_fat2unix(sbi, &inode->i_mtime, de->time, de->date, 0);
-	if (sbi->options.isvfat) {
-		fat_time_fat2unix(sbi, &inode->i_ctime, de->ctime,
-				  de->cdate, de->ctime_cs);
+	inode->i_ctime = inode->i_mtime;
+	if (sbi->options.isvfat)
 		fat_time_fat2unix(sbi, &inode->i_atime, 0, de->adate, 0);
-	} else
-		fat_truncate_time(inode, &inode->i_mtime, S_ATIME|S_CTIME);
+	else
+		fat_truncate_atime(sbi, &inode->i_mtime, &inode->i_atime);
 
 	return 0;
 }
@@ -888,8 +887,6 @@ static int __fat_write_inode(struct inode *inode, int wait)
 			  &raw_entry->date, NULL);
 	if (sbi->options.isvfat) {
 		__le16 atime;
-		fat_time_unix2fat(sbi, &inode->i_ctime, &raw_entry->ctime,
-				  &raw_entry->cdate, &raw_entry->ctime_cs);
 		fat_time_unix2fat(sbi, &inode->i_atime, &atime,
 				  &raw_entry->adate, NULL);
 	}
diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index c87df64f8b2b..ef09b6361602 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -341,10 +341,16 @@ int fat_truncate_time(struct inode *inode, struct timespec64 *now, int flags)
 
 	if (flags & S_ATIME)
 		fat_truncate_atime(sbi, now, &inode->i_atime);
-	if (flags & S_CTIME)
-		fat_truncate_crtime(sbi, now, &inode->i_ctime);
-	if (flags & S_MTIME)
+
+	/*
+	 * ctime and mtime share the same on-disk field, and should be
+	 * identical in memory. all mtime updates will be applied to ctime,
+	 * but ctime updates are ignored.
+	 */
+	if (flags & S_MTIME) {
 		fat_truncate_mtime(sbi, now, &inode->i_mtime);
+		inode->i_ctime = inode->i_mtime;
+	}
 
 	return 0;
 }
-- 
2.34.1

[PATCH v4 3/3] fat: report creation time in statx

[Chung-Chiang Cheng]

creation time is no longer mixed with change time. Add an in-memory
field for it, and report it in statx if supported.

The unused code in fat_truncate_crtime() is also removed because only
vfat support creation time.

Signed-off-by: Chung-Chiang Cheng <cccheng@synology.com>
---
 fs/fat/fat.h        |  1 +
 fs/fat/file.c       | 14 +++++++++++---
 fs/fat/inode.c      | 10 ++++++++--
 fs/fat/misc.c       |  3 +--
 fs/fat/namei_vfat.c |  1 +
 5 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index 508b4f2a1ffb..e4409ee82ea9 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -126,6 +126,7 @@ struct msdos_inode_info {
 	struct hlist_node i_fat_hash;	/* hash by i_location */
 	struct hlist_node i_dir_hash;	/* hash by i_logstart */
 	struct rw_semaphore truncate_lock; /* protect bmap against truncate */
+	struct timespec64 i_crtime;	/* File creation (birth) time */
 	struct inode vfs_inode;
 };
 
diff --git a/fs/fat/file.c b/fs/fat/file.c
index a5a309fcc7fa..8f5218450a3a 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -399,13 +399,21 @@ int fat_getattr(struct user_namespace *mnt_userns, const struct path *path,
 		struct kstat *stat, u32 request_mask, unsigned int flags)
 {
 	struct inode *inode = d_inode(path->dentry);
+	struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
+
 	generic_fillattr(mnt_userns, inode, stat);
-	stat->blksize = MSDOS_SB(inode->i_sb)->cluster_size;
+	stat->blksize = sbi->cluster_size;
 
-	if (MSDOS_SB(inode->i_sb)->options.nfs == FAT_NFS_NOSTALE_RO) {
+	if (sbi->options.nfs == FAT_NFS_NOSTALE_RO) {
 		/* Use i_pos for ino. This is used as fileid of nfs. */
-		stat->ino = fat_i_pos_read(MSDOS_SB(inode->i_sb), inode);
+		stat->ino = fat_i_pos_read(sbi, inode);
 	}
+
+	if (sbi->options.isvfat && request_mask & STATX_BTIME) {
+		stat->result_mask |= STATX_BTIME;
+		stat->btime = MSDOS_I(inode)->i_crtime;
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(fat_getattr);
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index f2ac55cd4ea4..ebc124f44e86 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -568,9 +568,11 @@ int fat_fill_inode(struct inode *inode, struct msdos_dir_entry *de)
 
 	fat_time_fat2unix(sbi, &inode->i_mtime, de->time, de->date, 0);
 	inode->i_ctime = inode->i_mtime;
-	if (sbi->options.isvfat)
+	if (sbi->options.isvfat) {
 		fat_time_fat2unix(sbi, &inode->i_atime, 0, de->adate, 0);
-	else
+		fat_time_fat2unix(sbi, &MSDOS_I(inode)->i_crtime, de->ctime,
+				  de->cdate, de->ctime_cs);
+	} else
 		fat_truncate_atime(sbi, &inode->i_mtime, &inode->i_atime);
 
 	return 0;
@@ -756,6 +758,8 @@ static struct inode *fat_alloc_inode(struct super_block *sb)
 	ei->i_logstart = 0;
 	ei->i_attrs = 0;
 	ei->i_pos = 0;
+	ei->i_crtime.tv_sec = 0;
+	ei->i_crtime.tv_nsec = 0;
 
 	return &ei->vfs_inode;
 }
@@ -887,6 +891,8 @@ static int __fat_write_inode(struct inode *inode, int wait)
 			  &raw_entry->date, NULL);
 	if (sbi->options.isvfat) {
 		__le16 atime;
+		fat_time_unix2fat(sbi, &MSDOS_I(inode)->i_crtime, &raw_entry->ctime,
+				  &raw_entry->cdate, &raw_entry->ctime_cs);
 		fat_time_unix2fat(sbi, &inode->i_atime, &atime,
 				  &raw_entry->adate, NULL);
 	}
diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index ef09b6361602..dfc5d6df3519 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -309,9 +309,8 @@ void fat_truncate_crtime(struct msdos_sb_info *sbi, struct timespec64 *ts,
 {
 	if (sbi->options.isvfat)
 		*crtime = fat_timespec64_trunc_10ms(*ts);
-	else
-		*crtime = fat_timespec64_trunc_2secs(*ts);
 }
+EXPORT_SYMBOL_GPL(fat_truncate_crtime);
 
 /*
  * truncate mtime to 2 second granularity
diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
index 5369d82e0bfb..9187979fed5d 100644
--- a/fs/fat/namei_vfat.c
+++ b/fs/fat/namei_vfat.c
@@ -781,6 +781,7 @@ static int vfat_create(struct user_namespace *mnt_userns, struct inode *dir,
 	}
 	inode_inc_iversion(inode);
 	fat_truncate_time(inode, &ts, S_ATIME|S_CTIME|S_MTIME);
+	fat_truncate_crtime(MSDOS_SB(sb), &MSDOS_I(inode)->i_crtime, &MSDOS_I(inode)->i_crtime);
 	/* timestamp is already written, so mark_inode_dirty() is unneeded. */
 
 	d_instantiate(dentry, inode);
-- 
2.34.1

Re: [PATCH v4 3/3] fat: report creation time in statx

[OGAWA Hirofumi]

Chung-Chiang Cheng <cccheng@synology.com> writes:

> diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
> index 5369d82e0bfb..9187979fed5d 100644
> --- a/fs/fat/namei_vfat.c
> +++ b/fs/fat/namei_vfat.c
> @@ -781,6 +781,7 @@ static int vfat_create(struct user_namespace *mnt_userns, struct inode *dir,
>  	}
>  	inode_inc_iversion(inode);
>  	fat_truncate_time(inode, &ts, S_ATIME|S_CTIME|S_MTIME);
> +	fat_truncate_crtime(MSDOS_SB(sb), &MSDOS_I(inode)->i_crtime, &MSDOS_I(inode)->i_crtime);
>  	/* timestamp is already written, so mark_inode_dirty() is unneeded. */
>  
>  	d_instantiate(dentry, inode);

Probably, above should be the follow line?

	fat_truncate_crtime(MSDOS_SB(sb), &ts, &MSDOS_I(inode)->i_crtime);

And furthermore, this is missing to add it to mkdir(2)? And another one,
we would have to update vfat_build_slots() for crtime? I'm not checking
fully though, this seems to need isvfat test

	fat_time_unix2fat(sbi, ts, &time, &date, &time_cs);
	de->time = de->ctime = time;
	de->date = de->cdate = de->adate = date;
	de->ctime_cs = time_cs;

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

Re: [PATCH v4 1/3] fat: split fat_truncate_time() into separate functions

[OGAWA Hirofumi]

Chung-Chiang Cheng <cccheng@synology.com> writes:

> +/*
> + * truncate atime to 24 hour granularity (00:00:00 in local timezone)
> + */
> +void fat_truncate_atime(struct msdos_sb_info *sbi, struct timespec64 *ts,
> +			struct timespec64 *atime)

[...]

> +void fat_truncate_crtime(struct msdos_sb_info *sbi, struct timespec64 *ts,
> +			 struct timespec64 *crtime)

[...]

> +void fat_truncate_mtime(struct msdos_sb_info *sbi, struct timespec64 *ts,
> +			struct timespec64 *mtime)

Small stuff and not strong opinion though, those are better to return
timespec64, instead of taking pointer? Because we can

	mtime = ctime = fat_truncate_mtime(sbi, &ts);

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

Re: [PATCH v4 1/3] fat: split fat_truncate_time() into separate functions

[Chung-Chiang Cheng]

> > +void fat_truncate_atime(struct msdos_sb_info *sbi, struct timespec64 *ts,
> > +                     struct timespec64 *atime)
> > +void fat_truncate_crtime(struct msdos_sb_info *sbi, struct timespec64 *ts,
> > +                      struct timespec64 *crtime)
> > +void fat_truncate_mtime(struct msdos_sb_info *sbi, struct timespec64 *ts,
> > +                     struct timespec64 *mtime)
>
> Small stuff and not strong opinion though, those are better to return
> timespec64, instead of taking pointer? Because we can
>
>         mtime = ctime = fat_truncate_mtime(sbi, &ts);

It's ok. I can change them to this style.

Thanks.

Re: [PATCH v4 3/3] fat: report creation time in statx

[Chung-Chiang Cheng]

On Thu, Apr 28, 2022 at 1:45 AM OGAWA Hirofumi
<hirofumi@mail.parknet.co.jp> wrote:
> Probably, above should be the follow line?
>
>         fat_truncate_crtime(MSDOS_SB(sb), &ts, &MSDOS_I(inode)->i_crtime);
>
> And furthermore, this is missing to add it to mkdir(2)? And another one,
> we would have to update vfat_build_slots() for crtime? I'm not checking
> fully though, this seems to need isvfat test
>
>         fat_time_unix2fat(sbi, ts, &time, &date, &time_cs);
>         de->time = de->ctime = time;
>         de->date = de->cdate = de->adate = date;
>         de->ctime_cs = time_cs;

Ah, you're right. I missed vfat_mkdir() in the previous patch. After
further study, I found that maybe we can remove fat_truncate_crtime()
in both vfat_create() and vfat_mkdir(). crtime will come from only these
two cases:

(1) read from disk
(2) generated in vfat_build_slots()

They are all in {cdate:16, ctime:16, ctime_cs:8} format, which ensures
crtime will be kept at the correct granularity (10 ms). The remaining
timestamps may be copied from the vfs inode, so we need to truncate them
to fit FAT's format. But crtime doesn't need to do that.

Thanks.