* [PATCH 1/2] fat: split fat_truncate_time() into separate functions @ 2022-03-21 9:58 Chung-Chiang Cheng 2022-03-21 9:58 ` [PATCH 2/2] fat: introduce creation time Chung-Chiang Cheng 0 siblings, 1 reply; 9+ messages in thread From: Chung-Chiang Cheng @ 2022-03-21 9:58 UTC (permalink / raw) To: hirofumi; +Cc: linux-fsdevel, shepjeng, kernel, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] fat: introduce creation time 2022-03-21 9:58 [PATCH 1/2] fat: split fat_truncate_time() into separate functions Chung-Chiang Cheng @ 2022-03-21 9:58 ` Chung-Chiang Cheng 2022-03-22 7:33 ` OGAWA Hirofumi 0 siblings, 1 reply; 9+ messages in thread From: Chung-Chiang Cheng @ 2022-03-21 9:58 UTC (permalink / raw) To: hirofumi; +Cc: linux-fsdevel, shepjeng, kernel, Chung-Chiang Cheng In the old days, FAT supports creation time, but there's no corresponding timestamp in VFS. The implementation mixed the meaning of change time and creation time into a single ctime field. This patch introduces a new field for creation time, and reports it in statx. The original ctime doesn't stand for create time any more. All the behaviors of ctime (change time) follow mtime, as exfat does. Signed-off-by: Chung-Chiang Cheng <cccheng@synology.com> --- fs/fat/fat.h | 1 + fs/fat/file.c | 6 ++++++ fs/fat/inode.c | 15 ++++++++++----- fs/fat/misc.c | 6 +++--- 4 files changed, 20 insertions(+), 8 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 13855ba49cd9..184fa0375152 100644 --- a/fs/fat/file.c +++ b/fs/fat/file.c @@ -405,6 +405,12 @@ int fat_getattr(struct user_namespace *mnt_userns, const struct path *path, /* Use i_pos for ino. This is used as fileid of nfs. */ stat->ino = fat_i_pos_read(MSDOS_SB(inode->i_sb), inode); } + + if (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 a6f1c6d426d1..41b85b95ee9d 100644 --- a/fs/fat/inode.c +++ b/fs/fat/inode.c @@ -566,12 +566,15 @@ 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); + inode->i_ctime = inode->i_mtime; if (sbi->options.isvfat) { - fat_time_fat2unix(sbi, &inode->i_ctime, de->ctime, - de->cdate, de->ctime_cs); fat_time_fat2unix(sbi, &inode->i_atime, 0, de->adate, 0); - } else - fat_truncate_time(inode, &inode->i_mtime, S_ATIME|S_CTIME); + 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); + fat_truncate_crtime(sbi, &inode->i_mtime, &MSDOS_I(inode)->i_crtime); + } return 0; } @@ -756,6 +759,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,7 +892,7 @@ 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, + 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 c87df64f8b2b..36b6da6461cc 100644 --- a/fs/fat/misc.c +++ b/fs/fat/misc.c @@ -341,10 +341,10 @@ 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) + if (flags & (S_CTIME | S_MTIME)) { fat_truncate_mtime(sbi, now, &inode->i_mtime); + inode->i_ctime = inode->i_mtime; + } return 0; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] fat: introduce creation time 2022-03-21 9:58 ` [PATCH 2/2] fat: introduce creation time Chung-Chiang Cheng @ 2022-03-22 7:33 ` OGAWA Hirofumi 2022-03-23 2:14 ` Chung-Chiang Cheng 0 siblings, 1 reply; 9+ messages in thread From: OGAWA Hirofumi @ 2022-03-22 7:33 UTC (permalink / raw) To: Chung-Chiang Cheng; +Cc: linux-fsdevel, shepjeng, kernel Chung-Chiang Cheng <cccheng@synology.com> writes: > In the old days, FAT supports creation time, but there's no corresponding > timestamp in VFS. The implementation mixed the meaning of change time and > creation time into a single ctime field. > > This patch introduces a new field for creation time, and reports it in > statx. The original ctime doesn't stand for create time any more. All > the behaviors of ctime (change time) follow mtime, as exfat does. Yes, ctime is issue (include compatibility issue when changing) from original author of this driver. And there is no perfect solution and subtle issue I think. I'm not against about this change though, this behavior makes utimes(2) behavior strange, e.g. user can change ctime, but FAT forget it anytime, because FAT can't save it. Did you consider about those behavior and choose this? Thanks. > Signed-off-by: Chung-Chiang Cheng <cccheng@synology.com> > --- > fs/fat/fat.h | 1 + > fs/fat/file.c | 6 ++++++ > fs/fat/inode.c | 15 ++++++++++----- > fs/fat/misc.c | 6 +++--- > 4 files changed, 20 insertions(+), 8 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 13855ba49cd9..184fa0375152 100644 > --- a/fs/fat/file.c > +++ b/fs/fat/file.c > @@ -405,6 +405,12 @@ int fat_getattr(struct user_namespace *mnt_userns, const struct path *path, > /* Use i_pos for ino. This is used as fileid of nfs. */ > stat->ino = fat_i_pos_read(MSDOS_SB(inode->i_sb), inode); > } > + > + if (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 a6f1c6d426d1..41b85b95ee9d 100644 > --- a/fs/fat/inode.c > +++ b/fs/fat/inode.c > @@ -566,12 +566,15 @@ 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); > + inode->i_ctime = inode->i_mtime; > if (sbi->options.isvfat) { > - fat_time_fat2unix(sbi, &inode->i_ctime, de->ctime, > - de->cdate, de->ctime_cs); > fat_time_fat2unix(sbi, &inode->i_atime, 0, de->adate, 0); > - } else > - fat_truncate_time(inode, &inode->i_mtime, S_ATIME|S_CTIME); > + 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); > + fat_truncate_crtime(sbi, &inode->i_mtime, &MSDOS_I(inode)->i_crtime); > + } > > return 0; > } > @@ -756,6 +759,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,7 +892,7 @@ 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, > + 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 c87df64f8b2b..36b6da6461cc 100644 > --- a/fs/fat/misc.c > +++ b/fs/fat/misc.c > @@ -341,10 +341,10 @@ 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) > + if (flags & (S_CTIME | S_MTIME)) { > fat_truncate_mtime(sbi, now, &inode->i_mtime); > + inode->i_ctime = inode->i_mtime; > + } > > return 0; > } -- OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] fat: introduce creation time 2022-03-22 7:33 ` OGAWA Hirofumi @ 2022-03-23 2:14 ` Chung-Chiang Cheng 2022-03-23 6:57 ` OGAWA Hirofumi 0 siblings, 1 reply; 9+ messages in thread From: Chung-Chiang Cheng @ 2022-03-23 2:14 UTC (permalink / raw) To: OGAWA Hirofumi; +Cc: Chung-Chiang Cheng, linux-fsdevel, kernel OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> 於 2022年3月22日 週二 下午3:33寫道： > > Chung-Chiang Cheng <cccheng@synology.com> writes: > > > In the old days, FAT supports creation time, but there's no corresponding > > timestamp in VFS. The implementation mixed the meaning of change time and > > creation time into a single ctime field. > > > > This patch introduces a new field for creation time, and reports it in > > statx. The original ctime doesn't stand for create time any more. All > > the behaviors of ctime (change time) follow mtime, as exfat does. > > Yes, ctime is issue (include compatibility issue when changing) from > original author of this driver. And there is no perfect solution and > subtle issue I think. > > I'm not against about this change though, this behavior makes utimes(2) > behavior strange, e.g. user can change ctime, but FAT forget it anytime, > because FAT can't save it. > > Did you consider about those behavior and choose this? Yes. I think it's not perfect but a better choice to distinguish between change-time and creation-time. While change-time is no longer saved to disk, the new behavior maintains the semantic of "creation" and is more compatible with non-linux systems. Thanks. > Thanks. > > > Signed-off-by: Chung-Chiang Cheng <cccheng@synology.com> > > --- > > fs/fat/fat.h | 1 + > > fs/fat/file.c | 6 ++++++ > > fs/fat/inode.c | 15 ++++++++++----- > > fs/fat/misc.c | 6 +++--- > > 4 files changed, 20 insertions(+), 8 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 13855ba49cd9..184fa0375152 100644 > > --- a/fs/fat/file.c > > +++ b/fs/fat/file.c > > @@ -405,6 +405,12 @@ int fat_getattr(struct user_namespace *mnt_userns, const struct path *path, > > /* Use i_pos for ino. This is used as fileid of nfs. */ > > stat->ino = fat_i_pos_read(MSDOS_SB(inode->i_sb), inode); > > } > > + > > + if (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 a6f1c6d426d1..41b85b95ee9d 100644 > > --- a/fs/fat/inode.c > > +++ b/fs/fat/inode.c > > @@ -566,12 +566,15 @@ 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); > > + inode->i_ctime = inode->i_mtime; > > if (sbi->options.isvfat) { > > - fat_time_fat2unix(sbi, &inode->i_ctime, de->ctime, > > - de->cdate, de->ctime_cs); > > fat_time_fat2unix(sbi, &inode->i_atime, 0, de->adate, 0); > > - } else > > - fat_truncate_time(inode, &inode->i_mtime, S_ATIME|S_CTIME); > > + 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); > > + fat_truncate_crtime(sbi, &inode->i_mtime, &MSDOS_I(inode)->i_crtime); > > + } > > > > return 0; > > } > > @@ -756,6 +759,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,7 +892,7 @@ 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, > > + 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 c87df64f8b2b..36b6da6461cc 100644 > > --- a/fs/fat/misc.c > > +++ b/fs/fat/misc.c > > @@ -341,10 +341,10 @@ 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) > > + if (flags & (S_CTIME | S_MTIME)) { > > fat_truncate_mtime(sbi, now, &inode->i_mtime); > > + inode->i_ctime = inode->i_mtime; > > + } > > > > return 0; > > } > > -- > OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] fat: introduce creation time 2022-03-23 2:14 ` Chung-Chiang Cheng @ 2022-03-23 6:57 ` OGAWA Hirofumi 2022-03-23 10:27 ` Chung-Chiang Cheng 0 siblings, 1 reply; 9+ messages in thread From: OGAWA Hirofumi @ 2022-03-23 6:57 UTC (permalink / raw) To: Chung-Chiang Cheng; +Cc: Chung-Chiang Cheng, linux-fsdevel, kernel Chung-Chiang Cheng <shepjeng@gmail.com> writes: >> Yes, ctime is issue (include compatibility issue when changing) from >> original author of this driver. And there is no perfect solution and >> subtle issue I think. >> >> I'm not against about this change though, this behavior makes utimes(2) >> behavior strange, e.g. user can change ctime, but FAT forget it anytime, >> because FAT can't save it. >> >> Did you consider about those behavior and choose this? > > Yes. I think it's not perfect but a better choice to distinguish between > change-time and creation-time. While change-time is no longer saved to > disk, the new behavior maintains the semantic of "creation" and is more > compatible with non-linux systems. Ok, right, creation time is good. But what I'm saying is about new ctime behavior. Now, you allow to change ctime as old behavior, but it is not saved. Why this behavior was preferred? Just for example, I think we can ignore ctime change, and define new behavior is as ctime==mtime always. This will prevent time wrap/backward etc. Thanks. -- OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] fat: introduce creation time 2022-03-23 6:57 ` OGAWA Hirofumi @ 2022-03-23 10:27 ` Chung-Chiang Cheng 2022-03-23 10:57 ` OGAWA Hirofumi 0 siblings, 1 reply; 9+ messages in thread From: Chung-Chiang Cheng @ 2022-03-23 10:27 UTC (permalink / raw) To: OGAWA Hirofumi; +Cc: Chung-Chiang Cheng, linux-fsdevel, kernel OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> 於 2022年3月23日 週三 下午2:57寫道： > > Chung-Chiang Cheng <shepjeng@gmail.com> writes: > > > > > Yes. I think it's not perfect but a better choice to distinguish between > > change-time and creation-time. While change-time is no longer saved to > > disk, the new behavior maintains the semantic of "creation" and is more > > compatible with non-linux systems. > > Ok, right, creation time is good. But what I'm saying is about new ctime > behavior. > > Now, you allow to change ctime as old behavior, but it is not saved. Why > this behavior was preferred? > > Just for example, I think we can ignore ctime change, and define new > behavior is as ctime==mtime always. This will prevent time wrap/backward > etc. I got your point. Correctly speaking ctime is not really dropped but mixed with mtime after my patch. They share the same field on disk. Before that ctime is mixed with crtime. I choose this new behavior because ctime and mtime are similar concepts. ctime is the update time for file attributes, and mtime is the one for file contents. They are updated together most of the time with few exceptions (rename, rmdir, unlink) in the current FAT implementation. Thanks. > > Thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] fat: introduce creation time 2022-03-23 10:27 ` Chung-Chiang Cheng @ 2022-03-23 10:57 ` OGAWA Hirofumi 2022-03-25 7:38 ` Chung-Chiang Cheng 0 siblings, 1 reply; 9+ messages in thread From: OGAWA Hirofumi @ 2022-03-23 10:57 UTC (permalink / raw) To: Chung-Chiang Cheng; +Cc: Chung-Chiang Cheng, linux-fsdevel, kernel Chung-Chiang Cheng <shepjeng@gmail.com> writes: > I got your point. Correctly speaking ctime is not really dropped but mixed > with mtime after my patch. They share the same field on disk. Before that > ctime is mixed with crtime. > > I choose this new behavior because ctime and mtime are similar concepts. > ctime is the update time for file attributes, and mtime is the one for file > contents. They are updated together most of the time with few exceptions > (rename, rmdir, unlink) in the current FAT implementation. No, a user can change the ctime to arbitrary time, and after the your patch, the changed ctime only hold on a memory inode. So a user sees ctime jump backward and forward when a memory inode is expired. (Of course, this happens just by "cp -a" in real world use case.) I'm pointing about this introduced new behavior by your patch. Thanks. -- OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] fat: introduce creation time 2022-03-23 10:57 ` OGAWA Hirofumi @ 2022-03-25 7:38 ` Chung-Chiang Cheng 2022-03-25 10:36 ` OGAWA Hirofumi 0 siblings, 1 reply; 9+ messages in thread From: Chung-Chiang Cheng @ 2022-03-25 7:38 UTC (permalink / raw) To: OGAWA Hirofumi; +Cc: Chung-Chiang Cheng, linux-fsdevel, kernel On Wed, Mar 23, 2022 at 6:57 PM OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> wrote: > > No, a user can change the ctime to arbitrary time, and after the your > patch, the changed ctime only hold on a memory inode. So a user sees > ctime jump backward and forward when a memory inode is expired. (Of > course, this happens just by "cp -a" in real world use case.) > > I'm pointing about this introduced new behavior by your patch. > As you mentioned, there are still some cases to consider that ctime isn't identical to mtime. If so, ctime won't be consistent after inode is expired because it will be filled with the value of on-disk mtime, which is weird and confusing. To solve the issue, I propose to keep ctime and mtime always the same in memory. If you agree with this approach, I'll send a v2 patch for it. Thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] fat: introduce creation time 2022-03-25 7:38 ` Chung-Chiang Cheng @ 2022-03-25 10:36 ` OGAWA Hirofumi 0 siblings, 0 replies; 9+ messages in thread From: OGAWA Hirofumi @ 2022-03-25 10:36 UTC (permalink / raw) To: Chung-Chiang Cheng; +Cc: Chung-Chiang Cheng, linux-fsdevel, kernel Chung-Chiang Cheng <shepjeng@gmail.com> writes: > On Wed, Mar 23, 2022 at 6:57 PM OGAWA Hirofumi > <hirofumi@mail.parknet.co.jp> wrote: >> >> No, a user can change the ctime to arbitrary time, and after the your >> patch, the changed ctime only hold on a memory inode. So a user sees >> ctime jump backward and forward when a memory inode is expired. (Of >> course, this happens just by "cp -a" in real world use case.) >> >> I'm pointing about this introduced new behavior by your patch. >> > > As you mentioned, there are still some cases to consider that ctime > isn't identical to mtime. If so, ctime won't be consistent after > inode is expired because it will be filled with the value of on-disk > mtime, which is weird and confusing. > > To solve the issue, I propose to keep ctime and mtime always the same > in memory. If you agree with this approach, I'll send a v2 patch for > it. Yes, exactly. Although I think it is better, in real world userspace, it may got actual compatibility issue and reported, then we may have to revert even if I personally think new is better. Thanks. -- OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-03-25 10:36 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-21 9:58 [PATCH 1/2] fat: split fat_truncate_time() into separate functions Chung-Chiang Cheng 2022-03-21 9:58 ` [PATCH 2/2] fat: introduce creation time Chung-Chiang Cheng 2022-03-22 7:33 ` OGAWA Hirofumi 2022-03-23 2:14 ` Chung-Chiang Cheng 2022-03-23 6:57 ` OGAWA Hirofumi 2022-03-23 10:27 ` Chung-Chiang Cheng 2022-03-23 10:57 ` OGAWA Hirofumi 2022-03-25 7:38 ` Chung-Chiang Cheng 2022-03-25 10:36 ` OGAWA Hirofumi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.