linux-kernel - Re: x86/mce: machine check warning during poweroff

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120116181135.GA2680@suse.de>
Date:	Mon, 16 Jan 2012 10:11:35 -0800
From:	Greg KH <gregkh@...e.de>
To:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Kay Sievers <kay.sievers@...y.org>
Cc:	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>,
	Ming Lei <tom.leiming@...il.com>,
	Djalal Harouni <tixxdz@...ndz.org>,
	Borislav Petkov <borislav.petkov@....com>,
	Tony Luck <tony.luck@...el.com>,
	Hidetoshi Seto <seto.hidetoshi@...fujitsu.com>,
	Ingo Molnar <mingo@...e.hu>, Andi Kleen <ak@...ux.intel.com>,
	linux-kernel@...r.kernel.org, Kay Sievers <kay.sievers@...y.org>,
	gouders@...bocholt.fh-gelsenkirchen.de,
	Marcos Souza <marcos.mage@...il.com>,
	Linux PM mailing list <linux-pm@...r.kernel.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	"tglx@...utronix.de" <tglx@...utronix.de>,
	prasad@...ux.vnet.ibm.com, justinmattock@...il.com,
	Jeff Chua <jeff.chua.linux@...il.com>,
	Suresh B Siddha <suresh.b.siddha@...el.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Mel Gorman <mgorman@...e.de>,
	Gilad Ben-Yossef <gilad@...yossef.com>
Subject: Re: x86/mce: machine check warning during poweroff

On Sat, Jan 14, 2012 at 06:49:38AM -0800, Greg KH wrote:
> On Fri, Jan 13, 2012 at 06:53:04PM -0800, Linus Torvalds wrote:
> > On Fri, Jan 13, 2012 at 6:41 PM, Srivatsa S. Bhat
> > <srivatsa.bhat@...ux.vnet.ibm.com> wrote:
> > >
> > > YES!! Finally I have a fix for this whole MCE thing! :-)
> > 
> > Goodie.
> > 
> > > The patch below works perfectly for me - I tested multiple CPU hotplug
> > > operations as well as multiple pm_test runs at core level. Please let me
> > > know if this solves the suspend issue as well..
> > 
> > Ok, I'll try, and I bet it does.
> > 
> > HOWEVER.
> > 
> > I'd be a whole lot happier knowing exactly which field in "struct
> > device" that needed to be NULL before it gets registered.
> > 
> > I don't like how
> > 
> >   device_register() + device_create_file(dev)..
> > 
> > is not sufficiently undone by
> > 
> >  .. device_remove_file(dev) +  device_unregister()
> > 
> > so that it can't be repeated. Exactly *what* state is stale and
> > re-used incorrectly if you do that device_register() a second time.
> > 
> > It smells like a misfeature of the device core handling.
> 
> It has to do with the fact that this is a "static" device that is being
> reused.  Normally it would be cleaned up properly in the release
> function, but as there isn't one, some fields are being left in a bad
> state.

Kay, I looked at this this morning, and it comes down to the line:

DEFINE_PER_CPU(struct device, mce_device);

Where we are creating static struct device variables.  I'm guessing this
is just done for "convenience" as we really don't care about where in
memory these structures are, we just want to make sure we have enough of
them around (this is the way all the other mce per-cpu structures are
handled.)

I couldn't figure out a "simple" way to create a variable per cpu here,
dynamically.  I tried doing something like:
	struct device *mce_device[CONFIG_NR_CPUS];
and dynamically create and clean them up when they go away, setting the
array value to NULL when they are unregistered, and let them clean up in
the release function, but does that race with creating the device again?

It seems that this would work, but I'm probably missing something
obvious here, any ideas?

The "correct" way to fix this up would be to have a per-cpu structure
for all of the different mce things that are created in this driver
(struct device, struct mce, exception counts, work queues, polling
banks, etc.), but that seems pretty messy, and I imagine some of these
want to stay as-is for some performance issues.  As I don't know this
code at all, I'm a bit leary to make that kind of change.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/