Re: [XEN][PATCH v9 09/19] xen/iommu: Move spin_lock from iommu_dt_device_is_assigned to caller

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.08.2023 02:28, Vikram Garhwal wrote:
> Rename iommu_dt_device_is_assigned() to iommu_dt_device_is_assigned_locked().
> Remove static type so this can also be used by SMMU drivers to check if the
> device is being used before removing.
> 
> Moving spin_lock to caller was done to prevent the concurrent access to
> iommu_dt_device_is_assigned while doing add/remove/assign/deassign. Follow-up
> patches in this series introduces node add/remove feature.
> 
> Also, caller is required hold the correct lock so moved the function prototype
> to a private header.
> 
> Signed-off-by: Vikram Garhwal <vikram.garhwal@xxxxxxx>
> 
> ---
> Changes from v7:
>     Update commit message.
>     Add ASSERT().
> ---
> ---
>  xen/drivers/passthrough/device_tree.c | 26 +++++++++++++++++++++----
>  xen/include/xen/iommu-private.h       | 28 +++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+), 4 deletions(-)
>  create mode 100644 xen/include/xen/iommu-private.h

I find it odd for new versions to be sent when discussion on the prior
version hasn't settled yet, and when you then also don't incorporate
the feedback given. I also find it odd to now be Cc-ed on the entire
series. Imo instead of that patches which aren't self-explanatory /
self-consistent simply need their descriptions improved (in the case
here to mention what further use of a function is intended). Yet
better would be to add declarations (and remove static) only at the
point that's actually needed. This then eliminates the risk of
subsequent changes in response to feedback (like Julien's here)
resulting in the declaration no longer being needed, thus leading to
a Misra violation (besides the general tidiness aspect).

Jan