From: Stefan Metzmacher metze@samba.org
[ Upstream commit a8e970358b31a5abba8b5737a67ba7b8d26f4258 ]
There's no need to get log message for every IB_WC_WR_FLUSH_ERR completion, but any other error should be logged at level ERR.
Cc: Steve French smfrench@gmail.com Cc: Tom Talpey tom@talpey.com Cc: Long Li longli@microsoft.com Cc: linux-cifs@vger.kernel.org Cc: samba-technical@lists.samba.org Acked-by: Namjae Jeon linkinjeon@kernel.org Signed-off-by: Stefan Metzmacher metze@samba.org Signed-off-by: Steve French stfrench@microsoft.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
Based on my exhaustive analysis, here is my recommendation:
## **Backport Status: YES**
### Extensive Analysis:
#### **What This Commit Does:**
This commit makes two logging improvements to the SMB Direct (RDMA) code:
1. **Human-readable error messages**: Changes from numeric status codes (`wc->status=%d`) to descriptive strings using `ib_wc_status_msg(wc->status)` (e.g., "WR flushed" instead of "6")
2. **Reduces log spam**: Adds conditional checks (`if (wc->status != IB_WC_WR_FLUSH_ERR)`) to skip logging for `IB_WC_WR_FLUSH_ERR` errors, which are benign and occur frequently during normal RDMA operations
3. **Better error visibility**: In `recv_done()` (line 607-608), changes the log level from INFO to ERR for real errors
#### **Deep Technical Context:**
**`IB_WC_WR_FLUSH_ERR` Background:** - This is a standard InfiniBand/RDMA work completion status indicating that work requests were flushed from the queue - Occurs during normal operations: QP (Queue Pair) error state transitions, connection teardown, and error recovery - **NOT an actionable error** - it's expected behavior that doesn't require logging - Other kernel RDMA drivers follow this pattern: `drivers/infiniband/core/mad.c:2366` has `if (wc->status == IB_WC_WR_FLUSH_ERR)` with special handling and no error logging
**SMB Client Logging History:** - Multiple commits address log spam in SMB client: d7cb986425ce2 "stop flooding dmesg in smb2_calc_signature()", 6bbed0b3ad8b2 "fix noisy when tree connecting" - This commit follows the same pattern - reducing noise while preserving important error information
#### **Backport Suitability Analysis:**
**✅ STRONG POSITIVE FACTORS:**
1. **Very small and safe**: Only 20 lines changed (12 insertions, 8 deletions) in a single file 2. **Logging-only changes**: No functional code paths altered - only what gets logged and how 3. **Zero dependencies**: Both `ib_wc_status_msg()` (introduced v4.2, 2015) and `IB_WC_WR_FLUSH_ERR` exist in v6.17 4. **Code compatibility**: The v6.17 send_done():275 and recv_done():450 functions match the pre-patch state exactly 5. **Trusted author**: Stefan Metzmacher is a Samba core developer with extensive SMB/CIFS expertise 6. **Maintainer approval**: Acked-by Namjae Jeon, Signed-off-by Steve French (CIFS maintainer) 7. **Real user benefit**: Reduces log spam that obscures real errors, improves observability for system administrators 8. **Industry best practice**: Aligns with how other RDMA drivers in the kernel handle IB_WC_WR_FLUSH_ERR 9. **Minimal testing burden**: Can be verified simply by observing logs during RDMA operations
**⚠️ CONSIDERATIONS:**
1. No explicit `Cc: stable@` tag (though this is common for QOL improvements) 2. Not a critical bugfix - it's a usability/observability enhancement 3. Doesn't fix crashes, data corruption, or security issues
#### **Regression Risk Assessment:**
**Risk Level: VERY LOW**
- Changes only affect logging statements - No changes to control flow, data structures, or RDMA operations - If something did go wrong (highly unlikely), worst case is missing log messages - The logic is straightforward: `if (status != FLUSH_ERR) log_error()`
#### **Specific Code Changes Analyzed:**
**send_done() fs/smb/client/smbdirect.c:415-429:** ```c - log_rdma_send(INFO, "...wc->status=%d", wc->status); + log_rdma_send(INFO, "...wc->status=%s", ib_wc_status_msg(wc->status));
- log_rdma_send(ERR, "wc->status=%d wc->opcode=%d\n", wc->status, wc->opcode); + if (wc->status != IB_WC_WR_FLUSH_ERR) + log_rdma_send(ERR, "wc->status=%s wc->opcode=%d\n", + ib_wc_status_msg(wc->status), wc->opcode); ```
**recv_done() fs/smb/client/smbdirect.c:597-608:** ```c - log_rdma_recv(INFO, "...wc status=%d...", wc->status, ...); + log_rdma_recv(INFO, "...wc status=%s...", ib_wc_status_msg(wc->status), ...);
- log_rdma_recv(INFO, "wc->status=%d opcode=%d\n", wc->status, wc->opcode); + if (wc->status != IB_WC_WR_FLUSH_ERR) + log_rdma_recv(ERR, "wc->status=%s opcode=%d\n", + ib_wc_status_msg(wc->status), wc->opcode); ```
Note the important change: ERROR level logging for recv_done (was INFO, now ERR) - this ensures real errors are more visible.
#### **Why This Should Be Backported:**
1. **Improves user experience**: System administrators using SMB Direct over RDMA will see cleaner logs with readable error messages 2. **Reduces support burden**: Less noise in logs means real errors are easier to identify 3. **Safe change**: Extremely low risk of introducing regressions 4. **Already backported elsewhere**: Evidence shows this commit (624cc9eac4e69) was already backported to another stable tree 5. **Aligns with stable tree philosophy**: Small, safe improvements that benefit users without risk
#### **Conclusion:**
While not a critical fix, this commit provides tangible benefits (reduced log spam, better error messages) with virtually zero risk. It improves the operational experience for anyone using SMB Direct with RDMA, which is important for high-performance SMB deployments. The change is small, safe, and comes from trusted maintainers.
**Recommendation: YES - suitable for backporting to stable trees, though not urgent priority.**
fs/smb/client/smbdirect.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/fs/smb/client/smbdirect.c b/fs/smb/client/smbdirect.c index 3fc5d2cebea5d..723f79f38364d 100644 --- a/fs/smb/client/smbdirect.c +++ b/fs/smb/client/smbdirect.c @@ -281,8 +281,8 @@ static void send_done(struct ib_cq *cq, struct ib_wc *wc) struct smbd_connection *info = container_of(sc, struct smbd_connection, socket);
- log_rdma_send(INFO, "smbdirect_send_io 0x%p completed wc->status=%d\n", - request, wc->status); + log_rdma_send(INFO, "smbdirect_send_io 0x%p completed wc->status=%s\n", + request, ib_wc_status_msg(wc->status));
for (i = 0; i < request->num_sge; i++) ib_dma_unmap_single(sc->ib.dev, @@ -291,8 +291,9 @@ static void send_done(struct ib_cq *cq, struct ib_wc *wc) DMA_TO_DEVICE);
if (wc->status != IB_WC_SUCCESS || wc->opcode != IB_WC_SEND) { - log_rdma_send(ERR, "wc->status=%d wc->opcode=%d\n", - wc->status, wc->opcode); + if (wc->status != IB_WC_WR_FLUSH_ERR) + log_rdma_send(ERR, "wc->status=%s wc->opcode=%d\n", + ib_wc_status_msg(wc->status), wc->opcode); mempool_free(request, sc->send_io.mem.pool); smbd_disconnect_rdma_connection(info); return; @@ -462,13 +463,16 @@ static void recv_done(struct ib_cq *cq, struct ib_wc *wc) u32 data_length = 0; u32 remaining_data_length = 0;
- log_rdma_recv(INFO, "response=0x%p type=%d wc status=%d wc opcode %d byte_len=%d pkey_index=%u\n", - response, sc->recv_io.expected, wc->status, wc->opcode, + log_rdma_recv(INFO, + "response=0x%p type=%d wc status=%s wc opcode %d byte_len=%d pkey_index=%u\n", + response, sc->recv_io.expected, + ib_wc_status_msg(wc->status), wc->opcode, wc->byte_len, wc->pkey_index);
if (wc->status != IB_WC_SUCCESS || wc->opcode != IB_WC_RECV) { - log_rdma_recv(INFO, "wc->status=%d opcode=%d\n", - wc->status, wc->opcode); + if (wc->status != IB_WC_WR_FLUSH_ERR) + log_rdma_recv(ERR, "wc->status=%s opcode=%d\n", + ib_wc_status_msg(wc->status), wc->opcode); goto error; }