I do agree the description should probably be changed.  There shouldn't be
any panics involved, only a performance impact as it will be reallocating
always if it is on a node with no memory.

My intention on this was to make certain that the memory used is from the
closest node possible.  As such I believe this change likely honours that.

Thanks,

Alex


On Mon, Jul 21, 2014 at 10:42 AM, Nishanth Aravamudan <
nacc@linux.vnet.ibm.com> wrote:

> On 11.07.2014 [15:37:32 +0800], Jiang Liu wrote:
> > When CONFIG_HAVE_MEMORYLESS_NODES is enabled,
> cpu_to_node()/numa_node_id()
> > may return a node without memory, and later cause system failure/panic
> > when calling kmalloc_node() and friends with returned node id.
> > So use cpu_to_mem()/numa_mem_id() instead to get the nearest node with
> > memory for the/current cpu.
> >
> > If CONFIG_HAVE_MEMORYLESS_NODES is disabled, cpu_to_mem()/numa_mem_id()
> > is the same as cpu_to_node()/numa_node_id().
> >
> > Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> > ---
> >  drivers/net/ethernet/intel/igb/igb_main.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> b/drivers/net/ethernet/intel/igb/igb_main.c
> > index f145adbb55ac..2b74bffa5648 100644
> > --- a/drivers/net/ethernet/intel/igb/igb_main.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> > @@ -6518,7 +6518,7 @@ static bool igb_can_reuse_rx_page(struct
> igb_rx_buffer *rx_buffer,
> >                                 unsigned int truesize)
> >  {
> >       /* avoid re-using remote pages */
> > -     if (unlikely(page_to_nid(page) != numa_node_id()))
> > +     if (unlikely(page_to_nid(page) != numa_mem_id()))
> >               return false;
> >
> >  #if (PAGE_SIZE < 8192)
> > @@ -6588,7 +6588,7 @@ static bool igb_add_rx_frag(struct igb_ring
> *rx_ring,
> >               memcpy(__skb_put(skb, size), va, ALIGN(size,
> sizeof(long)));
> >
> >               /* we can reuse buffer as-is, just make sure it is local */
> > -             if (likely(page_to_nid(page) == numa_node_id()))
> > +             if (likely(page_to_nid(page) == numa_mem_id()))
> >                       return true;
> >
> >               /* this page cannot be reused so discard it */
>
> This doesn't seem to have anything to do with crashes or errors?
>
> The original code is checking if the NUMA node of a page is remote to
> the NUMA node current is running on. Your change makes it check if the
> NUMA node of a page is not equal to the nearest NUMA node with memory.
> That's not necessarily local, though, which seems like that is the whole
> point. In this case, perhaps the driver author doesn't want to reuse the
> memory at all for performance reasons? In any case, I don't think this
> patch has appropriate justification.
>
> Thanks,
> Nish
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>