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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.