[PATCH 1/2] smb: client: disable directory caching when dir_cache_timeout is zero

[PATCH 1/2] smb: client: disable directory caching when dir_cache_timeout is zero

[Henrique Carvalho]

According to the dir_cache_timeout description, setting it to zero
should disable the caching of directory contents. However, even when
dir_cache_timeout is zero, some caching related functions are still
invoked, and the worker thread is initiated, which is unintended
behavior.

Fix the issue by setting tcon->nohandlecache to true when
dir_cache_timeout is zero, ensuring that directory handle caching
is properly disabled.

Clean up the code to reflect this change, to improve consistency,
and to remove other unnecessary checks.

is_smb1_server() check inside open_cached_dir() can be removed because
dir caching is only enabled for SMB versions >= 2.0.

Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
---
 fs/smb/client/cached_dir.c | 12 +++++++-----
 fs/smb/client/cifsproto.h  |  2 +-
 fs/smb/client/connect.c    | 10 +++++-----
 fs/smb/client/misc.c       |  4 ++--
 4 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 8b510c858f4ff..d8b1cf1043c35 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -162,15 +162,17 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 	const char *npath;
 	int retries = 0, cur_sleep = 1;
 
-	if (tcon == NULL || tcon->cfids == NULL || tcon->nohandlecache ||
-	    is_smb1_server(tcon->ses->server) || (dir_cache_timeout == 0))
+	if (cifs_sb->root == NULL)
+		return -ENOENT;
+
+	if (tcon == NULL)
 		return -EOPNOTSUPP;
 
 	ses = tcon->ses;
 	cfids = tcon->cfids;
 
-	if (cifs_sb->root == NULL)
-		return -ENOENT;
+	if (cfids == NULL)
+		return -EOPNOTSUPP;
 
 replay_again:
 	/* reinitialize for possible replay */
