iommu/arm-smmu-v3: Shrink command/event/PRI queues in kdump kernel
Subsystems (2)
PATCH 1/1 iommu/arm-smmu-v3: Shrink command/event/PRI queues in kdump kernel
The command, event and PRI queues are sized from the maxima the hardware advertises in IDR1, which can be several megabytes each. On systems with many SMMUv3 instances that cost is paid per instance and adds up to tens of megabytes of coherent DMA in the capture kernel.
A kdump capture kernel runs from a small crashkernel reservation and only has to drive the few devices used to save the dump, so deep queues serve no purpose. The queues carry invalidation commands and fault records, not DMA data, so dump throughput is unaffected; a shallower queue only bounds how many commands may be in flight before a sync, which does not matter for the capture kernel's small device count and modest I/O.
Clamp every queue to a single page when is_kdump_kernel() is true. Doing it in arm_smmu_init_one_queue() covers the command, event and PRI queues in one place. The command queue still holds at least one batch plus a sync (256 entries on a 4K-page kernel, well above CMDQ_BATCH_ENTRIES), so command batching keeps working.
--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +14 -0
@@ -4414,6 +4414,20 @@ int arm_smmu_init_one_queue(struct arm_smmu_device *smmu, { size_t qsz; + /* + * A kdump capture kernel runs from a small crashkernel reservation and + * only has to drive the few devices used to save the dump, so there is + * no point sizing the queues for the (multi-megabyte) maxima the + * hardware advertises. Clamp each queue to a single page. ent_sz_shift + * is the log2 of the entry size in bytes (dwords * 8). + */ + if (is_kdump_kernel()) { + u32 ent_sz_shift = ilog2(dwords) + 3; + + q->llq.max_n_shift = min_t(u32, q->llq.max_n_shift, + PAGE_SHIFT - ent_sz_shift); + } + do { qsz = ((1 << q->llq.max_n_shift) * dwords) << 3; q->base = dmam_alloc_coherent(smmu->dev, qsz, &q->base_dma,
Signature · 1 line
2.54.0
Quoted 18 lines · Kiryl Shutsemau (Meta)
The command, event and PRI queues are sized from the maxima the hardware advertises in IDR1, which can be several megabytes each. On systems with many SMMUv3 instances that cost is paid per instance and adds up to tens of megabytes of coherent DMA in the capture kernel.
A kdump capture kernel runs from a small crashkernel reservation and only has to drive the few devices used to save the dump, so deep queues serve no purpose. The queues carry invalidation commands and fault records, not DMA data, so dump throughput is unaffected; a shallower queue only bounds how many commands may be in flight before a sync, which does not matter for the capture kernel's small device count and modest I/O.
Clamp every queue to a single page when
is_kdump_kernel()is true. Doing it inarm_smmu_init_one_queue()covers the command, event and PRI queues in one place. The command queue still holds at least one batch plus a sync (256 entries on a 4K-page kernel, well aboveCMDQ_BATCH_ENTRIES), so command batching keeps working.
Quoted 5 lines
Suggested-by Kyle McMartin <[email protected]>Signed-off-by Kiryl Shutsemau (Meta) <[email protected]>--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
Make sense to me
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c int arm_smmu_init_one_queue(struct arm_smmu_device *smmu, · line 4428
Quoted 5 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +5
-0+if (is_kdump_kernel ()) { +u32 ent_sz_shift =ilog2 (dwords) +3 ; + + q->llq.max_n_shift =min_t (u32, q->llq.max_n_shift, +PAGE_SHIFT - ent_sz_shift);
I saw lately many people saying you should not use min_t, why is it needed here?
Jason
Breno Leitao
Reviewed-byQuoted 27 lines · Jason Gunthorpe
Quoted 18 lines · Kiryl Shutsemau (Meta)
The command, event and PRI queues are sized from the maxima the hardware advertises in IDR1, which can be several megabytes each. On systems with many SMMUv3 instances that cost is paid per instance and adds up to tens of megabytes of coherent DMA in the capture kernel.
A kdump capture kernel runs from a small crashkernel reservation and only has to drive the few devices used to save the dump, so deep queues serve no purpose. The queues carry invalidation commands and fault records, not DMA data, so dump throughput is unaffected; a shallower queue only bounds how many commands may be in flight before a sync, which does not matter for the capture kernel's small device count and modest I/O.
Clamp every queue to a single page when
is_kdump_kernel()is true. Doing it inarm_smmu_init_one_queue()covers the command, event and PRI queues in one place. The command queue still holds at least one batch plus a sync (256 entries on a 4K-page kernel, well aboveCMDQ_BATCH_ENTRIES), so command batching keeps working.Quoted 5 lines
Suggested-by Kyle McMartin <[email protected]>Signed-off-by Kiryl Shutsemau (Meta) <[email protected]>--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)Make sense to me
Quoted 8 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c int arm_smmu_init_one_queue(struct arm_smmu_device *smmu, · line 4428
Quoted 5 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +5
-0+if (is_kdump_kernel ()) { +u32 ent_sz_shift =ilog2 (dwords) +3 ; + + q->llq.max_n_shift =min_t (u32, q->llq.max_n_shift, +PAGE_SHIFT - ent_sz_shift);I saw lately many people saying you should not use
min_t, why is it needed here?
Good point, it seems that both of them are u32
- q->llq.max_n_shift is u32 (struct arm_smmu_ll_queue)
- ent_sz_shift is u32, and PAGE_SHIFT is a small int constant, so PAGE_SHIFT - ent_sz_shift promotes to u32 too.
min() should be enough, I would say. With that, please feel free to add: