[PATCH] cifs: Changes made to crediting code to make debugging easier.

[Shyam Prasad N]

Hi all,

While debugging an out-of-credits scenario recently, I made some
changes (with help from Pavel and Steve) in-order to make debugging
easier from the client side. Attached the patch; Please review.

Specifically, I keen on your views on the following:
@@ -1159,7 +1181,9 @@ compound_send_recv(const unsigned int xid,
struct cifs_ses *ses,
        /*
         * Compounding is never used during session establish.
         */
-       if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP))
+       if ((ses->status == CifsNew) ||
+                       (optype & CIFS_NEG_OP) ||
+                       (optype & CIFS_SESS_OP))
                smb311_update_preauth_hash(ses, rqst[0].rq_iov,
                                           rqst[0].rq_nvec);

@@ -1224,7 +1248,9 @@ compound_send_recv(const unsigned int xid,
struct cifs_ses *ses,
        /*
         * Compounding is never used during session establish.
         */
-       if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP)) {
+       if ((ses->status == CifsNew) ||
+                       (optype & CIFS_NEG_OP) ||
+                       (optype & CIFS_SESS_OP)) {
                struct kvec iov = {
                        .iov_base = resp_iov[0].iov_base,
                        .iov_len = resp_iov[0].iov_len
@Steve French @ronnie sahlberg I was not sure if this gets called
negotiate requests or just session setup. Hence retained check for
both flags. Please advise if this is okay.

@@ -97,17 +99,25 @@ smb2_add_credits(struct TCP_Server_Info *server,
-       if (server->tcpStatus == CifsNeedReconnect
-           || server->tcpStatus == CifsExiting)
-               return;

@Pavel Shilovsky This check prevented a tracepoint from getting
printed. I do not see much value in these lines, since all we do is
print the tracepoint and exit. Hence removing it. Please let me know
if that is not okay.

        while (1) {
                if (*credits < num_credits) {
+                       scredits = *credits;
                        spin_unlock(&server->req_lock);
+
                        cifs_num_waiters_inc(server);
                        rc = wait_event_killable_timeout(server->request_q,
                                has_credits(server, credits, num_credits), t);
                        cifs_num_waiters_dec(server);
                        if (!rc) {
+                               spin_lock(&server->req_lock);
+                               scredits = *credits;
+                               sin_flight = server->in_flight;
+                               spin_unlock(&server->req_lock);
+
                                trace_smb3_credit_timeout(server->CurrentMid,
-                                       server->hostname, num_credits, 0);
+                                               server->conn_id,
server->hostname, scredits, num_credits, sin_flight);
                                cifs_server_dbg(VFS, "wait timed out
after %d ms\n",
-                                        timeout);
-                               return -ENOTSUPP;
+                                               timeout);
+                               return -EBUSY;
                        }

@Steve French Replacing ENOTSUPP here with EBUSY. My reasoning for it
is in the commit message. Doing this only for the case of
credit_timeout. For insufficient_credits, I'm keeping ENOTSUPP, since
that is clearly a buggy situation and needs to be flagged.

Please let me know if I'm missing something.

-- 
-Shyam
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-cifs-Changes-made-to-crediting-code-to-make-debuggin.patch
Type: application/octet-stream
Size: 26518 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20210130/80e78763/0001-cifs-Changes-made-to-crediting-code-to-make-debuggin.obj>