Re: [PATCH RFC v2 1/4] iommu/arm-smmu-v3: Convert streams from RB tree to XArray

[Peng Fan <peng.fan@oss.nxp.com>]

Hi Nicolin,

Thanks for reviewing.

On Mon, Sep 21, 2026 at 12:24:13PM -0700, Nicolin Chen wrote:
>On Mon, Sep 21, 2026 at 07:35:00PM +0800, Peng Fan (OSS) wrote:
>> From: Peng Fan <peng.fan@nxp.com>
>> 
>> Replace the smmu->streams RB tree with an XArray for SID ->
>> arm_smmu_stream lookups. The existing streams_mutex serialises all
>> accesses (both xa_store/xa_erase and xa_load), protecting the lifetime
>> of returned pointers against concurrent arm_smmu_remove_master()
>> without requiring RCU grace periods. A mutex (rather than xa_lock) is
>> needed because several paths sleep while the lock is held:
>> dmam_alloc_coherent(GFP_KERNEL) in arm_smmu_init_sid_strtab(), and
>> down_read() inside iommu_report_device_fault().
>
>The commit message reads very unconvincing.
>
>The first paragraph starts with an action "replace" without giving
>a clear reason for the action.
>
>> This removes the RB tree comparators, the rb_node from
>
>Then the second paragraph repeats the action...
>
>> arm_smmu_stream, and simplifies duplicate-SID handling for bridged PCI
>> devices.
>
>... and slightly mentions "simplifies". Maybe it should write about
>why this is necessary and how xarray would simplify vs RB tree?

Jean had a comment that "maybe simplify the driver first by moving to a xarray",
see [1] [2].

[1] https://lore.kernel.org/linux-iommu/20230518130459.GA2587493@myrica/
[2] https://lore.kernel.org/linux-iommu/ecb3725c-27c4-944b-b42c-f4e293521f94@arm.com/#t


Does below commit message looks good to you?

iommu/arm-smmu-v3: Convert streams from RB tree to XArray

As suggested by Jean-Philippe Brucker [1], convert the smmu->streams
RB tree to an XArray as a preparatory simplification for shared-SID
support.

A subsequent patch needs to look up an existing stream by SID and
then decide to share it rather than reject it - two separate
operations. The RB tree's rb_find_add() fuses lookup and insertion
atomically: it either inserts or returns the collision, making
"find, then conditionally share" impossible without restructuring.
XArray's xa_load() and xa_store() are independent operations that
naturally support this pattern.

Eliminates the two comparator functions (arm_smmu_streams_cmp_key/node),
the per-stream rb_node field.

The existing streams_mutex continues to serialise all accesses.  A
mutex (rather than xa_lock) is needed because several paths sleep
while the lock is held: dmam_alloc_coherent(GFP_KERNEL) in
arm_smmu_init_sid_strtab(), and down_read() inside
iommu_report_device_fault().

No behavioural change.

[1] https://lore.kernel.org/linux-iommu/20230518130459.GA2587493@myrica/#t


Thanks,
Peng

>
>Nicolin
>