[PATCH] iommu/arm-smmu-v3: Stop queue allocation retry at PAGE_SIZE

[PATCH] iommu/arm-smmu-v3: Stop queue allocation retry at PAGE_SIZE

[leo.jiang1224]

From: LoserJL <leo.jiang1224@foxmail.com>

In arm_smmu_init_one_queue(), the loop reduces max_n_shift if
dmam_alloc_coherent() fails. However, since dmam_alloc_coherent()
allocates at least PAGE_SIZE, retrying with a smaller size after
a PAGE_SIZE failure is logically redundant.

Moreover, if a sub-page retry were to succeed due to concurrent memory
release, the hardware would be configured with a smaller queue depth
despite a full page being allocated. This leads to inefficient memory
usage and unnecessary hardware performance limitation.

Terminate the loop once qsz reaches PAGE_SIZE to ensure logical
consistency and optimal hardware configuration.

Signed-off-by: LoserJL <leo.jiang1224@foxmail.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index e8d7dbe495f0..e0ec118ff560 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -4418,7 +4418,14 @@ int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
 		qsz = ((1 << q->llq.max_n_shift) * dwords) << 3;
 		q->base = dmam_alloc_coherent(smmu->dev, qsz, &q->base_dma,
 					      GFP_KERNEL);
-		if (q->base || qsz < PAGE_SIZE)
+		/*
+		 * If allocation succeeds, we're done. If it fails, only retry
+		 * if the requested size is still larger than a page. Since
+		 * dmam_alloc_coherent() allocates at least PAGE_SIZE, retrying
+		 * with a sub-page size is logically redundant and could lead
+		 * to sub-optimal hardware configuration.
+		 */
+		if (q->base || qsz <= PAGE_SIZE)
 			break;
 
 		q->llq.max_n_shift--;
-- 
2.43.0

Re: [PATCH] iommu/arm-smmu-v3: Stop queue allocation retry at PAGE_SIZE

[Will Deacon]

On Sat, Apr 18, 2026 at 01:31:43PM +0800, leo.jiang1224@foxmail.com wrote:
> From: LoserJL <leo.jiang1224@foxmail.com>
> 
> In arm_smmu_init_one_queue(), the loop reduces max_n_shift if
> dmam_alloc_coherent() fails. However, since dmam_alloc_coherent()
> allocates at least PAGE_SIZE, retrying with a smaller size after
> a PAGE_SIZE failure is logically redundant.
> 
> Moreover, if a sub-page retry were to succeed due to concurrent memory
> release, the hardware would be configured with a smaller queue depth
> despite a full page being allocated. This leads to inefficient memory
> usage and unnecessary hardware performance limitation.
> 
> Terminate the loop once qsz reaches PAGE_SIZE to ensure logical
> consistency and optimal hardware configuration.
> 
> Signed-off-by: LoserJL <leo.jiang1224@foxmail.com>

No pseudonyms, please.

> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index e8d7dbe495f0..e0ec118ff560 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -4418,7 +4418,14 @@ int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
>  		qsz = ((1 << q->llq.max_n_shift) * dwords) << 3;
>  		q->base = dmam_alloc_coherent(smmu->dev, qsz, &q->base_dma,
>  					      GFP_KERNEL);
> -		if (q->base || qsz < PAGE_SIZE)
> +		/*
> +		 * If allocation succeeds, we're done. If it fails, only retry
> +		 * if the requested size is still larger than a page. Since
> +		 * dmam_alloc_coherent() allocates at least PAGE_SIZE, retrying
> +		 * with a sub-page size is logically redundant and could lead
> +		 * to sub-optimal hardware configuration.

What do you mean by "sub-optimal hardware configuration"? I think you can
probably just drop this comment.

> +		 */
> +		if (q->base || qsz <= PAGE_SIZE)
>  			break;

I think this part is fine.

Will

Re: [PATCH] iommu/arm-smmu-v3: Stop queue allocation retry at PAGE_SIZE

[Robin Murphy]

On 18/04/2026 6:31 am, leo.jiang1224@foxmail.com wrote:
> From: LoserJL <leo.jiang1224@foxmail.com>
> 
> In arm_smmu_init_one_queue(), the loop reduces max_n_shift if
> dmam_alloc_coherent() fails. However, since dmam_alloc_coherent()
> allocates at least PAGE_SIZE, retrying with a smaller size after
> a PAGE_SIZE failure is logically redundant.

Says who? It's certainly not a guarantee offered by the DMA API itself, 
and indeed some allocation paths can definitely still allocate less than 
a page - e.g. anything which hits a per-device or global coherent pool.

> Moreover, if a sub-page retry were to succeed due to concurrent memory
> release, the hardware would be configured with a smaller queue depth
> despite a full page being allocated. This leads to inefficient memory
> usage and unnecessary hardware performance limitation.
> 
> Terminate the loop once qsz reaches PAGE_SIZE to ensure logical
> consistency and optimal hardware configuration.

That's really not an argument - even if an allocator does happen to 
over-allocate for the requested size, that is hardly the caller's 
concern; and as far as "optimal" queue sizes go in this case, those very 
much depend on the number of CPUs issuing commands and volume of 
expected stall/PRI events - in many cases PAGE_SIZE would already be far 
too small to really work well.

Also note that if we _were_ to fail to allocate a PAGE_SIZE or smaller 
queue, there would be very little chance of the subsequent allocation(s) 
for the stream table succeeding, so realistically the driver is probably 
going to end up failing to probe in such circumstances anyway.

Thanks,
Robin.

> Signed-off-by: LoserJL <leo.jiang1224@foxmail.com>
> ---
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index e8d7dbe495f0..e0ec118ff560 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -4418,7 +4418,14 @@ int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
>   		qsz = ((1 << q->llq.max_n_shift) * dwords) << 3;
>   		q->base = dmam_alloc_coherent(smmu->dev, qsz, &q->base_dma,
>   					      GFP_KERNEL);
> -		if (q->base || qsz < PAGE_SIZE)
> +		/*
> +		 * If allocation succeeds, we're done. If it fails, only retry
> +		 * if the requested size is still larger than a page. Since
> +		 * dmam_alloc_coherent() allocates at least PAGE_SIZE, retrying
> +		 * with a sub-page size is logically redundant and could lead
> +		 * to sub-optimal hardware configuration.
> +		 */
> +		if (q->base || qsz <= PAGE_SIZE)
>   			break;
>   
>   		q->llq.max_n_shift--;

Re: [PATCH] iommu/arm-smmu-v3: Stop queue allocation retry at PAGE_SIZE

[Will Deacon]

On Tue, Apr 21, 2026 at 04:56:47PM +0100, Robin Murphy wrote:
> On 18/04/2026 6:31 am, leo.jiang1224@foxmail.com wrote:
> > From: LoserJL <leo.jiang1224@foxmail.com>
> > 
> > In arm_smmu_init_one_queue(), the loop reduces max_n_shift if
> > dmam_alloc_coherent() fails. However, since dmam_alloc_coherent()
> > allocates at least PAGE_SIZE, retrying with a smaller size after
> > a PAGE_SIZE failure is logically redundant.
> 
> Says who? It's certainly not a guarantee offered by the DMA API itself, and
> indeed some allocation paths can definitely still allocate less than a page
> - e.g. anything which hits a per-device or global coherent pool.
> 
> > Moreover, if a sub-page retry were to succeed due to concurrent memory
> > release, the hardware would be configured with a smaller queue depth
> > despite a full page being allocated. This leads to inefficient memory
> > usage and unnecessary hardware performance limitation.
> > 
> > Terminate the loop once qsz reaches PAGE_SIZE to ensure logical
> > consistency and optimal hardware configuration.
> 
> That's really not an argument - even if an allocator does happen to
> over-allocate for the requested size, that is hardly the caller's concern;
> and as far as "optimal" queue sizes go in this case, those very much depend
> on the number of CPUs issuing commands and volume of expected stall/PRI
> events - in many cases PAGE_SIZE would already be far too small to really
> work well.
> 
> Also note that if we _were_ to fail to allocate a PAGE_SIZE or smaller
> queue, there would be very little chance of the subsequent allocation(s) for
> the stream table succeeding, so realistically the driver is probably going
> to end up failing to probe in such circumstances anyway.

That's all true, but tbf I think I just fscked up the comparison in
d25f6ead162e ("iommu/arm-smmu-v3: Increase maximum size of queues") so
I'm not against fixing that up even though the "rationale" given by
Loser doesn't make a whole lot of sense.

Will

