Re: [PATCH] IB/iser: skip an unregistered direction on remote invalidation
Yehyeong Lee <[email protected]>
| Newsgroups | org.kernel.vger.linux-rdma,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
The AI review on this patch asks two things: whether a bogus remote invalidation is now silently accepted, and whether that desynchronises the MR state. The first is true by design; the second does not happen. need_inval lives in the ib_mr inside struct iser_fr_desc (iscsi_iser.h:320 and :333). With no descriptor there is no flag to clear, and the cleanup path already returns early on the same condition this patch adds - iser_unreg_mem_fastreg() at iser_memory.c:156. A need_inval left true is cleared by the next registration, which posts LOCAL_INV ahead of the registration work request (iser_memory.c:316). The state that would be wrong is need_inval false with a live MR, and skipping never clears it. With the default always_register, desc is NULL only for a write sent entirely as immediate data (iser_memory.c:364, iser_initiator.c:105), and that path advertises no stag (iser_initiator.c:116), so a conforming target has nothing to invalidate for such a task. Returning an error would fail the whole connection (iser_initiator.c:657). Two corrections to what I wrote earlier. The claim that returning an error would break a bidirectional command, which I made under the --- and again in a follow-up note, is wrong: ISCSI_FLAG_CMD_READ and ISCSI_FLAG_CMD_WRITE are set in one if/else in libiscsi.c:368-417, so they cannot both be set. And the eight-versus-132 and ten-versus-three counts in that note were each from a single run; two other runs of the error-returning build saw three invalidations and no rejections. Best regards, Yehyeong Lee