Hello James, On Wed, Oct 26, 2022 at 05:09:40PM +0100, James Morse wrote: > Hi guys, > > On 18/10/2022 10:35, Lorenzo Pieralisi wrote: > > On Fri, Oct 14, 2022 at 06:06:23PM +0200, Uwe Kleine-K�nig wrote: > >> Returning an error value in a platform driver's remove callback results in > >> a generic error message being emitted by the driver core, but otherwise it > >> doesn't make a difference. The device goes away anyhow. > >> > >> So instead of triggering the generic platform error message, emit a more > >> helpful message if a problem occurs and return 0 to suppress the generic > >> message. > >> > >> This patch is a preparation for making platform remove callbacks return > >> void. > > > > If that's the plan - I don't have anything against this patch. > > > >> Signed-off-by: Uwe Kleine-K�nig <u.kleine-koenig@xxxxxxxxxxxxxx> > >> --- > >> Hello, > >> > >> note that in the situations where the driver returned an error before > >> and now emits a message, there is a resource leak. Someone who knows > >> more about this driver and maybe even can test stuff, might want to > >> address this. This might not only be about non-freed memory, the device > >> disappears but it is kept in sdei_list and so might be used after being > >> gone. > > > I'd need James' input on this. I guess we may ignore > > sdei_event_disable() return value and continue anyway in agdi_remove(), > > whether that's the right thing to do it is a different question. > > The unregister stuff is allowed to fail if the event is 'in progress' on another CPU. > Given the handler panic()s the machine, if an event is in progress, the resource leak > isn't something worth worrying about. The real problem is that the handler code may be > free()d while another CPU is still executing it, which is only a problem for modules. > > As this thing can't be built as a module, and the handler panic()s the machine, I don't > think there is going to be a problem here. Is that an Ack? Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature