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

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

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

> -	fat_truncate_time(dir, NULL, S_ATIME|S_MTIME);
> +	fat_truncate_time(dir, NULL, S_ATIME|S_CTIME|S_MTIME);

fat_truncate_time() updates i_ctime too. So S_CTIME should not be
necessary here. And I think this is better to use only S_MTIME to tell
this is the point of mtime update.

(And, in fat_truncate_time(), I think S_CTIME is not required, because
we ignore ctime change, isn't it?)

Or you are going to update mtime on rename, etc too?

> +	/*
> +	 * 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;
> +	}

[...]

>  	clear_nlink(inode);
> -	fat_truncate_time(inode, NULL, S_CTIME);
> +	fat_truncate_time(inode, NULL, S_CTIME|S_MTIME);

This is the point to update ctime. You want to affect ctime change to
mtime? As I said in previous post, I think we are better to ignore ctime
change, because it may become yet another incompatible behavior.

>  	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);

ditto

>  	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);

ditto

>  	}
>  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);

ditto

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