Re: [PATCH] ksmbd: set ATTR_CTIME flags when setting mtime

[David Disseldorp <ddiss@suse.de>]

Hi Namjae,

On Sat,  7 Dec 2024 00:38:58 +0900, Namjae Jeon wrote:
...
>  fs/smb/server/smb2pdu.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
> index 5a70df87074c..803b35b89513 100644
> --- a/fs/smb/server/smb2pdu.c
> +++ b/fs/smb/server/smb2pdu.c
> @@ -6026,15 +6026,13 @@ static int set_file_basic_info(struct ksmbd_file *fp,
>  		attrs.ia_valid |= (ATTR_ATIME | ATTR_ATIME_SET);
>  	}
>  
> -	attrs.ia_valid |= ATTR_CTIME;

This will mean that we now only call through to notify_change() on atime
or mtime updates, which doesn't seem right. Can we get here with a valid
file_info->ChangeTime only (i.e. no atime/mtime change)?

>  	if (file_info->ChangeTime)
> -		attrs.ia_ctime = ksmbd_NTtimeToUnix(file_info->ChangeTime);
> -	else
> -		attrs.ia_ctime = inode_get_ctime(inode);
> +		inode_set_ctime_to_ts(inode,
> +				ksmbd_NTtimeToUnix(file_info->ChangeTime));
>  
>  	if (file_info->LastWriteTime) {
>  		attrs.ia_mtime = ksmbd_NTtimeToUnix(file_info->LastWriteTime);
> -		attrs.ia_valid |= (ATTR_MTIME | ATTR_MTIME_SET);
> +		attrs.ia_valid |= (ATTR_MTIME | ATTR_MTIME_SET | ATTR_CTIME);
>  	}
>  
>  	if (file_info->Attributes) {
> @@ -6076,8 +6074,6 @@ static int set_file_basic_info(struct ksmbd_file *fp,
>  			return -EACCES;
>  
>  		inode_lock(inode);
> -		inode_set_ctime_to_ts(inode, attrs.ia_ctime);
> -		attrs.ia_valid &= ~ATTR_CTIME;
>  		rc = notify_change(idmap, dentry, &attrs, NULL);
>  		inode_unlock(inode);
>  	}