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). > > 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. -- mattia :wq!
