Re: [RFC PATCH] fat: truncate timestamps returned from fat_getattr

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

Frank Sorenson <sorenson@redhat.com> writes:

Hi,

> Author: Frank Sorenson <sorenson@redhat.com>
> Date:   2018-03-23 23:28:32 -0500
>
>     fat: add function to truncate timestamps returned from fat_getattr
>     
>     vfat/msdos timestamps are stored on-disk with several different
>     granularities, some of them lower resolution than timespec_trunc()
>     can provide.  However, 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.
>     
>     This patch adds a function to adjust the timestamps returned from
>     fat_getattr() so that userspace sees consistent times across a
>     remount.  The existing functions are used to convert the timespec
>     to the on-disk representation, then back to timespec again.
>     
>     Signed-off-by: Frank Sorenson <sorenson@redhat.com>

Already tried to use ->update_time() callback? With quick look though,
it seems to be possible to control timestamp granularity with
->update_time(), and add fat_current_time() or such to get proper
timestamp for FAT.

I think we should give consistent in-core inode timestamp too. For
example, overlayfs may be copy timestamps from in-core inode, etc., then
this stat workaround will not work.

Thanks.

> diff --git a/fs/fat/fat.h b/fs/fat/fat.h
> index 8fc1093da47d..0367be021b23 100644
> --- a/fs/fat/fat.h
> +++ b/fs/fat/fat.h
> @@ -410,6 +410,7 @@ extern void fat_time_fat2unix(struct msdos_sb_info *sbi, struct timespec *ts,
>  			      __le16 __time, __le16 __date, u8 time_cs);
>  extern void fat_time_unix2fat(struct msdos_sb_info *sbi, struct timespec *ts,
>  			      __le16 *time, __le16 *date, u8 *time_cs);
> +extern void fat_trunc_kstat_times(struct msdos_sb_info *sbi, struct kstat *stat);
>  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 4724cc9ad650..11639e393ef7 100644
> --- a/fs/fat/file.c
> +++ b/fs/fat/file.c
> @@ -370,6 +370,7 @@ int fat_getattr(const struct path *path, struct kstat *stat,
>  {
>  	struct inode *inode = d_inode(path->dentry);
>  	generic_fillattr(inode, stat);
> +	fat_trunc_kstat_times(MSDOS_SB(inode->i_sb), stat);
>  	stat->blksize = MSDOS_SB(inode->i_sb)->cluster_size;
>  
>  	if (MSDOS_SB(inode->i_sb)->options.nfs == FAT_NFS_NOSTALE_RO) {
> diff --git a/fs/fat/misc.c b/fs/fat/misc.c
> index f9bdc1e01c98..52f40edefd12 100644
> --- a/fs/fat/misc.c
> +++ b/fs/fat/misc.c
> @@ -262,6 +262,29 @@ void fat_time_unix2fat(struct msdos_sb_info *sbi, struct timespec *ts,
>  }
>  EXPORT_SYMBOL_GPL(fat_time_unix2fat);
>  
> +void fat_trunc_kstat_times(struct msdos_sb_info *sbi, struct kstat *stat)
> +{
> +	if (stat->ino == MSDOS_ROOT_INO)
> +		stat->mtime = stat->ctime = stat->atime =
> +			(struct timespec){0, 0};
> +	else {
> +		__le16 time;
> +		__le16 date;
> +		u8 ctime_cs;
> +
> +		fat_time_unix2fat(sbi, &stat->mtime, &time, &date, NULL);
> +		fat_time_fat2unix(sbi, &stat->mtime, time, date, 0);
> +
> +		fat_time_unix2fat(sbi, &stat->ctime, &time, &date,
> +			sbi->options.isvfat ? &ctime_cs :  NULL);
> +		fat_time_fat2unix(sbi, &stat->ctime, time, date,
> +			sbi->options.isvfat ? ctime_cs : 0);
> +
> +		fat_time_unix2fat(sbi, &stat->atime, &time, &date, NULL);
> +		fat_time_fat2unix(sbi, &stat->atime, 0, date, 0);
> +	}
> +}
> +
>  int fat_sync_bhs(struct buffer_head **bhs, int nr_bhs)
>  {
>  	int i, err = 0;
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>