On Tue, May 5, 2020 at 5:15 PM Mattia Dongili <malattia@xxxxxxxx> wrote: > On Tue, May 05, 2020 at 04:26:18PM +0300, Andy Shevchenko wrote: > > On Tue, May 5, 2020 at 4:17 PM Mattia Dongili <malattia@xxxxxxxx> wrote: > > > On Tue, May 05, 2020 at 03:38:15PM +0300, Andy Shevchenko wrote: > > > > On Tue, May 5, 2020 at 1:16 PM <malattia@xxxxxxxx> wrote: > > > > > > > > > > From: Mattia Dongili <malattia@xxxxxxxx> > > > > > > > > > > The thermal handle object may fail initialization when the module is > > > > > loaded the first time. > > > > > > > > > > > > > But isn't it a papering over the real issue that it fails first time? > > > > AFAIU user needs to try first time and if it fails to try again. Can > > > > we rather understand the root cause first? > > > > > > I think this is a bug regardless of what ACPICA does. > > > > I didn't speak about ACPICA. > > Oh wait. Are you talking about my wording "when the module is loaded the > first time"? > Maybe I should have written "when the module is loaded in the first > place", I suppose that's clearer in English. > Initializing the thermal handle should fail consistently (unless > the system is low on memory and magically recovers just after failing > acquiring some memory in sony_nc_thermal_setup). Yes, I talk about "first time" phrase. > > > If the driver fails completing whatever initialization, should it not > > > avoid dereferencing a NULL pointer on resume? > > > > Yes, but *why* it fails? > > In the case that was reported in the linked bugzilla, ACPICA behaviour > changed and the driver wasn't quite ready to deal with it (which is I > assumed you were talking about ACPICA). > Instead of quietly performing the functions it could, sony-laptop caused > a NULL pointer deref on resume. sony_nc_thermal_setup failed at some > point and th_handle was then set to NULL on the way out of that > function. > > The sony-laptop driver is entirely reverse engineered from DSDT and > corresponding behaviours in Windows. There's probably a few reasons why > loading thermal profiles could fail, but the most common is probably > because of newer Vaio models using the same SNC function codes but > having a different interface (look at sony_nc_backlight_setup for an > example). > > Arguably, a better fix could be to fail loading the module if any of the > SNC functions fails initialization. The downside then is that those new > Vaio models that are only partially (in)compatible would remain without > *any* SNC function until some kind soul comes along and reverses > engineer the new behaviour. > > Another (possibly better) fix is to keep a list of which functions > successfully initialized and only attempt to resume those instead of > blindly going through all the known ones. > I'm not against this last one but it's quite more invasive. OK, can we go with the series out of two (besides another fix for ACPICA changed behaviour): - (temporary) fix for the failing initialization (so, can be backported) - real invasive fix as described by you ? -- With Best Regards, Andy Shevchenko
