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