Re: [PATCH 3/6] cifs: add a warning when the in-flight count goes negative

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/26/2023 2:33 AM, Shyam Prasad N wrote:
On Fri, Jun 23, 2023 at 9:52 PM Tom Talpey <tom@xxxxxxxxxx> wrote:

On 6/11/2023 4:01 AM, Shyam Prasad N wrote:
On Sun, Jun 11, 2023 at 1:19 AM Steve French <smfrench@xxxxxxxxx> wrote:

should this be a warn once? Could it get very noisy?

On Fri, Jun 9, 2023 at 12:47 PM Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote:

We've seen the in-flight count go into negative with some
internal stress testing in Microsoft.

Adding a WARN when this happens, in hope of understanding
why this happens when it happens.

Signed-off-by: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
---
   fs/smb/client/smb2ops.c | 1 +
   1 file changed, 1 insertion(+)

diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 6e3be58cfe49..43162915e03c 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -91,6 +91,7 @@ smb2_add_credits(struct TCP_Server_Info *server,
                                              server->conn_id, server->hostname, *val,
                                              add, server->in_flight);
          }
+       WARN_ON(server->in_flight == 0);
          server->in_flight--;
          if (server->in_flight == 0 &&
             ((optype & CIFS_OP_MASK) != CIFS_NEG_OP) &&
--
2.34.1



--
Thanks,

Steve

Makes sense. We can have a warn once.

Which sounds great, but isn't this connection basically toast?
It's not super helpful to just whine. Why not clamp it at zero?

Tom.

So there's no "legal" way that this count can go negative.
If it has, that's definitely because there's a bug. The WARN will
hopefully help us catch and fix the bug.

To be clear, I'm ok with the warn, it's the fact that the code
goes to all the trouble to say it but doesn't do anything to
recover.

We could also have a clamp at 0. I'll send an updated patch.

Sounds good.

Tom.



[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux