On 4/8/21 5:48 AM, John Garry wrote:
> The following does not look atomic safe with the mutex usage:
> drivers/block/nbd.c:819: blk_mq_tagset_busy_iter(&nbd->tag_set,
> nbd_clear_req, NULL);
>
> static bool nbd_clear_req(struct request *req, void *data, bool reserved)
> {
> struct nbd_cmd *cmd = blk_mq_rq_to_pdu(req);
>
> mutex_lock(&cmd->lock);
> cmd->status = BLK_STS_IOERR;
> mutex_unlock(&cmd->lock);
>
> blk_mq_complete_request(req);
> return true;
> }
>
> But blk_mq_tagset_busy_iter() uses BT_TAG_ITER_MAY sleep flag in your
> series.
I will mention the nbd driver in the commit message.
> As for the fc, I am not sure. I assume that you would know more about
> this. My concern is
>
> __nvme_fc_abort_op(struct nvme_fc_ctrl *ctrl, struct nvme_fc_fcp_op *op)
> {
> ...
>
> ctrl->lport->ops->fcp_abort(&ctrl->lport->localport, ..);
> }
>
> Looking at many instances of fcp_abort callback, they look atomic safe
> from general high usage of spinlock, but I am not certain. They are
> quite complex.
I have not tried to analyze whether or not it is safe to call
__nvme_fc_abort_op() from an atomic context. Instead I analyzed the
contexts from which this function is called, namely the
blk_mq_tagset_busy_iter() calls in __nvme_fc_abort_outstanding_ios() and
__nvme_fc_abort_outstanding_ios(). Both blk_mq_tagset_busy_iter() calls
are followed by a call to a function that may sleep. Hence it is safe to
sleep inside the blk_mq_tagset_busy_iter() calls from the nvme_fc code.
I have not tried to analyze whether it would be safe to change these
blk_mq_tagset_busy_iter() calls into blk_mq_tagset_busy_iter_atomic()
calls. Does this answer your question?
Bart.