@@ -394,7 +396,7 @@ int open_cached_dir_by_dentry(struct cifs_tcon *tcon,
 	struct cached_fids *cfids = tcon->cfids;
 
 	if (cfids == NULL)
-		return -ENOENT;
+		return -EOPNOTSUPP;
 
 	spin_lock(&cfids->cfid_list_lock);
 	list_for_each_entry(cfid, &cfids->entries, entry) {
diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index 075985bfb13a8..d89d31b6dd97a 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -530,7 +530,7 @@ extern int CIFSSMBLogoff(const unsigned int xid, struct cifs_ses *ses);
 
 extern struct cifs_ses *sesInfoAlloc(void);
 extern void sesInfoFree(struct cifs_ses *);
-extern struct cifs_tcon *tcon_info_alloc(bool dir_leases_enabled,
+extern struct cifs_tcon *tcon_info_alloc(bool enable_dir_cache,
 					 enum smb3_tcon_ref_trace trace);
 extern void tconInfoFree(struct cifs_tcon *tcon, enum smb3_tcon_ref_trace trace);
 
diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index b227d61a6f205..f74e0b94f848c 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -2593,7 +2593,7 @@ static struct cifs_tcon *
 cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
 {
 	struct cifs_tcon *tcon;
-	bool nohandlecache;
+	bool enable_dir_cache;
 	int rc, xid;
 
 	tcon = cifs_find_tcon(ses, ctx);
@@ -2614,15 +2614,15 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
 
 	if (ses->server->dialect >= SMB20_PROT_ID &&
 	    (ses->server->capabilities & SMB2_GLOBAL_CAP_DIRECTORY_LEASING))
-		nohandlecache = ctx->nohandlecache;
+		enable_dir_cache = !ctx->nohandlecache && (dir_cache_timeout != 0);
 	else
-		nohandlecache = true;
-	tcon = tcon_info_alloc(!nohandlecache, netfs_trace_tcon_ref_new);
+		enable_dir_cache = false;
+	tcon = tcon_info_alloc(enable_dir_cache, netfs_trace_tcon_ref_new);
 	if (tcon == NULL) {
 		rc = -ENOMEM;
 		goto out_fail;
 	}
-	tcon->nohandlecache = nohandlecache;
+	tcon->nohandlecache = !enable_dir_cache;
 
 	if (ctx->snapshot_time) {
 		if (ses->server->vals->protocol_id == 0) {
diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
index 4373dd64b66d4..60f35d827382c 100644
--- a/fs/smb/client/misc.c
+++ b/fs/smb/client/misc.c
@@ -111,7 +111,7 @@ sesInfoFree(struct cifs_ses *buf_to_free)
 }
 
 struct cifs_tcon *
-tcon_info_alloc(bool dir_leases_enabled, enum smb3_tcon_ref_trace trace)
+tcon_info_alloc(bool enable_dir_cache, enum smb3_tcon_ref_trace trace)
 {
 	struct cifs_tcon *ret_buf;
 	static atomic_t tcon_debug_id;
@@ -120,7 +120,7 @@ tcon_info_alloc(bool dir_leases_enabled, enum smb3_tcon_ref_trace trace)
 	if (!ret_buf)
 		return NULL;
 
-	if (dir_leases_enabled == true) {
+	if (enable_dir_cache) {
 		ret_buf->cfids = init_cached_dirs();
 		if (!ret_buf->cfids) {
 			kfree(ret_buf);
-- 
2.46.0

[PATCH 2/2] smb: client: remove unnecessary NULL check in open_cached_dir_by_dentry()

[Henrique Carvalho]

The function open_cached_dir_by_dentry() is only called by
cifs_dentry_needs_reval(), and it always passes dentry->d_parent as the
argument to dentry.

Since dentry->d_parent cannot be NULL, the check for dentry == NULL
is unnecessary and can be removed.

Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
---
 fs/smb/client/cached_dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index d8b1cf1043c35..9ac503dee0793 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -400,7 +400,7 @@ int open_cached_dir_by_dentry(struct cifs_tcon *tcon,
 
 	spin_lock(&cfids->cfid_list_lock);
 	list_for_each_entry(cfid, &cfids->entries, entry) {
-		if (dentry && cfid->dentry == dentry) {
+		if (cfid->dentry == dentry) {
 			cifs_dbg(FYI, "found a cached file handle by dentry\n");
 			kref_get(&cfid->refcount);
 			*ret_cfid = cfid;
-- 
2.46.0

Re: [PATCH 1/2] smb: client: disable directory caching when dir_cache_timeout is zero

[Paulo Alcantara]

Henrique Carvalho <henrique.carvalho@suse.com> writes:

> According to the dir_cache_timeout description, setting it to zero
> should disable the caching of directory contents. However, even when
> dir_cache_timeout is zero, some caching related functions are still
> invoked, and the worker thread is initiated, which is unintended
> behavior.
>
> Fix the issue by setting tcon->nohandlecache to true when
> dir_cache_timeout is zero, ensuring that directory handle caching
> is properly disabled.
>
> Clean up the code to reflect this change, to improve consistency,
> and to remove other unnecessary checks.
>
> is_smb1_server() check inside open_cached_dir() can be removed because
> dir caching is only enabled for SMB versions >= 2.0.
>
> Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
> ---
>  fs/smb/client/cached_dir.c | 12 +++++++-----
>  fs/smb/client/cifsproto.h  |  2 +-
>  fs/smb/client/connect.c    | 10 +++++-----
>  fs/smb/client/misc.c       |  4 ++--
>  4 files changed, 15 insertions(+), 13 deletions(-)

The fix could be simply this:

	diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
	index b227d61a6f20..62a29183c655 100644
	--- a/fs/smb/client/connect.c
	+++ b/fs/smb/client/connect.c
	@@ -2614,7 +2614,7 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
	 
	 	if (ses->server->dialect >= SMB20_PROT_ID &&
	 	    (ses->server->capabilities & SMB2_GLOBAL_CAP_DIRECTORY_LEASING))
	-		nohandlecache = ctx->nohandlecache;
	+		nohandlecache = ctx->nohandlecache || !dir_cache_timeout;
	 	else
	 		nohandlecache = true;
	 	tcon = tcon_info_alloc(!nohandlecache, netfs_trace_tcon_ref_new);

and easily backported to -stable kernels that have

        238b351d0935 ("smb3: allow controlling length of time directory entries are cached with dir leases")

And yes, is_smb1_server() check makes no sense as tcon->nohandlecache
would already be set.

Re: [PATCH 2/2] smb: client: remove unnecessary NULL check in open_cached_dir_by_dentry()

[Paulo Alcantara]

Henrique Carvalho <henrique.carvalho@suse.com> writes:

> The function open_cached_dir_by_dentry() is only called by
> cifs_dentry_needs_reval(), and it always passes dentry->d_parent as the
> argument to dentry.
>
> Since dentry->d_parent cannot be NULL, the check for dentry == NULL
> is unnecessary and can be removed.
>
> Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
> ---
>  fs/smb/client/cached_dir.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Looks good,

Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>

Re: [PATCH 1/2] smb: client: disable directory caching when dir_cache_timeout is zero

[Henrique Carvalho]

On Fri, Nov 22, 2024 at 07:07:06PM GMT, Paulo Alcantara wrote:
> Henrique Carvalho <henrique.carvalho@suse.com> writes:
> 
> > According to the dir_cache_timeout description, setting it to zero
> > should disable the caching of directory contents. However, even when
> > dir_cache_timeout is zero, some caching related functions are still
> > invoked, and the worker thread is initiated, which is unintended
> > behavior.
> >
> > Fix the issue by setting tcon->nohandlecache to true when
> > dir_cache_timeout is zero, ensuring that directory handle caching
> > is properly disabled.
> >
> > Clean up the code to reflect this change, to improve consistency,
> > and to remove other unnecessary checks.
> >
> > is_smb1_server() check inside open_cached_dir() can be removed because
> > dir caching is only enabled for SMB versions >= 2.0.
> >
> > Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
> > ---
> >  fs/smb/client/cached_dir.c | 12 +++++++-----
> >  fs/smb/client/cifsproto.h  |  2 +-
> >  fs/smb/client/connect.c    | 10 +++++-----
> >  fs/smb/client/misc.c       |  4 ++--
> >  4 files changed, 15 insertions(+), 13 deletions(-)
> 
> The fix could be simply this:
> 
> 	diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> 	index b227d61a6f20..62a29183c655 100644
> 	--- a/fs/smb/client/connect.c
> 	+++ b/fs/smb/client/connect.c
> 	@@ -2614,7 +2614,7 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
> 	 
> 	 	if (ses->server->dialect >= SMB20_PROT_ID &&
> 	 	    (ses->server->capabilities & SMB2_GLOBAL_CAP_DIRECTORY_LEASING))
> 	-		nohandlecache = ctx->nohandlecache;
> 	+		nohandlecache = ctx->nohandlecache || !dir_cache_timeout;
> 	 	else
> 	 		nohandlecache = true;
> 	 	tcon = tcon_info_alloc(!nohandlecache, netfs_trace_tcon_ref_new);
> 
> and easily backported to -stable kernels that have
> 
>         238b351d0935 ("smb3: allow controlling length of time directory entries are cached with dir leases")
>

Then I could split this into two separate patches, one with the fix for
the mentioned commit, and another patch for the changes in cached_dir.c
(which I still make sense to have).

The other changes are mostly cosmetic and could be dropped.

Sounds good?

--
Henrique

Re: [PATCH 1/2] smb: client: disable directory caching when dir_cache_timeout is zero

[Paulo Alcantara]

Henrique Carvalho <henrique.carvalho@suse.com> writes:

> On Fri, Nov 22, 2024 at 07:07:06PM GMT, Paulo Alcantara wrote:
>> Henrique Carvalho <henrique.carvalho@suse.com> writes:
>> 
>> > According to the dir_cache_timeout description, setting it to zero
>> > should disable the caching of directory contents. However, even when
>> > dir_cache_timeout is zero, some caching related functions are still
>> > invoked, and the worker thread is initiated, which is unintended
>> > behavior.
>> >
>> > Fix the issue by setting tcon->nohandlecache to true when
>> > dir_cache_timeout is zero, ensuring that directory handle caching
>> > is properly disabled.
>> >
>> > Clean up the code to reflect this change, to improve consistency,
>> > and to remove other unnecessary checks.
>> >
>> > is_smb1_server() check inside open_cached_dir() can be removed because
>> > dir caching is only enabled for SMB versions >= 2.0.
>> >
>> > Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
>> > ---
>> >  fs/smb/client/cached_dir.c | 12 +++++++-----
>> >  fs/smb/client/cifsproto.h  |  2 +-
>> >  fs/smb/client/connect.c    | 10 +++++-----
>> >  fs/smb/client/misc.c       |  4 ++--
>> >  4 files changed, 15 insertions(+), 13 deletions(-)
>> 
>> The fix could be simply this:
>> 
>> 	diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
>> 	index b227d61a6f20..62a29183c655 100644
>> 	--- a/fs/smb/client/connect.c
>> 	+++ b/fs/smb/client/connect.c
>> 	@@ -2614,7 +2614,7 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
>> 	 
>> 	 	if (ses->server->dialect >= SMB20_PROT_ID &&
>> 	 	    (ses->server->capabilities & SMB2_GLOBAL_CAP_DIRECTORY_LEASING))
>> 	-		nohandlecache = ctx->nohandlecache;
>> 	+		nohandlecache = ctx->nohandlecache || !dir_cache_timeout;
>> 	 	else
>> 	 		nohandlecache = true;
>> 	 	tcon = tcon_info_alloc(!nohandlecache, netfs_trace_tcon_ref_new);
>> 
>> and easily backported to -stable kernels that have
>> 
>>         238b351d0935 ("smb3: allow controlling length of time directory entries are cached with dir leases")
>>
>
> Then I could split this into two separate patches, one with the fix for
> the mentioned commit, and another patch for the changes in cached_dir.c
> (which I still make sense to have).

Removing is_smb1_server() check looks good, but the other changes don't
make much sense as we could potentially return -EOPNOTSUPP in
cifs_readdir(), for example, and -ENOENT is probably what you want.

Am I missing something?

Re: [PATCH 1/2] smb: client: disable directory caching when dir_cache_timeout is zero

[Henrique Carvalho]

On Fri, Nov 22, 2024 at 07:55:35PM GMT, Paulo Alcantara wrote:
> Henrique Carvalho <henrique.carvalho@suse.com> writes:
> 
> > On Fri, Nov 22, 2024 at 07:07:06PM GMT, Paulo Alcantara wrote:
> >> Henrique Carvalho <henrique.carvalho@suse.com> writes:
> >> 
> >> > According to the dir_cache_timeout description, setting it to zero
> >> > should disable the caching of directory contents. However, even when
> >> > dir_cache_timeout is zero, some caching related functions are still
> >> > invoked, and the worker thread is initiated, which is unintended
> >> > behavior.
> >> >
> >> > Fix the issue by setting tcon->nohandlecache to true when
> >> > dir_cache_timeout is zero, ensuring that directory handle caching
> >> > is properly disabled.
> >> >
> >> > Clean up the code to reflect this change, to improve consistency,
> >> > and to remove other unnecessary checks.
> >> >
> >> > is_smb1_server() check inside open_cached_dir() can be removed because
> >> > dir caching is only enabled for SMB versions >= 2.0.
> >> >
> >> > Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
> >> > ---
> >> >  fs/smb/client/cached_dir.c | 12 +++++++-----
> >> >  fs/smb/client/cifsproto.h  |  2 +-
> >> >  fs/smb/client/connect.c    | 10 +++++-----
> >> >  fs/smb/client/misc.c       |  4 ++--
> >> >  4 files changed, 15 insertions(+), 13 deletions(-)
> >> 
> >> The fix could be simply this:
> >> 
> >> 	diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> >> 	index b227d61a6f20..62a29183c655 100644
> >> 	--- a/fs/smb/client/connect.c
> >> 	+++ b/fs/smb/client/connect.c
> >> 	@@ -2614,7 +2614,7 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
> >> 	 
> >> 	 	if (ses->server->dialect >= SMB20_PROT_ID &&
> >> 	 	    (ses->server->capabilities & SMB2_GLOBAL_CAP_DIRECTORY_LEASING))
> >> 	-		nohandlecache = ctx->nohandlecache;
> >> 	+		nohandlecache = ctx->nohandlecache || !dir_cache_timeout;
> >> 	 	else
> >> 	 		nohandlecache = true;
> >> 	 	tcon = tcon_info_alloc(!nohandlecache, netfs_trace_tcon_ref_new);
> >> 
> >> and easily backported to -stable kernels that have
> >> 
> >>         238b351d0935 ("smb3: allow controlling length of time directory entries are cached with dir leases")
> >>
> >
> > Then I could split this into two separate patches, one with the fix for
> > the mentioned commit, and another patch for the changes in cached_dir.c
> > (which I still make sense to have).
> 
> Removing is_smb1_server() check looks good, but the other changes don't
> make much sense as we could potentially return -EOPNOTSUPP in
> cifs_readdir(), for example, and -ENOENT is probably what you want.
> 
> Am I missing something?

I might be missing something, but only place I'm changing the return
value is in open_cached_dir_by_dentry() so it is consistent with
open_cached_dir().

open_cached_dir() already returns -EOPNOTSUPP for these checks:

 if (tcon == NULL || tcon->cfids == NULL || tcon->nohandlecache ||
     is_smb1_server(tcon->ses->server) || (dir_cache_timeout == 0))
         return -EOPNOTSUPP;


The changes in this function relate to removing unnecessary checks.

--
Henrique

Re: [PATCH 1/2] smb: client: disable directory caching when dir_cache_timeout is zero

[Paulo Alcantara]

Henrique Carvalho <henrique.carvalho@suse.com> writes:

> On Fri, Nov 22, 2024 at 07:55:35PM GMT, Paulo Alcantara wrote:
>> Henrique Carvalho <henrique.carvalho@suse.com> writes:
>> 
>> > On Fri, Nov 22, 2024 at 07:07:06PM GMT, Paulo Alcantara wrote:
>> >> Henrique Carvalho <henrique.carvalho@suse.com> writes:
>> >> 
>> >> > According to the dir_cache_timeout description, setting it to zero
>> >> > should disable the caching of directory contents. However, even when
>> >> > dir_cache_timeout is zero, some caching related functions are still
>> >> > invoked, and the worker thread is initiated, which is unintended
>> >> > behavior.
>> >> >
>> >> > Fix the issue by setting tcon->nohandlecache to true when
>> >> > dir_cache_timeout is zero, ensuring that directory handle caching
>> >> > is properly disabled.
>> >> >
>> >> > Clean up the code to reflect this change, to improve consistency,
>> >> > and to remove other unnecessary checks.
>> >> >
>> >> > is_smb1_server() check inside open_cached_dir() can be removed because
>> >> > dir caching is only enabled for SMB versions >= 2.0.
>> >> >
>> >> > Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
>> >> > ---
>> >> >  fs/smb/client/cached_dir.c | 12 +++++++-----
>> >> >  fs/smb/client/cifsproto.h  |  2 +-
>> >> >  fs/smb/client/connect.c    | 10 +++++-----
>> >> >  fs/smb/client/misc.c       |  4 ++--
>> >> >  4 files changed, 15 insertions(+), 13 deletions(-)
>> >> 
>> >> The fix could be simply this:
>> >> 
>> >> 	diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
>> >> 	index b227d61a6f20..62a29183c655 100644
>> >> 	--- a/fs/smb/client/connect.c
>> >> 	+++ b/fs/smb/client/connect.c
>> >> 	@@ -2614,7 +2614,7 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
>> >> 	 
>> >> 	 	if (ses->server->dialect >= SMB20_PROT_ID &&
>> >> 	 	    (ses->server->capabilities & SMB2_GLOBAL_CAP_DIRECTORY_LEASING))
>> >> 	-		nohandlecache = ctx->nohandlecache;
>> >> 	+		nohandlecache = ctx->nohandlecache || !dir_cache_timeout;
>> >> 	 	else
>> >> 	 		nohandlecache = true;
>> >> 	 	tcon = tcon_info_alloc(!nohandlecache, netfs_trace_tcon_ref_new);
>> >> 
>> >> and easily backported to -stable kernels that have
>> >> 
>> >>         238b351d0935 ("smb3: allow controlling length of time directory entries are cached with dir leases")
>> >>
>> >
>> > Then I could split this into two separate patches, one with the fix for
>> > the mentioned commit, and another patch for the changes in cached_dir.c
>> > (which I still make sense to have).
>> 
>> Removing is_smb1_server() check looks good, but the other changes don't
>> make much sense as we could potentially return -EOPNOTSUPP in
>> cifs_readdir(), for example, and -ENOENT is probably what you want.
>> 
>> Am I missing something?
>
> I might be missing something, but only place I'm changing the return
> value is in open_cached_dir_by_dentry() so it is consistent with
> open_cached_dir().
>
> open_cached_dir() already returns -EOPNOTSUPP for these checks:
>
>  if (tcon == NULL || tcon->cfids == NULL || tcon->nohandlecache ||
>      is_smb1_server(tcon->ses->server) || (dir_cache_timeout == 0))
>          return -EOPNOTSUPP;
>
>
> The changes in this function relate to removing unnecessary checks.

Sounds good.
>
> --
> Henrique

Re: [PATCH 1/2] smb: client: disable directory caching when dir_cache_timeout is zero

[Henrique Carvalho]

On Fri, Nov 22, 2024 at 08:59:53PM GMT, Paulo Alcantara wrote:
> Henrique Carvalho <henrique.carvalho@suse.com> writes:
> 
> > On Fri, Nov 22, 2024 at 07:55:35PM GMT, Paulo Alcantara wrote:
> >> Henrique Carvalho <henrique.carvalho@suse.com> writes:
> >> 
> >> > On Fri, Nov 22, 2024 at 07:07:06PM GMT, Paulo Alcantara wrote:
> >> >> Henrique Carvalho <henrique.carvalho@suse.com> writes:
> >> >> 
> >> >> > According to the dir_cache_timeout description, setting it to zero
> >> >> > should disable the caching of directory contents. However, even when
> >> >> > dir_cache_timeout is zero, some caching related functions are still
> >> >> > invoked, and the worker thread is initiated, which is unintended
> >> >> > behavior.
> >> >> >
> >> >> > Fix the issue by setting tcon->nohandlecache to true when
> >> >> > dir_cache_timeout is zero, ensuring that directory handle caching
> >> >> > is properly disabled.
> >> >> >
> >> >> > Clean up the code to reflect this change, to improve consistency,
> >> >> > and to remove other unnecessary checks.
> >> >> >
> >> >> > is_smb1_server() check inside open_cached_dir() can be removed because
> >> >> > dir caching is only enabled for SMB versions >= 2.0.
> >> >> >
> >> >> > Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
> >> >> > ---
> >> >> >  fs/smb/client/cached_dir.c | 12 +++++++-----
> >> >> >  fs/smb/client/cifsproto.h  |  2 +-
> >> >> >  fs/smb/client/connect.c    | 10 +++++-----
> >> >> >  fs/smb/client/misc.c       |  4 ++--
> >> >> >  4 files changed, 15 insertions(+), 13 deletions(-)
> >> >> 
> >> >> The fix could be simply this:
> >> >> 
> >> >> 	diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> >> >> 	index b227d61a6f20..62a29183c655 100644
> >> >> 	--- a/fs/smb/client/connect.c
> >> >> 	+++ b/fs/smb/client/connect.c
> >> >> 	@@ -2614,7 +2614,7 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
> >> >> 	 
> >> >> 	 	if (ses->server->dialect >= SMB20_PROT_ID &&
> >> >> 	 	    (ses->server->capabilities & SMB2_GLOBAL_CAP_DIRECTORY_LEASING))
> >> >> 	-		nohandlecache = ctx->nohandlecache;
> >> >> 	+		nohandlecache = ctx->nohandlecache || !dir_cache_timeout;
> >> >> 	 	else
> >> >> 	 		nohandlecache = true;
> >> >> 	 	tcon = tcon_info_alloc(!nohandlecache, netfs_trace_tcon_ref_new);
> >> >> 
> >> >> and easily backported to -stable kernels that have
> >> >> 
> >> >>         238b351d0935 ("smb3: allow controlling length of time directory entries are cached with dir leases")
> >> >>
> >> >
> >> > Then I could split this into two separate patches, one with the fix for
> >> > the mentioned commit, and another patch for the changes in cached_dir.c
> >> > (which I still make sense to have).
> >> 
> >> Removing is_smb1_server() check looks good, but the other changes don't
> >> make much sense as we could potentially return -EOPNOTSUPP in
> >> cifs_readdir(), for example, and -ENOENT is probably what you want.
> >> 
> >> Am I missing something?
> >
> > I might be missing something, but only place I'm changing the return
> > value is in open_cached_dir_by_dentry() so it is consistent with
> > open_cached_dir().
> >
> > open_cached_dir() already returns -EOPNOTSUPP for these checks:
> >
> >  if (tcon == NULL || tcon->cfids == NULL || tcon->nohandlecache ||
> >      is_smb1_server(tcon->ses->server) || (dir_cache_timeout == 0))
> >          return -EOPNOTSUPP;
> >
> >
> > The changes in this function relate to removing unnecessary checks.
> 
> Sounds good.

Resent. Thanks a lot Paulo!