Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <069b28e7-4ab2-47c8-cce3-186cd987834d@linux.ibm.com>
Date:   Tue, 8 Mar 2022 10:36:48 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.0
Subject: Re: [PATCH v18 08/18] s390/vfio-ap: allow assignment of unavailable
 AP queues to mdev device
Content-Language: en-US
To:     Halil Pasic <pasic@linux.ibm.com>
Cc:     jjherne@linux.ibm.com, linux-s390@vger.kernel.org,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        freude@linux.ibm.com, borntraeger@de.ibm.com, cohuck@redhat.com,
        mjrosato@linux.ibm.com, alex.williamson@redhat.com,
        kwankhede@nvidia.com, fiuczy@linux.ibm.com
References: <20220215005040.52697-1-akrowiak@linux.ibm.com>
 <20220215005040.52697-9-akrowiak@linux.ibm.com>
 <97681738-50a1-976d-9f0f-be326eab7202@linux.ibm.com>
 <9ac3908e-06da-6276-d1df-94898918fc5b@linux.ibm.com>
 <20220307142711.5af33ece.pasic@linux.ibm.com>
 <151241e6-3099-4be2-da54-1f0e5cb3a705@linux.ibm.com>
 <20220307181027.29c821b6.pasic@linux.ibm.com>
 <eb30a519-5707-717a-ff22-cc3a8e65dc7e@linux.ibm.com>
 <20220308110637.2e839732.pasic@linux.ibm.com>
From:   Tony Krowiak <akrowiak@linux.ibm.com>
In-Reply-To: <20220308110637.2e839732.pasic@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 3/8/22 05:06, Halil Pasic wrote:
> On Mon, 7 Mar 2022 18:45:45 -0500
> Tony Krowiak <akrowiak@linux.ibm.com> wrote:
>
> [..]
>>>>> s/belong to the matrix_mdev's matrix/are fields of the matrix_mdev
>>>>> object/
>>>> This is the comment I wrote:
>>>>
>>>>            /*
>>>>             * Comparing an mdev's newly updated apm/aqm with itself would
>>>>             * result in a false positive when verifying whether any APQNs
>>>>             * are shared; so, if the input apm and aqm belong to the
>>>>             * matrix_mdev's matrix, then move on to the next one.
>>>>             */
>>>>
>>>> However, I'd be happy to change it to whatever either of you want.
>>> What ain't obvious for the comment is that "belong to" actually means
>>> composition and not association. In other words, there there is no
>>> pointer/indirection involved, a pointer that would tell us what matrix
>>> does belong to what matrix_mdev, but rather the matrix is just a part
>>> of the matrix_mdev object.
>>>
>>> I don't like 'false positive' either, and whether the apm/aqm is
>>> newly updated or not is also redundant and confusing in my opinion. When
>>> we check because of inuse there is not updated whatever. IMHO the old
>>> message was better than this one.
>>>
>>> Just my opinion, if you two agree, that this is the way to go, I'm fine
>>> with that.
>>>
>>> Regards,
>>> Halil
>> Feel free to recommend the verbiage for this comment. I'm not married
>> to my comments and am open to anything that helps others to
>> understand what is going on here. It seems obvious to me, but I wrote
>> the code. Obviously, it is not so obvious based on Jason's comments,
>> so maybe someone else can compose a better comment.
> /*
> * If the input apm and aqm are fields of the matrix_mdev object,
> * then move on to the next matrix_mdev.
> */

Perfect, you write better English than me!

>
> Regards,
> Halil