On 3/19/20 2:26 PM, Sachin Sant wrote:
>
>
>> On 19-Mar-2020, at 6:53 PM, Vlastimil Babka <vbabka@xxxxxxx> wrote:
>>
>> On 3/19/20 9:52 AM, Sachin Sant wrote:
>>>
>>>> OK how about this version? It's somewhat ugly, but important is that the fast
>>>> path case (c->page exists) is unaffected and another common case (c->page is
>>>> NULL, but node is NUMA_NO_NODE) is just one extra check - impossible to avoid at
>>>> some point anyway.
>>>>
>>>
>>> I attempted the suggested tests.
>>>
>>> Test 1: March 18 linux-next + Patch 1 [1] + Patch 2 [2]
>>>
>>> Machine boots fine. numactl o/p after boot:
>>
>> Great, thanks! Can I add your Tested-by: then?
>
> Sure.
> Tested-by: Sachin Sant <sachinp@xxxxxxxxxxxxxxxxxx>
>
> Thank you for the fix.
Thanks! Sorry to bother, but in the end I decided to do further change so I
would appreciate verification if it still works as intended.
The point is to use node_state(N_NORMAL_MEMORY) instead of node_present_pages(),
as that is really what SLUB uses to decide whether to allocate the
kmem_cache_node. So we should match this properly given the opportunity.
I have also again removed the node_online() check in alloc_slab_page() as it
really shouldn't be reachable with an offline node - everything is taken care of
in ___slab_alloc, or callers use NUMA_NO_NODE.
----8<----
diff --git a/mm/slub.c b/mm/slub.c
index 17dc00e33115..7113b1f9cd77 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1973,8 +1973,6 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
if (node == NUMA_NO_NODE)
searchnode = numa_mem_id();
- else if (!node_present_pages(node))
- searchnode = node_to_mem_node(node);
object = get_partial_node(s, get_node(s, searchnode), c, flags);
if (object || node != NUMA_NO_NODE)
@@ -2563,17 +2561,27 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
struct page *page;
page = c->page;
- if (!page)
+ if (!page) {
+ /*
+ * if the node is not online or has no normal memory, just
+ * ignore the node constraint
+ */
+ if (unlikely(node != NUMA_NO_NODE &&
+ !node_state(node, N_NORMAL_MEMORY)))
+ node = NUMA_NO_NODE;
goto new_slab;
+ }
redo:
if (unlikely(!node_match(page, node))) {
- int searchnode = node;
-
- if (node != NUMA_NO_NODE && !node_present_pages(node))
- searchnode = node_to_mem_node(node);
-
- if (unlikely(!node_match(page, searchnode))) {
+ /*
+ * same as above but node_match() being false already
+ * implies node != NUMA_NO_NODE
+ */
+ if (!node_state(node, N_NORMAL_MEMORY)) {
+ node = NUMA_NO_NODE;
+ goto redo;
+ } else {
stat(s, ALLOC_NODE_MISMATCH);
deactivate_slab(s, page, c->freelist, c);
goto new_slab;
--
2.25.1
