> On Oct 26, 2017, at 11:42 AM, J. Bruce Fields <bfields@xxxxxxxxxx> wrote:
>
> On Wed, Oct 25, 2017 at 04:03:07PM -0400, Chuck Lever wrote:
>> Hi Bruce-
>>
>> svc_rdma_transport.c :: svc_rdma_wc_send has this snippet:
>>
>> 355 ctxt = container_of(cqe, struct svc_rdma_op_ctxt, cqe);
>> 356 svc_rdma_unmap_dma(ctxt);
>> 357 svc_rdma_put_context(ctxt, 1);
>> 358
>> 359 if (unlikely(wc->status != IB_WC_SUCCESS)) {
>> 360 set_bit(XPT_CLOSE, &xprt->sc_xprt.xpt_flags);
>> 361 if (wc->status != IB_WC_WR_FLUSH_ERR)
>> 362 pr_err("svcrdma: Send: %s (%u/0x%x)\n",
>> 363 ib_wc_status_msg(wc->status),
>> 364 wc->status, wc->vendor_err);
>> 365 }
>>
>> This releases resources that were used to send a Reply. It's
>> the last thing that is done for each RPC.
>>
>> I'm wondering about the set_bit(XPT_CLOSE). In order to
>> guarantee that the svc layer will see this bit, should this
>> code also call svc_enqueue_xprt?
>>
>> There are several other places in the svc_rdma code that set
>> XPT_CLOSE but do not also enqueue the xprt.
>
> Yes, that sounds wrong to me. You may just want to check to make sure
> that's not already done later by callers?
The above sample is the last code that executes as part of
an RPC. If the client sends no more requests, then I can't
see where there is another opportunity to invoke
svc_enqueue_xprt. I suspect many of the other sites that
set XPT_CLOSE are similar.
Was also wondering if svc_xprt_close would be a better
choice here.
> (In which case it might still be worth a comment here.)
>
> I can't think of any reason *not* to call svc_xprt_enqueue. It should
> be safe to call from almost any context.
These completion handlers are now running in process
context (ib_poll workqueue), thankfully.
--
Chuck Lever
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
