Re: Possible bug report: kernel 6.5.0/6.5.1 high load when CIFS share is mounted (cifsd-cfid-laundromat in"D" state)

[Tom Talpey <tom@talpey.com>]

On 9/19/2023 9:38 AM, Steve French wrote:
> Minor updates (pointed out by Paulo) to patch. See attached.

So, was the thread crashing before??

+	if (cfids == NULL)
+		return;
+
  	spin_lock(&cfids->cfid_list_lock);


This if/else, IMO...

@@ -2492,7 +2493,10 @@ cifs_get_tcon(struct cifs_ses *ses, struct 
smb3_fs_context *ctx)
  		goto out_fail;
  	}

-	tcon = tconInfoAlloc();
+	if (ses->server->capabilities & SMB2_GLOBAL_CAP_DIRECTORY_LEASING)
+		tcon = tcon_info_alloc(true);
+	else
+		tcon = tcon_info_alloc(false);

would be more readable as...

	tcon = tcon_info_alloc(ses->server->capabilities & 
SMB2_GLOBAL_CAP_DIRECTORY_LEASING != 0);


These changes are good, but I'm skeptical they will reduce the load
when the laundromat thread is actually running. All these do is avoid 
creating it when not necessary, right?

Acked-by: Tom Talpey <tom@talpey.com>

Tom.


> 
> On Tue, Sep 19, 2023 at 8:21 AM Brian Pardy <brian.pardy@gmail.com> wrote:
>>
>> On Tue, Sep 19, 2023 at 1:36 AM Steve French <smfrench@gmail.com> wrote:
>>>
>>> Does the attached patch help in your case?  It avoids starting the
>>> laundromat thread for IPC shares (which cuts the number of the threads
>>> in half for many cases) and also avoids starting them if the server
>>> does not support directory leases (e.g. if Samba server instead of
>>> Windows server).
>>
>> Hello,
>>
>> I applied the 0001-smb3-do-not-start-laundromat-thread-when-dir-leases-.patch
>> you provided against the 6.5.3 kernel.
>>
>> I can confirm that it resolves this issue - no laundromat threads are
>> created, and the reported load average is as expected, not falsely
>> high.
>>
>> This appears to fully fix the issue in my case.  Thank you very much!
>>
>>> On Mon, Sep 18, 2023 at 10:00 PM Steve French <smfrench@gmail.com> wrote:
>>>>
>>>> Paulo and I were discussing the laundromat thread at the SMB3.1.1 test
>>>> event (at SDC this week) which is now going on - will let you know
>>>> what we find.
>>>>
>>>> One obvious thing is that it probably isn't necessary for cases when
>>>> the server does not support directory leases, but we noticed another
>>>> problem as well.
> 
> 
>