Re: [PATCH 2/6] cifs: cifs: handlecache, only track the dentry for the root handle

[Tom Talpey <tom@talpey.com>]

On 8/23/2022 8:27 PM, Ronnie Sahlberg wrote:
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
>   fs/cifs/cached_dir.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/cifs/cached_dir.c b/fs/cifs/cached_dir.c
> index c2f5b71a3c9f..77880470c7ea 100644
> --- a/fs/cifs/cached_dir.c
> +++ b/fs/cifs/cached_dir.c
> @@ -47,11 +47,12 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
>   	if (cifs_sb->root == NULL)
>   		return -ENOENT;
>   
> +	if (!strlen(path))
> +		dentry = cifs_sb->root;

Wouldn't it be safer and more efficient to simply test
"if (path[0] == 0)"?

But, why would a non-null path ever be passed, if it
always fails? Seems like a pointless call in the first
place.

> +
>   	if (strlen(path))

Simply "else"? No need to recompute strlen.

Tom.

>   		return -ENOENT;
>   
> -	dentry = cifs_sb->root;
> -
>   	cfid = &tcon->cfids->cfid;
>   	mutex_lock(&cfid->fid_mutex);
>   	if (cfid->is_valid) {
> @@ -177,7 +178,8 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
>   	cfid->tcon = tcon;
>   	cfid->is_valid = true;
>   	cfid->dentry = dentry;
> -	dget(dentry);
> +	if (dentry)
> +		dget(dentry);
>   	kref_init(&cfid->refcount);
>   
>   	/* BB TBD check to see if oplock level check can be removed below */