panics due to buggy ACPI in Dell Latitude E6530?

[David Demelier]

Le jeudi 4 avril 2013 18:45:58 Andriy Gapon a écrit :
> on 31/03/2013 14:42 kron said the following:
> > I'm sorry I forgot to update the thread - good you're reminding.
> > Andriy did a brilliant job at debugging the issue and I owe him
> > to say in public: Thank you, Andriy!
> 
> I also apologize for not sharing the information promptly.
> 
> So here is a summary.
> 
> The problem turned out to be with the reference count in ACPICA.  It doesn't
> have any internal locking and so it relied on locks obtained by the
> callers.  But not all of the callers obtained the relevant locks
> (namespace, interpreter) and they not really needed them (except for the
> reference counting).  Also, our acpi_battery driver uses ACPICA interfaces
> that were problematic.  Additionally the driver allows parallel queries,
> not sure if that is an intentional choice.
> 
> So now the ACPICA developers have fixed the reference counting code and no
> changes in FreeBSD code should be required.  We are just waiting for the
> next ACPICA release. That's for head.  Not sure which approach we will take
> for stable branches, because we haven't been doing any MFCs of ACPICA
> imports.  So there are tow choices: - use the below patch to prevent
> parallel execution in the batter driver - manually apply the specific
> reference count change to ACPICA code in the branches
> 
> Finally many thanks to Olli/kron for doing a lot of testing and debugging.
> And many thanks to Tom Lislegaard who did a lot of testing and debugging too
> - in our debugging session I reached all the same conclusions and came up
> with a (different) patch, but then got distracted and completely forgot
> about the issue until it resurfaced again.
> 

Thanks a lot for the very detailed explanation. For now I'll use the patch 
until the acpica release is merge into the next FreeBSD release :-).

Regards,

> > The results are:
> >  - hw.acpi.osname="Linux" is not relevant
> >  - there's some ACPICA issue Andriy took to discuss with other
> >  
> >    hackers (and much above my competence to comment)
> >  
> >  - a temporary workaround:
> > --- sys/dev/acpica/acpi_battery.c       (revision 248682)
> > +++ sys/dev/acpica/acpi_battery.c       (working copy)
> > @@ -360,6 +360,8 @@
> > 
> >      int error, unit;
> >      device_t dev;
> > 
> > +    mtx_lock(&Giant);
> > +
> > 
> >      /* For commands that use the ioctl_arg struct, validate it first. */
> >      error = ENXIO;
> >      unit = 0;
> > 
> > @@ -417,6 +419,7 @@
> > 
> >         error = EINVAL;
> >      
> >      }
> > 
> > +    mtx_unlock(&Giant);
> > 
> >      return (error);
> >  
> >  }
> > 
> > The patch works for me without any problem. I guess it won't hurt
> > your system ;-) but I actually don't know if/how it relates to your
> > PR.
-- 
David Demelier