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

[Marion & Christophe JAILLET]

Le 31/05/2024 à 07:36, Akhil R a écrit :
> > -----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.


Yes, that is my point.

> 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.
Yes, I spoke too fast.
What is freed is dev_iommu_fwspec_get(dev);, not dev. So the sentence I 
wrote makes no sense and should be removed :(


CJ