[PATCH] ACPI: AGDI: Improve error reporting for problems during .remove()

[Uwe Kleine-König]

On Tue, Feb 14, 2023 at 05:36:23PM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> On Mon, Dec 19, 2022 at 11:18:19PM +0100, Uwe Kleine-König wrote:
> > On Wed, Oct 26, 2022 at 07:23:35PM +0200, Uwe Kleine-König wrote:
> > > On Wed, Oct 26, 2022 at 05:09:40PM +0100, James Morse wrote:
> > > > 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 at pengutronix.de>
> > > > >> ---
> > > > >> 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?
> > 
> > This patch wasn't applied anywhere (at least it didn't appear in next
> > since October). Did it fell through the cracks? Is there anything
> > missing?
> 
> gentle ping!
> 
> Working on making struct platform_driver::remove() return void, I'd like
> to base another patch on top of this one. For that it would be great if
> it entered the mainline ...

gentle ping ++

Would it help to resend?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20230412/25c672cc/attachment-0001.sig>