Re: [PATCH v2 2/3] fat: make ctime and mtime identical explicitly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Chung-Chiang Cheng <cccheng@xxxxxxxxxxxx> writes:

> 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.
>
> In address this issue, this patch changes the behavior of ctime. ctime
> 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.

Hm, this changes mtime includes ctime update. So, the question is, this
behavior is compatible with Windows's fatfs behavior? E.g. Windows
updates mtime on rename?

If not same behavior with Windows, new behavior is new incompatible
behavior, and looks break fundamental purpose of this.

I was thinking, we ignores ctime update (because fatfs doesn't have) and
always same with mtime. What behavior was actually compatible with
Windows?

Thanks.

> Signed-off-by: Chung-Chiang Cheng <cccheng@xxxxxxxxxxxx>
> ---
>  fs/fat/dir.c         |  2 +-
>  fs/fat/file.c        |  4 ++++
>  fs/fat/inode.c       | 11 ++++-------
>  fs/fat/misc.c        | 11 ++++++++---
>  fs/fat/namei_msdos.c |  6 +++---
>  fs/fat/namei_vfat.c  |  6 +++---
>  6 files changed, 23 insertions(+), 17 deletions(-)
>
> diff --git a/fs/fat/dir.c b/fs/fat/dir.c
> index 249825017da7..0ae0dfe278fb 100644
> --- a/fs/fat/dir.c
> +++ b/fs/fat/dir.c
> @@ -1068,7 +1068,7 @@ int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo)
>  		}
>  	}
>  
> -	fat_truncate_time(dir, NULL, S_ATIME|S_MTIME);
> +	fat_truncate_time(dir, NULL, S_ATIME|S_CTIME|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 a5a309fcc7fa..178c1dde3488 100644
> --- a/fs/fat/file.c
> +++ b/fs/fat/file.c
> @@ -544,6 +544,10 @@ int fat_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
>  	/*
>  	 * setattr_copy can't truncate these appropriately, so we'll
>  	 * copy them ourselves
> +	 *
> +	 * fat_truncate_time() keeps ctime and mtime the same. if both
> +	 * ctime and mtime are need to update here, mtime will overwrite
> +	 * ctime
>  	 */
>  	if (attr->ia_valid & ATTR_ATIME)
>  		fat_truncate_time(inode, &attr->ia_atime, S_ATIME);
> 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..71e6dadf12a2 100644
> --- a/fs/fat/misc.c
> +++ b/fs/fat/misc.c
> @@ -341,10 +341,15 @@ 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.
> +	 */
> +	if (flags & (S_CTIME|S_MTIME)) {
>  		fat_truncate_mtime(sbi, now, &inode->i_mtime);
> +		inode->i_ctime = inode->i_mtime;
> +	}
>  
>  	return 0;
>  }
> diff --git a/fs/fat/namei_msdos.c b/fs/fat/namei_msdos.c
> index efba301d68ae..b2760a716707 100644
> --- a/fs/fat/namei_msdos.c
> +++ b/fs/fat/namei_msdos.c
> @@ -328,7 +328,7 @@ static int msdos_rmdir(struct inode *dir, struct dentry *dentry)
>  	drop_nlink(dir);
>  
>  	clear_nlink(inode);
> -	fat_truncate_time(inode, NULL, S_CTIME);
> +	fat_truncate_time(inode, NULL, S_CTIME|S_MTIME);
>  	fat_detach(inode);
>  out:
>  	mutex_unlock(&MSDOS_SB(sb)->s_lock);
> @@ -415,7 +415,7 @@ static int msdos_unlink(struct inode *dir, struct dentry *dentry)
>  	if (err)
>  		goto out;
>  	clear_nlink(inode);
> -	fat_truncate_time(inode, NULL, S_CTIME);
> +	fat_truncate_time(inode, NULL, S_CTIME|S_MTIME);
>  	fat_detach(inode);
>  out:
>  	mutex_unlock(&MSDOS_SB(sb)->s_lock);
> @@ -550,7 +550,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);
> -		fat_truncate_time(new_inode, &ts, S_CTIME);
> +		fat_truncate_time(new_inode, &ts, S_CTIME|S_MTIME);
>  	}
>  out:
>  	brelse(sinfo.bh);
> diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
> index 5369d82e0bfb..b8deb859b2b5 100644
> --- a/fs/fat/namei_vfat.c
> +++ b/fs/fat/namei_vfat.c
> @@ -811,7 +811,7 @@ static int vfat_rmdir(struct inode *dir, struct dentry *dentry)
>  	drop_nlink(dir);
>  
>  	clear_nlink(inode);
> -	fat_truncate_time(inode, NULL, S_ATIME|S_MTIME);
> +	fat_truncate_time(inode, NULL, S_ATIME|S_CTIME|S_MTIME);
>  	fat_detach(inode);
>  	vfat_d_version_set(dentry, inode_query_iversion(dir));
>  out:
> @@ -837,7 +837,7 @@ static int vfat_unlink(struct inode *dir, struct dentry *dentry)
>  	if (err)
>  		goto out;
>  	clear_nlink(inode);
> -	fat_truncate_time(inode, NULL, S_ATIME|S_MTIME);
> +	fat_truncate_time(inode, NULL, S_ATIME|S_CTIME|S_MTIME);
>  	fat_detach(inode);
>  	vfat_d_version_set(dentry, inode_query_iversion(dir));
>  out:
> @@ -981,7 +981,7 @@ static int vfat_rename(struct user_namespace *mnt_userns, struct inode *old_dir,
>  		drop_nlink(new_inode);
>  		if (is_dir)
>  			drop_nlink(new_inode);
> -		fat_truncate_time(new_inode, &ts, S_CTIME);
> +		fat_truncate_time(new_inode, &ts, S_CTIME|S_MTIME);
>  	}
>  out:
>  	brelse(sinfo.bh);

-- 
OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux