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] iommu/arm-smmu-v3: poll cmdq until it has space
Thread-Topic: [PATCH] iommu/arm-smmu-v3: poll cmdq until it has space
From:   "Sieber, Fernand" <sieberf@amazon.com>
To:     John Garry <john.garry@huawei.com>,
        "will@kernel.org" <will@kernel.org>,
        "robin.murphy@arm.com" <robin.murphy@arm.com>
CC:     "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Thread-Index: AQHXrt3+ZWYnaFhO3kmkBSrWrVwBSauurBkAgAAMX78=
Date:   Tue, 21 Sep 2021 17:12:25 +0000
Message-ID: <1632244345526.44611@amazon.com>
References: <20210921114338.1144521-1-sieberf@amazon.com>,<c1c10203-ffd3-25f9-f2c6-9cee3458aac9@huawei.com>
In-Reply-To: <c1c10203-ffd3-25f9-f2c6-9cee3458aac9@huawei.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Precedence: bulk

Hi John,=0A=
=0A=
> But is the polarity really correct? That is, if we don't have space,=0A=
> then exit with success (the function to check for space).=0A=
=0A=
You are absolutely correct, this is a mistake that I made as I was resolvin=
g conflicts while porting this patch to iommu/next from 5.4 where I impleme=
nted and tested it.=0A=
It should be:=0A=
=0A=
> -             if (!queue_full(llq))=0A=
> +             if (queue_has_space(llq, n))=0A=
=0A=
=0A=
> what is llq->state->val?=0A=
=0A=
This is an other oversight for the same reason, llq->state->val has since t=
hen been renamed llq->val=0A=
=0A=
Will fix both of these in the next revision.=0A=
Thanks and kind regards,=0A=
=0A=
--Fernand=0A=
=0A=
________________________________________=0A=
From: John Garry <john.garry@huawei.com>=0A=
Sent: Tuesday, September 21, 2021 18:22=0A=
To: Sieber, Fernand; will@kernel.org; robin.murphy@arm.com=0A=
Cc: linux-arm-kernel@lists.infradead.org; iommu@lists.linux-foundation.org;=
 linux-kernel@vger.kernel.org=0A=
Subject: RE: [EXTERNAL] [PATCH] iommu/arm-smmu-v3: poll cmdq until it has s=
pace=0A=
=0A=
CAUTION: This email originated from outside of the organization. Do not cli=
ck links or open attachments unless you can confirm the sender and know the=
 content is safe.=0A=
=0A=
=0A=
=0A=
On 21/09/2021 12:43, Fernand Sieber wrote:=0A=
>       do {=0A=
=0A=
I didn't follow the full logic of this change yet ...=0A=
=0A=
>               llq->val =3D READ_ONCE(cmdq->q.llq.val);=0A=
> -             if (!queue_full(llq))=0A=
> +             if (!queue_has_space(llq, n))=0A=
=0A=
But is the polarity really correct? That is, if we don't have space,=0A=
then exit with success (the function to check for space).=0A=
=0A=
>                       break;=0A=
>=0A=
> +             /*=0A=
> +              * We must return here even if there's no space, because th=
e producer=0A=
> +              * having moved forward could mean that the last thread obs=
erving the=0A=
> +              * SMMU progress has allocated space in the cmdq and moved =
on, leaving=0A=
> +              * us in this waiting loop with no other thread updating=0A=
> +              * llq->state->val.=0A=
=0A=
what is llq->state->val?=0A=
=0A=
> +              */=0A=
> +             if (llq->prod !=3D prod)=0A=
> +                     return -EAGAIN;=0A=
> +=0A=
>               ret =3D queue_poll(&qp);=0A=
=0A=
Thanks,=0A=
John=0A=