Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 00/13] ibmvfc: initial MQ development
To:     Hannes Reinecke <hare@suse.de>,
        james.bottomley@hansenpartnership.com
Cc:     martin.petersen@oracle.com, linux-scsi@vger.kernel.org,
        linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
        brking@linux.ibm.com
References: <20201126014824.123831-1-tyreld@linux.ibm.com>
 <90e9a8ac-d2b9-bb64-7c7d-607adaea0f26@suse.de>
From:   Tyrel Datwyler <tyreld@linux.ibm.com>
Message-ID: <1d5ec685-7160-52da-417b-23a53bcfc47e@linux.ibm.com>
Date:   Wed, 2 Dec 2020 09:19:12 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.12.0
MIME-Version: 1.0
In-Reply-To: <90e9a8ac-d2b9-bb64-7c7d-607adaea0f26@suse.de>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 12/2/20 4:03 AM, Hannes Reinecke wrote:
> On 11/26/20 2:48 AM, Tyrel Datwyler wrote:
>> Recent updates in pHyp Firmware and VIOS releases provide new infrastructure
>> towards enabling Subordinate Command Response Queues (Sub-CRQs) such that each
>> Sub-CRQ is a channel backed by an actual hardware queue in the FC stack on the
>> partner VIOS. Sub-CRQs are registered with the firmware via hypercalls and then
>> negotiated with the VIOS via new Management Datagrams (MADs) for channel setup.
>>
>> This initial implementation adds the necessary Sub-CRQ framework and implements
>> the new MADs for negotiating and assigning a set of Sub-CRQs to associated VIOS
>> HW backed channels. The event pool and locking still leverages the legacy single
>> queue implementation, and as such lock contention is problematic when increasing
>> the number of queues. However, this initial work demonstrates a 1.2x factor
>> increase in IOPs when configured with two HW queues despite lock contention.
>>
> Why do you still hold the hold lock during submission?

Proof of concept.

> An initial check on the submission code path didn't reveal anything obvious, so
> it _should_ be possible to drop the host lock there.

Its used to protect the event pool and the event free/sent lists. This could
probably have its own lock instead of the host lock.

> Or at least move it into the submission function itself to avoid lock
> contention. Hmm?

I have a followup patch to do that, but I didn't see any change in performance.
I've got another patch I'm finishing that provides dedicated event pools for
each subqueue such that they will no longer have any dependency on the host lock.

-Tyrel

> 
> Cheers,
> 
> Hannes