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 <
[email protected]> 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 <[email protected]>
> > ---
> > 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
------------------------------------------------------------------------------
Want fast and easy access to all the code in your enterprise? Index and
search up to 200,000 lines of code with a free copy of Black Duck
Code Sight - the same software that powers the world's largest code
search on Ohloh, the Black Duck Open Hub! Try it now.
http://p.sf.net/sfu/bds
_______________________________________________
E1000-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit
http://communities.intel.com/community/wired
