On 07/04/2021 19:42, Bart Van Assche wrote:
- scsi_host_busy(), scsi_host_complete_all_commands() and
scsi_host_busy_iter() are used by multiple SCSI LLDs so analyzing
whether
or not these functions may sleep is hard. Instead of performing that
analysis, make it safe to call these functions from atomic context.
Please help me understand this solution. The background is that we are
unsure if the SCSI iters callback functions may sleep. So we use the
blk_mq_all_tag_iter_atomic() iter, which tells us that we must not
sleep. And internally, it uses rcu read lock protection mechanism,
which relies on not sleeping. So it seems that we're making the SCSI
iter functions being safe in atomic context, and, as such, rely on the
iter callbacks not to sleep.
But if we call the SCSI iter function from non-atomic context and the
iter callback may sleep, then that is a problem, right? We're still
using rcu.
Hi Bart,
Please take a look at the output of the following grep command:
git grep -nHEw 'blk_mq_tagset_busy_iter|scsi_host_busy_iter'\ drivers/scsi
Do you agree with me that it is safe to call all the callback functions
passed to blk_mq_tagset_busy_iter() and scsi_host_busy_iter() from an
atomic context?
That list output I got is at the bottom (with this patchset, not
mainline - not sure which you meant).
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.
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.
Thanks,
John
block/blk-core.c:287: * blk_mq_tagset_busy_iter() and also for their
atomic variants to finish
block/blk-mq-debugfs.c:418:
blk_mq_tagset_busy_iter(hctx->queue->tag_set, hctx_show_busy_rq,
block/blk-mq-tag.c:405: * blk_mq_tagset_busy_iter - iterate over all
started requests in a tag set
block/blk-mq-tag.c:416:void blk_mq_tagset_busy_iter(struct
blk_mq_tag_set *tagset,
block/blk-mq-tag.c:436:EXPORT_SYMBOL(blk_mq_tagset_busy_iter);
drivers/block/mtip32xx/mtip32xx.c:2685:
blk_mq_tagset_busy_iter(&dd->tags, mtip_queue_cmd, dd);
drivers/block/mtip32xx/mtip32xx.c:2690:
blk_mq_tagset_busy_iter(&dd->tags,
drivers/block/mtip32xx/mtip32xx.c:3800:
blk_mq_tagset_busy_iter(&dd->tags, mtip_no_dev_cleanup, dd);
drivers/block/nbd.c:819: blk_mq_tagset_busy_iter(&nbd->tag_set,
nbd_clear_req, NULL);
drivers/nvme/host/core.c:392:
blk_mq_tagset_busy_iter(ctrl->tagset,
drivers/nvme/host/core.c:402:
blk_mq_tagset_busy_iter(ctrl->admin_tagset,
drivers/nvme/host/fc.c:2477:
blk_mq_tagset_busy_iter(&ctrl->tag_set,
drivers/nvme/host/fc.c:2500:
blk_mq_tagset_busy_iter(&ctrl->admin_tag_set,
drivers/nvme/host/pci.c:2504: blk_mq_tagset_busy_iter(&dev->tagset,
nvme_cancel_request, &dev->ctrl);
drivers/nvme/host/pci.c:2505:
blk_mq_tagset_busy_iter(&dev->admin_tagset, nvme_cancel_request,
&dev->ctrl);
drivers/nvme/target/loop.c:420:
blk_mq_tagset_busy_iter(&ctrl->tag_set,
drivers/nvme/target/loop.c:430:
blk_mq_tagset_busy_iter(&ctrl->admin_tag_set,
include/linux/blk-mq.h:527:void blk_mq_tagset_busy_iter(struct
blk_mq_tag_set *tagset,
lines 1-18/18 (END)
