diff mbox series

[RFC,v3,01/22] blk-mq: Don't get budget for reserved requests

Message ID 1666693096-180008-2-git-send-email-john.garry@huawei.com
State New
Headers
Series blk-mq/libata/scsi: SCSI driver tagging improvements Part I |

Commit Message

John Garry Oct. 25, 2022, 10:17 a.m. UTC 
  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(-)

Comments

Damien Le Moal Oct. 27, 2022, 1:16 a.m. UTC | #1 
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;
>  }
>
John Garry Oct. 27, 2022, 9:09 a.m. UTC | #2 
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 mbox series

Patch

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;
 }