diff mbox series

[3/9] blk-mq: Relocate shared sbitmap resize in blk_mq_update_nr_requests()

Message ID 1626275195-215652-4-git-send-email-john.garry@huawei.com (mailing list archive)
State New, archived
Headers show
Series blk-mq: Reduce static requests memory footprint for shared sbitmap | expand

Commit Message

John Garry July 14, 2021, 3:06 p.m. UTC
For shared sbitmap, if the call to blk_mq_tag_update_depth() was
successful for any hctx when hctx->sched_tags is not set, then it would be
successful for all (due to nature in which blk_mq_tag_update_depth()
fails).

As such, there is no need to call blk_mq_tag_resize_shared_sbitmap() for
each hctx. So relocate the call until after the hctx iteration under the
!q->elevator check, which is equivalent (to !hctx->sched_tags).

Signed-off-by: John Garry <john.garry@huawei.com>
---
 block/blk-mq.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Ming Lei July 20, 2021, 7:50 a.m. UTC | #1
On Wed, Jul 14, 2021 at 11:06:29PM +0800, John Garry wrote:
> For shared sbitmap, if the call to blk_mq_tag_update_depth() was
> successful for any hctx when hctx->sched_tags is not set, then it would be
> successful for all (due to nature in which blk_mq_tag_update_depth()
> fails).
> 
> As such, there is no need to call blk_mq_tag_resize_shared_sbitmap() for
> each hctx. So relocate the call until after the hctx iteration under the
> !q->elevator check, which is equivalent (to !hctx->sched_tags).
> 
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
>  block/blk-mq.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index ae28f470893c..56e3c6fdba60 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -3624,8 +3624,6 @@ int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
>  		if (!hctx->sched_tags) {
>  			ret = blk_mq_tag_update_depth(hctx, &hctx->tags, nr,
>  							false);
> -			if (!ret && blk_mq_is_sbitmap_shared(set->flags))
> -				blk_mq_tag_resize_shared_sbitmap(set, nr);
>  		} else {
>  			ret = blk_mq_tag_update_depth(hctx, &hctx->sched_tags,
>  							nr, true);
> @@ -3643,9 +3641,14 @@ int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
>  	}
>  	if (!ret) {
>  		q->nr_requests = nr;
> -		if (q->elevator && blk_mq_is_sbitmap_shared(set->flags))
> -			sbitmap_queue_resize(&q->sched_bitmap_tags,
> -					     nr - set->reserved_tags);
> +		if (blk_mq_is_sbitmap_shared(set->flags)) {
> +			if (q->elevator) {
> +				sbitmap_queue_resize(&q->sched_bitmap_tags,
> +						     nr - set->reserved_tags);
> +			} else {
> +				blk_mq_tag_resize_shared_sbitmap(set, nr);
> +			}

The above two {} can be removed.
John Garry July 20, 2021, 8:06 a.m. UTC | #2
On 20/07/2021 08:50, Ming Lei wrote:
>> Signed-off-by: John Garry<john.garry@huawei.com>
>> ---
>>   block/blk-mq.c | 13 ++++++++-----
>>   1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>> index ae28f470893c..56e3c6fdba60 100644
>> --- a/block/blk-mq.c
>> +++ b/block/blk-mq.c
>> @@ -3624,8 +3624,6 @@ int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
>>   		if (!hctx->sched_tags) {
>>   			ret = blk_mq_tag_update_depth(hctx, &hctx->tags, nr,
>>   							false);
>> -			if (!ret && blk_mq_is_sbitmap_shared(set->flags))
>> -				blk_mq_tag_resize_shared_sbitmap(set, nr);
>>   		} else {
>>   			ret = blk_mq_tag_update_depth(hctx, &hctx->sched_tags,
>>   							nr, true);
>> @@ -3643,9 +3641,14 @@ int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
>>   	}
>>   	if (!ret) {
>>   		q->nr_requests = nr;
>> -		if (q->elevator && blk_mq_is_sbitmap_shared(set->flags))
>> -			sbitmap_queue_resize(&q->sched_bitmap_tags,
>> -					     nr - set->reserved_tags);
>> +		if (blk_mq_is_sbitmap_shared(set->flags)) {

Hi Ming,

>> +			if (q->elevator) {
>> +				sbitmap_queue_resize(&q->sched_bitmap_tags,
>> +						     nr - set->reserved_tags);

I have learned that some people prefer {} for multi-line single 
statements, like this.

Anyway, more code is added here later in the series, so better to add {} 
now, rather than re-arrange code later.

>> +			} else {
>> +				blk_mq_tag_resize_shared_sbitmap(set, nr);
>> +			}
> The above two {} can be removed.

Thanks,
John
diff mbox series

Patch

diff --git a/block/blk-mq.c b/block/blk-mq.c
index ae28f470893c..56e3c6fdba60 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3624,8 +3624,6 @@  int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
 		if (!hctx->sched_tags) {
 			ret = blk_mq_tag_update_depth(hctx, &hctx->tags, nr,
 							false);
-			if (!ret && blk_mq_is_sbitmap_shared(set->flags))
-				blk_mq_tag_resize_shared_sbitmap(set, nr);
 		} else {
 			ret = blk_mq_tag_update_depth(hctx, &hctx->sched_tags,
 							nr, true);
@@ -3643,9 +3641,14 @@  int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
 	}
 	if (!ret) {
 		q->nr_requests = nr;
-		if (q->elevator && blk_mq_is_sbitmap_shared(set->flags))
-			sbitmap_queue_resize(&q->sched_bitmap_tags,
-					     nr - set->reserved_tags);
+		if (blk_mq_is_sbitmap_shared(set->flags)) {
+			if (q->elevator) {
+				sbitmap_queue_resize(&q->sched_bitmap_tags,
+						     nr - set->reserved_tags);
+			} else {
+				blk_mq_tag_resize_shared_sbitmap(set, nr);
+			}
+		}
 	}
 
 	blk_mq_unquiesce_queue(q);