On Wed, 2019-08-14 at 21:47 +0300, Leon Romanovsky wrote:
> On Wed, Aug 14, 2019 at 11:05:04AM -0400, Doug Ledford wrote:
> > On Wed, 2019-08-14 at 14:02 +0800, Yangyang Li wrote:
> > > > I don't know your hardware, but this patch sounds
> > > > wrong/dangerous to
> > > > me.
> > > > As long as the resources this card might access are allocated by
> > > > the
> > > > kernel, you can't get random data corruption by the card writing
> > > > to
> > > > memory used elsewhere in the kernel. So if your card is not
> > > > responding
> > > > to your requests to free the resources, it would seem safer to
> > > > leak
> > > > those resources permanently than to free them and risk the card
> > > > coming
> > > > back to life long enough to corrupt memory reallocated to some
> > > > other
> > > > task.
> > > >
> > > > Only if you can guarantee me that there is no way your commands
> > > > to
> > > > the
> > > > card will fail and then the card start working again later would
> > > > I
> > > > consider this patch safe. And if it's possible for the card to
> > > > hang
> > > > like this, should that be triggering a reset of the device?
> > > >
> > >
> > > Thanks for your suggestion, I agree with you, it would seem safer
> > > to
> > > leak
> > > those resources permanently than to free them. I will abandon this
> > > change
> > > and consider cleaning up these leaked resources during
> > > uninstallation
> > > or reset.
> >
> > Ok, patch dropped from patchworks, thanks.
>
> Sorry for being late, but I don't like the idea of having leaked
> memory.
>
> All my allocation patches are actually trying to avoid such situation
> by ensuring that no driver does crazy stuff like this. It means that
> once I'll have time to work on QP allocations, I'll ensure that memory
> is freed, so it is better to free such memory now.
You can't free something if the card might still access it. A leak is
always preferable to data corruption.
--
Doug Ledford <dledford@xxxxxxxxxx>
GPG KeyID: B826A3330E572FDD
Fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
Attachment:
signature.asc
Description: This is a digitally signed message part
