| Message ID | 1666693096-180008-2-git-send-email-john.garry@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | blk-mq/libata/scsi: SCSI driver tagging improvements Part I | expand |
On 10/25/22 19:17, John Garry wrote: > It should be possible to send reserved requests even when there is no > budget, so don't request a budget in that case. > > This comes into play when we need to allocate a reserved request from the > target device request queue for error handling for that same device. > > Signed-off-by: John Garry <john.garry@huawei.com> > --- > block/blk-mq.c | 4 +++- > drivers/scsi/scsi_lib.c | 3 ++- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/block/blk-mq.c b/block/blk-mq.c > index 260adeb2e455..d8baabb32ea4 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -1955,11 +1955,13 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list, > errors = queued = 0; > do { > struct blk_mq_queue_data bd; > + bool need_budget; > > rq = list_first_entry(list, struct request, queuelist); > > WARN_ON_ONCE(hctx != rq->mq_hctx); > - prep = blk_mq_prep_dispatch_rq(rq, !nr_budgets); > + need_budget = !nr_budgets && !blk_mq_is_reserved_rq(rq); > + prep = blk_mq_prep_dispatch_rq(rq, need_budget); > if (prep != PREP_DISPATCH_OK) > break; Below this code, there is: if (nr_budgets) nr_budgets--; Don't you need to change that to: if (need_budget && nr_budgets) nr_budgets--; ? Otherwise, the accounting will be off. > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > index fa96d3cfdfa3..39d4fd124375 100644 > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -298,7 +298,8 @@ void scsi_device_unbusy(struct scsi_device *sdev, struct scsi_cmnd *cmd) > if (starget->can_queue > 0) > atomic_dec(&starget->target_busy); > > - sbitmap_put(&sdev->budget_map, cmd->budget_token); > + if (!blk_mq_is_reserved_rq(scsi_cmd_to_rq(cmd))) > + sbitmap_put(&sdev->budget_map, cmd->budget_token); > cmd->budget_token = -1; > } >
On 27/10/2022 02:16, Damien Le Moal wrote: >> Signed-off-by: John Garry<john.garry@huawei.com> >> --- >> block/blk-mq.c | 4 +++- >> drivers/scsi/scsi_lib.c | 3 ++- >> 2 files changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/block/blk-mq.c b/block/blk-mq.c >> index 260adeb2e455..d8baabb32ea4 100644 >> --- a/block/blk-mq.c >> +++ b/block/blk-mq.c >> @@ -1955,11 +1955,13 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list, >> errors = queued = 0; >> do { >> struct blk_mq_queue_data bd; >> + bool need_budget; >> >> rq = list_first_entry(list, struct request, queuelist); >> >> WARN_ON_ONCE(hctx != rq->mq_hctx); >> - prep = blk_mq_prep_dispatch_rq(rq, !nr_budgets); >> + need_budget = !nr_budgets && !blk_mq_is_reserved_rq(rq); >> + prep = blk_mq_prep_dispatch_rq(rq, need_budget); >> if (prep != PREP_DISPATCH_OK) >> break; > Below this code, there is: > > if (nr_budgets) > nr_budgets--; > > Don't you need to change that to: > > if (need_budget && nr_budgets) > nr_budgets--; > > ? Otherwise, the accounting will be off. > Ah, yes, I think that you are right. I actually need to check nr_budgets usage further as nr_budgets initial value would be dependent on any reserved request requiring a budget (which we don't get). Thanks, John
diff --git a/block/blk-mq.c b/block/blk-mq.c index 260adeb2e455..d8baabb32ea4 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -1955,11 +1955,13 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list, errors = queued = 0; do { struct blk_mq_queue_data bd; + bool need_budget; rq = list_first_entry(list, struct request, queuelist); WARN_ON_ONCE(hctx != rq->mq_hctx); - prep = blk_mq_prep_dispatch_rq(rq, !nr_budgets); + need_budget = !nr_budgets && !blk_mq_is_reserved_rq(rq); + prep = blk_mq_prep_dispatch_rq(rq, need_budget); if (prep != PREP_DISPATCH_OK) break; diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index fa96d3cfdfa3..39d4fd124375 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -298,7 +298,8 @@ void scsi_device_unbusy(struct scsi_device *sdev, struct scsi_cmnd *cmd) if (starget->can_queue > 0) atomic_dec(&starget->target_busy); - sbitmap_put(&sdev->budget_map, cmd->budget_token); + if (!blk_mq_is_reserved_rq(scsi_cmd_to_rq(cmd))) + sbitmap_put(&sdev->budget_map, cmd->budget_token); cmd->budget_token = -1; }
It should be possible to send reserved requests even when there is no budget, so don't request a budget in that case. This comes into play when we need to allocate a reserved request from the target device request queue for error handling for that same device. Signed-off-by: John Garry <john.garry@huawei.com> --- block/blk-mq.c | 4 +++- drivers/scsi/scsi_lib.c | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-)