Re: [PATCH 09/11] megaraid_sas: switch fusion adapters to MQ

From: John Garry <john.garry@huawei.com>
To: Ming Lei <ming.lei@redhat.com>
Cc: Sumit Saxena <sumit.saxena@broadcom.com>,
	Hannes Reinecke <hare@suse.de>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Jens Axboe <axboe@kernel.dk>, Christoph Hellwig <hch@lst.de>,
	James Bottomley <james.bottomley@hansenpartnership.com>,
	Linux SCSI List <linux-scsi@vger.kernel.org>,
	<linux-block@vger.kernel.org>, Hannes Reinecke <hare@suse.com>
Subject: Re: [PATCH 09/11] megaraid_sas: switch fusion adapters to MQ
Date: Tue, 14 Jan 2020 13:57:08 +0000	[thread overview]
Message-ID: <71cf0932-ab93-675c-4d7c-37889c003468@huawei.com> (raw)
In-Reply-To: <1383a868-76d8-5c26-556d-7374e189b7ce@huawei.com>

On 10/01/2020 12:09, John Garry wrote:
>>>>>
>>>>> Fusion adapters can steer completions to individual queues, and
>>>>> we now have support for shared host-wide tags.
>>>>> So we can enable multiqueue support for fusion adapters and
>>>>> drop the hand-crafted interrupt affinity settings.
>>>>
>>>> Hi Hannes,
>>>>
>>>> Ming Lei also proposed similar changes in megaraid_sas driver some
>>>> time back and it had resulted in performance drop-
>>>> https://patchwork.kernel.org/patch/10969511/
>>>>
>>>> So, we will do some performance tests with this patch and update you.
>>>>
>>>
>>> Hi Sumit,
>>>
>>> I was wondering if you had a chance to do this test yet?
>>>
>>> It would be good to know, so we can try to progress this work.
>>
>> Looks most of the comment in the following link isn't addressed:
>>
>> https://lore.kernel.org/linux-block/20191129002540.GA1829@ming.t460p/
> 
> OK, but I was waiting for results first, which I hoped would not take 
> too long. They weren't forgotten, for sure. Let me check them now.

Hi Ming,

I think that your questions here were related to the shared scheduler 
tags, which was Hannes' proposal (I initially had it in v2 series, but 
dropped it for v3).

I was just content to maintain the concept of shared driver tags.

Thanks,
John

> 
>>
>>> Firstly too much((nr_hw_queues - 1) times) memory is wasted. Secondly IO
>>> latency could be increased by too deep scheduler queue depth. Finally 
>>> CPU
>>> could be wasted in the retrying of running busy hw queue.
>>>
>>> Wrt. driver tags, this patch may be worse, given the average limit for
>>> each LUN is reduced by (nr_hw_queues) times, see hctx_may_queue().
>>>
>>> Another change is bt_wait_ptr(). Before your patches, there is single
>>> .wait_index, now the number of .wait_index is changed to nr_hw_queues.
>>>
>>> Also the run queue number is increased a lot in SCSI's IO completion, 
>>> see
>>> scsi_end_request().
>>
>> I guess memory waste won't be a blocker.
> 
> Yeah, that's a trade-off. And, as I remember, memory waste does not seem 
> extreme.
> 
>>
>> But it may not be one accepted behavior to reduce average active queue
>> depth for each LUN by nr_hw_queues times, meantime scheduler queue depth
>> is increased by nr_hw_queues times, compared with single queue.
>>
> 
> Thanks,
> John