Re: [PATCH 2/2] smb: invalidate and close cached directory when creating child entries

[Henrique Carvalho <henrique.carvalho@suse.com>]

Hi Bharath,

On Tue, Jul 01, 2025 at 12:23:03AM +0530, Bharath SM wrote:
> @@ -190,6 +190,7 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
>  	int disposition;
>  	struct TCP_Server_Info *server = tcon->ses->server;
>  	struct cifs_open_parms oparms;
> +	struct cached_fid *parent_cfid = NULL;
>  	int rdwr_for_fscache = 0;
>  	__le32 lease_flags = 0;
>
> @@ -313,10 +314,10 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
>  	if (!tcon->unix_ext && (mode & S_IWUGO) == 0)
>  		create_options |= CREATE_OPTION_READONLY;
>  
> +
>  retry_open:
>  	if (tcon->cfids && direntry->d_parent && server->dialect >= SMB30_PROT_ID) {
> -		struct cached_fid *parent_cfid;
> -
> +		parent_cfid = NULL;

I believe setting to NULL here is unnecessary, no?

>  		spin_lock(&tcon->cfids->cfid_list_lock);
>  		list_for_each_entry(parent_cfid, &tcon->cfids->entries, entry) {
>  			if (parent_cfid->dentry == direntry->d_parent) {
> @@ -327,6 +328,7 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
>  					memcpy(fid->parent_lease_key,
>  					       parent_cfid->fid.lease_key,
>  					       SMB2_LEASE_KEY_SIZE);
> +					parent_cfid->dirents.is_valid = false;

Shouldn't we set dirents.is_valid to false only after the open is
successful?

>  				}
>  				break;
>  			}
> @@ -355,6 +357,10 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
>  		}
>  		goto out;
>  	}
> +
> +	if (parent_cfid && !parent_cfid->dirents.is_valid)
> +		close_cached_dir(parent_cfid);
> +
>  	if (rdwr_for_fscache == 2)
>  		cifs_invalidate_cache(inode, FSCACHE_INVAL_DIO_WRITE);
>

Apart from the above,

Reviewed-by: Henrique Carvalho <henrique.carvalho@suse.com>