RE: [PATCH] crypto: tegra - Remove an incorrect iommu_fwspec_free() call in tegra_se_remove()

From: Akhil R
Date: Fri May 31 2024 - 01:37:12 EST




> -----Original Message-----
> From: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
> Sent: Friday, May 31, 2024 10:53 AM
> To: Akhil R <akhilrajeev@xxxxxxxxxx>
> Cc: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>; David S. Miller
> <davem@xxxxxxxxxxxxx>; Thierry Reding <thierry.reding@xxxxxxxxx>; Jon
> Hunter <jonathanh@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; kernel-
> janitors@xxxxxxxxxxxxxxx; linux-crypto@xxxxxxxxxxxxxxx; linux-
> tegra@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] crypto: tegra - Remove an incorrect iommu_fwspec_free()
> call in tegra_se_remove()
>
> External email: Use caution opening links or attachments
>
>
> On Wed, May 29, 2024 at 06:53:42AM +0000, Akhil R wrote:
> > > The only iommu function call in this driver is a
> > > tegra_dev_iommu_get_stream_id() which does not allocate anything and
> does
> > > not take any reference.
> > >
> > > More-over, what is freed is "se->dev" which has been devm_kzalloc()'ed in the
> > > probe.
> >
> > I did not completely understand what is being tried to convey here.
> > If I understand it right, iommu_fwspec_free() does not do anything
> > with the "devm_kzalloc"ed variable.
> >
> > It would probably be a good idea to remove this line from the commit message.
>
> I think he means that as the memory was allocated via devm, it will
> be automatically freed by the kernel and the driver does not need
> to (and should not) free the memory by hand.

Ya. But iommu_fwspec_free() does not free the memory allocated via devm.

I think iommu_fwspec_free() is expected to be called in symmetry with
iommu_fwspec_init(). So, I do agree that the SE driver does not allocate
what is freed by iommu_fwspec_free(), but I feel this line is a bit misleading.