Re: [PATCH rc v2] iommu/tegra241-cmdqv: Fix warnings due to dmam_free_coherent()

[Jason Gunthorpe <jgg@nvidia.com>]

On Mon, Apr 07, 2025 at 01:19:08PM -0700, Nicolin Chen wrote:
> Two WARNINGs are observed when SMMU driver rolls back upon failure:
>  arm-smmu-v3.9.auto: Failed to register iommu
>  arm-smmu-v3.9.auto: probe with driver arm-smmu-v3 failed with error -22
>  ------------[ cut here ]------------
>  WARNING: CPU: 5 PID: 1 at kernel/dma/mapping.c:74 dmam_free_coherent+0xc0/0xd8
>  Call trace:
>   dmam_free_coherent+0xc0/0xd8 (P)
>   tegra241_vintf_free_lvcmdq+0x74/0x188
>   tegra241_cmdqv_remove_vintf+0x60/0x148
>   tegra241_cmdqv_remove+0x48/0xc8
>   arm_smmu_impl_remove+0x28/0x60
>   devm_action_release+0x1c/0x40
>  ------------[ cut here ]------------
>  128 pages are still in use!
>  WARNING: CPU: 16 PID: 1 at mm/page_alloc.c:6902 free_contig_range+0x18c/0x1c8
>  Call trace:
>   free_contig_range+0x18c/0x1c8 (P)
>   cma_release+0x154/0x2f0
>   dma_free_contiguous+0x38/0xa0
>   dma_direct_free+0x10c/0x248
>   dma_free_attrs+0x100/0x290
>   dmam_free_coherent+0x78/0xd8
>   tegra241_vintf_free_lvcmdq+0x74/0x160
>   tegra241_cmdqv_remove+0x98/0x198
>   arm_smmu_impl_remove+0x28/0x60
>   devm_action_release+0x1c/0x40
> 
> This is because the LVCMDQ queue memory are managed by devres, while that
> dmam_free_coherent() is called in the context of devm_action_release().

> Jason pointed out that "arm_smmu_impl_probe() has mis-ordered the devres
> callbacks if ops->device_remove() is going to be manually freeing things
> that probe allocated":
> https://lore.kernel.org/linux-iommu/20250407174408.GB1722458@nvidia.com/
> 
> In fact, tegra241_cmdqv_init_structures() only allocates memory resources
> which means any failure that it generates would be similar to -ENOMEM, so
> there is no point in having that "falling back to standard SMMU" routine,
> as the standard SMMU would likely fail to allocate memory too.
> 
> Remove the unwind part in tegra241_cmdqv_init_structures(), and return a
> proper error code to ask SMMU driver to call tegra241_cmdqv_remove() via
> impl_ops->device_remove(). Then, drop tegra241_vintf_free_lvcmdq() since
> devres will take care of that.
> 
> Fixes: 483e0bd8883a ("iommu/tegra241-cmdqv: Do not allocate vcmdq until dma_set_mask_and_coherent")
> Cc: stable@vger.kernel.org
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
> Changelog
> v2
>  * Fail tegra241_cmdqv_init_structures() and let devres take care of the
>    lvcmdq queue memory space
> v1
>  https://lore.kernel.org/all/cover.1744014481.git.nicolinc@nvidia.com/
> 
>  .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c    | 32 +++----------------
>  1 file changed, 5 insertions(+), 27 deletions(-)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason