[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-acpi
Subject:    Re: [PATCH V2] x86: Fix an issue with invalid ACPI NUMA config
From:       Bjorn Helgaas <helgaas () kernel ! org>
Date:       2019-01-29 21:10:15
Message-ID: 20190129211015.GC91506 () google ! com
[Download RAW message or body]

On Tue, Jan 29, 2019 at 07:45:34PM +0000, Jonathan Cameron wrote:
> On Tue, 29 Jan 2019 13:05:56 -0600
> Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Tue, Jan 29, 2019 at 09:51:05AM +0000, Jonathan Cameron wrote:

> > > However, if there is NUMA description, but with bugs then we should
> > > protect in depth.  A simple example being that we declare 2 nodes, but
> > > then use _PXM for a third. I've done that by accident and blows up
> > > in a nasty fashion (not done it for a while, but probably still true).
> > > 
> > > Given DSDT is only parsed long after SRAT we can just check on _PXM
> > > queries.  Or I suppose we could do a verification parse for all _PXM
> > > entries and put out some warnings if they don't match SRAT entries?  
> > 
> > I'm assuming the crash happens when we call kmalloc_node() with a node
> > not mentioned in SRAT.  I think that's just sub-optimal implementation
> > in kmalloc_node().
> > 
> > We *could* fail the allocation and return a NULL pointer, but I think
> > even that is excessive.  I think we should simply fall back to
> > kmalloc().  We could print a one-time warning if that's useful.
> > 
> > If kmalloc_node() for an unknown node fell back to kmalloc(), would
> > anything else be required?
> 
> It will deal with that case, but it may not be the only one.  I
> think there are interrupt related issues as well, but will have to
> check.

Sounds like a valid concern.  Also, kmalloc() in general looks like a
performance path, so maybe it would be better to address this on the
other end, i.e., by ensuring that dev->numa_node always contains
something valid for kmalloc(), interrupts, etc.

Maybe set_dev_node() could be made smarter along that line?

Bjorn
[prev in list] [next in list] [prev in thread] [next in thread]