Re: [PATCH] iommu/arm-smmu-v3: Stop queue allocation retry at PAGE_SIZE

[Leo Jiang]

On Tue, Apr 21, 2026 at 05:38:40PM +0100, Will Deacon wrote:

> On Tue, Apr 21, 2026 at 04:56:47PM +0100, Robin Murphy wrote:
> > On 18/04/2026 6:31 am, leo.jiang1224@foxmail.com wrote:
> > > From: LoserJL <leo.jiang1224@foxmail.com>
> > > 
> > > In arm_smmu_init_one_queue(), the loop reduces max_n_shift if
> > > dmam_alloc_coherent() fails. However, since dmam_alloc_coherent()
> > > allocates at least PAGE_SIZE, retrying with a smaller size after
> > > a PAGE_SIZE failure is logically redundant.
> > 
> > Says who? It's certainly not a guarantee offered by the DMA API itself, and
> > indeed some allocation paths can definitely still allocate less than a page
> > - e.g. anything which hits a per-device or global coherent pool.
> > 
> > > Moreover, if a sub-page retry were to succeed due to concurrent memory
> > > release, the hardware would be configured with a smaller queue depth
> > > despite a full page being allocated. This leads to inefficient memory
> > > usage and unnecessary hardware performance limitation.
> > > 
> > > Terminate the loop once qsz reaches PAGE_SIZE to ensure logical
> > > consistency and optimal hardware configuration.
> > 
> > That's really not an argument - even if an allocator does happen to
> > over-allocate for the requested size, that is hardly the caller's concern;
> > and as far as "optimal" queue sizes go in this case, those very much depend
> > on the number of CPUs issuing commands and volume of expected stall/PRI
> > events - in many cases PAGE_SIZE would already be far too small to really
> > work well.
> > 
> > Also note that if we _were_ to fail to allocate a PAGE_SIZE or smaller
> > queue, there would be very little chance of the subsequent allocation(s) for
> > the stream table succeeding, so realistically the driver is probably going
> > to end up failing to probe in such circumstances anyway.
> 
> That's all true, but tbf I think I just fscked up the comparison in
> d25f6ead162e ("iommu/arm-smmu-v3: Increase maximum size of queues") so
> I'm not against fixing that up even though the "rationale" given by
> Loser doesn't make a whole lot of sense.

Hi Will, Robin,

Thank you both for the detailed feedback.

Robin, you are absolutely correct. After a deeper look into the source code,
I see that allocations smaller than a page are indeed possible in certain
cases. My previous assumption about the DMA API's granularity was wrong,
and I appreciate the correction.

However, as Will noted that the current logic deviates from the original
intent, I have prepared a v2 to limit the queue allocation retry boundary
to PAGE_SIZE.

In v2, I have:
 - Updated my identity to Leo Jiang.
 - Removed the code comments as suggested.

I will send the v2 as a follow-up shortly.

Best regards,
Leo Jiang

[PATCH v2] iommu/arm-smmu-v3: Limit queue allocation retry boundary to PAGE_SIZE

[Leo Jiang]

In arm_smmu_init_one_queue(), the driver attempts to allocate the largest
supported queue and retries by halving the size on failure. Currently,
the retry logic allows the allocation to fall below PAGE_SIZE if the
PAGE_SIZE attempt fails.

While dmam_alloc_coherent() can theoretically return allocations smaller
than a page in certain configurations, it is preferable to limit the
retry boundary to PAGE_SIZE for SMMUv3 queues to align with the
original design intent.

This patch ensures the retry loop terminates when qsz reaches PAGE_SIZE,
per the feedback from the maintainer.

Signed-off-by: Leo Jiang <leo.jiang1224@foxmail.com>
---
v2:
- Use real name "Leo Jiang".
- Remove code comments as suggested by Will.
- Limit the queue allocation retry boundary to PAGE_SIZE

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index e8d7dbe495f0..4a0b15b1d4d2 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -4418,7 +4418,7 @@ int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
 		qsz = ((1 << q->llq.max_n_shift) * dwords) << 3;
 		q->base = dmam_alloc_coherent(smmu->dev, qsz, &q->base_dma,
 					      GFP_KERNEL);
-		if (q->base || qsz < PAGE_SIZE)
+		if (q->base || qsz <= PAGE_SIZE)
 			break;
 
 		q->llq.max_n_shift--;
-- 
2.34.1