Henrique Carvalho <henrique.carvalho@xxxxxxxx> writes:
> On Fri, Nov 22, 2024 at 07:07:06PM GMT, Paulo Alcantara wrote:
>> Henrique Carvalho <henrique.carvalho@xxxxxxxx> 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@xxxxxxxx>
>> > ---
>> > 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?
