Re: [Intel-wired-lan] [Patch V3 5/9] i40e: Use numa_mem_id() to better support memoryless node

On 2015/10/9 17:08, Kamezawa Hiroyuki wrote:

On 2015/10/09 14:52, Jiang Liu wrote:

quoted

On 2015/10/9 4:20, Andrew Morton wrote:

quoted

On Wed, 19 Aug 2015 17:18:15 -0700 (PDT) David Rientjes
[off-list ref] wrote:

quoted

On Wed, 19 Aug 2015, Patil, Kiran wrote:

quoted

Acked-by: Kiran Patil <redacted>

Where's the call to preempt_disable() to prevent kernels with
preemption
from making numa_node_id() invalid during this iteration?

David asked this question twice, received no answer and now the patch
is in the maintainer tree, destined for mainline.

If I was asked this question I would respond

   The use of numa_mem_id() is racy and best-effort.  If the unlikely
   race occurs, the memory allocation will occur on the wrong node, the
   overall result being very slightly suboptimal performance.  The
   existing use of numa_node_id() suffers from the same issue.

But I'm not the person proposing the patch.  Please don't just ignore
reviewer comments!

Hi Andrew,
    Apologize for the slow response due to personal reasons!
And thanks for answering the question from David. To be honest,
I didn't know how to answer this question before. Actually this
question has puzzled me for a long time when dealing with memory
hot-removal. For normal cases, it only causes sub-optimal memory
allocation if schedule event happens between querying NUMA node id
and calling alloc_pages_node(). But what happens if system run into
following execution sequence?
1) node = numa_mem_id();
2) memory hot-removal event triggers
2.1) remove affected memory
2.2) reset pgdat to zero if node becomes empty after memory removal

I'm sorry if I misunderstand something.
After commit b0dc3a342af36f95a68fe229b8f0f73552c5ca08, there is no
memset().

Hi Kamezawa,
	Thanks for the information. The commit solved the issue what
I was puzzling about. With this change applied, thing should work
as expected. Seems it would be better to enhance __build_all_zonelists()
to handle those offlined empty nodes too, but that really doesn't
make to much difference:)
	Thanks for the info again!
Thanks!
Gerry

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>