Date: Thu, 04 Apr 2013 18:45:58 +0300 From: Andriy Gapon <avg@FreeBSD.org> To: kron <kron24@gmail.com>, David Demelier <demelier.david@gmail.com>, freebsd-acpi@FreeBSD.org Subject: Re: panics due to buggy ACPI in Dell Latitude E6530? Message-ID: <515DA036.5070206@FreeBSD.org> In-Reply-To: <51582122.3050703@gmail.com> References: <512E24CD.9090404@gmail.com> <512E397D.1070406@FreeBSD.org> <2041632.on0aZtOfZI@melon> <227443103.MjG0Okhn3r@melon> <51582122.3050703@gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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. > 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. -- Andriy Gapon
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?515DA036.5070206>