diff for duplicates of <20130118044242.GA18665@lge.com> diff --git a/a/1.txt b/N1/1.txt index 6702aed..941408b 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -19,182 +19,3 @@ I'm not test this patch, yet. Just for sharing my idea to fix a problem. -----------------8<----------------------- ->From aaf529e347b7deb8c35dd484abc9166a5d3c0629 Mon Sep 17 00:00:00 2001 -From: Joonsoo Kim <iamjoonsoo.kim@lge.com> -Date: Fri, 18 Jan 2013 13:10:33 +0900 -Subject: [RFC PATCH] slub: change an approach of checking node id in - slab_alloc_node() - -Below description borrows from Steven's "[RFC]slub: Keep page and object -in sync in slab_alloc_node()" - -In slab_alloc_node(), after the cpu_slab is assigned, if the task is -preempted and moves to another CPU, there's nothing keeping the page and -object in sync. The -rt kernel crashed because page was NULL and object -was not, and the node_match() dereferences page. Even though the crash -happened on -rt, there's nothing that's keeping this from happening on -mainline. - -To fix this, I add new variable, node, to struct kmem_cache_cpu. -With it, we can safely compare current cpu slab's node with node of -allocation request without considering both preemption and irq -in fast-path of allocation. - -This variable is set in slow-path, so this doesn't make much -performance degration. - -Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com> - -diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h -index 9db4825..b54dffa 100644 ---- a/include/linux/slub_def.h -+++ b/include/linux/slub_def.h -@@ -46,6 +46,9 @@ enum stat_item { - struct kmem_cache_cpu { - void **freelist; /* Pointer to next available object */ - unsigned long tid; /* Globally unique transaction id */ -+#ifdef CONFIG_NUMA -+ int node; -+#endif - struct page *page; /* The slab from which we are allocating */ - struct page *partial; /* Partially allocated frozen slabs */ - #ifdef CONFIG_SLUB_STATS -diff --git a/mm/slub.c b/mm/slub.c -index ba2ca53..d4d3d07 100644 ---- a/mm/slub.c -+++ b/mm/slub.c -@@ -272,6 +272,17 @@ static inline void set_freepointer(struct kmem_cache *s, void *object, void *fp) - *(void **)(object + s->offset) = fp; - } - -+static inline void set_cpuslab(struct kmem_cache_cpu *c, struct page *page) -+{ -+ c->page = page; -+#ifdef CONFIG_NUMA -+ if (page) -+ c->node = page_to_nid(page); -+ else -+ c->node = NUMA_NO_NODE; -+#endif -+} -+ - /* Loop over all objects in a slab */ - #define for_each_object(__p, __s, __addr, __objects) \ - for (__p = (__addr); __p < (__addr) + (__objects) * (__s)->size;\ -@@ -1562,7 +1573,7 @@ static void *get_partial_node(struct kmem_cache *s, struct kmem_cache_node *n, - break; - - if (!object) { -- c->page = page; -+ set_cpuslab(c, page); - stat(s, ALLOC_FROM_PARTIAL); - object = t; - available = page->objects - page->inuse; -@@ -1993,7 +2004,7 @@ static inline void flush_slab(struct kmem_cache *s, struct kmem_cache_cpu *c) - deactivate_slab(s, c->page, c->freelist); - - c->tid = next_tid(c->tid); -- c->page = NULL; -+ set_cpuslab(c, NULL); - c->freelist = NULL; - } - -@@ -2038,10 +2049,10 @@ static void flush_all(struct kmem_cache *s) - * Check if the objects in a per cpu structure fit numa - * locality expectations. - */ --static inline int node_match(struct page *page, int node) -+static inline int node_match(struct kmem_cache_cpu *c, int node) - { - #ifdef CONFIG_NUMA -- if (node != NUMA_NO_NODE && page_to_nid(page) != node) -+ if (node != NUMA_NO_NODE && c->node != node) - return 0; - #endif - return 1; -@@ -2136,7 +2147,7 @@ static inline void *new_slab_objects(struct kmem_cache *s, gfp_t flags, - page->freelist = NULL; - - stat(s, ALLOC_SLAB); -- c->page = page; -+ set_cpuslab(c, page); - *pc = c; - } else - freelist = NULL; -@@ -2224,10 +2235,10 @@ static void *__slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node, - goto new_slab; - redo: - -- if (unlikely(!node_match(page, node))) { -+ if (unlikely(!node_match(c, node))) { - stat(s, ALLOC_NODE_MISMATCH); - deactivate_slab(s, page, c->freelist); -- c->page = NULL; -+ set_cpuslab(c, NULL); - c->freelist = NULL; - goto new_slab; - } -@@ -2239,7 +2250,7 @@ redo: - */ - if (unlikely(!pfmemalloc_match(page, gfpflags))) { - deactivate_slab(s, page, c->freelist); -- c->page = NULL; -+ set_cpuslab(c, NULL); - c->freelist = NULL; - goto new_slab; - } -@@ -2254,7 +2265,7 @@ redo: - freelist = get_freelist(s, page); - - if (!freelist) { -- c->page = NULL; -+ set_cpuslab(c, NULL); - stat(s, DEACTIVATE_BYPASS); - goto new_slab; - } -@@ -2276,8 +2287,8 @@ load_freelist: - new_slab: - - if (c->partial) { -- page = c->page = c->partial; -- c->partial = page->next; -+ set_cpuslab(c, c->partial); -+ c->partial = c->partial->next; - stat(s, CPU_PARTIAL_ALLOC); - c->freelist = NULL; - goto redo; -@@ -2302,7 +2313,7 @@ new_slab: - goto new_slab; /* Slab failed checks. Next slab needed */ - - deactivate_slab(s, page, get_freepointer(s, freelist)); -- c->page = NULL; -+ set_cpuslab(c, NULL); - c->freelist = NULL; - local_irq_restore(flags); - return freelist; -@@ -2323,7 +2334,6 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s, - { - void **object; - struct kmem_cache_cpu *c; -- struct page *page; - unsigned long tid; - - if (slab_pre_alloc_hook(s, gfpflags)) -@@ -2350,8 +2360,7 @@ redo: - barrier(); - - object = c->freelist; -- page = c->page; -- if (unlikely(!object || !node_match(page, node))) -+ if (unlikely(!object || !node_match(c, node))) - object = __slab_alloc(s, gfpflags, node, addr, c); - - else { --- -1.7.9.5 - --- -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> diff --git a/a/content_digest b/N1/content_digest index 547ffd2..4a1ff48 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -45,185 +45,6 @@ "I'm not test this patch, yet.\n" "Just for sharing my idea to fix a problem.\n" "\n" - "-----------------8<-----------------------\n" - ">From aaf529e347b7deb8c35dd484abc9166a5d3c0629 Mon Sep 17 00:00:00 2001\n" - "From: Joonsoo Kim <iamjoonsoo.kim@lge.com>\n" - "Date: Fri, 18 Jan 2013 13:10:33 +0900\n" - "Subject: [RFC PATCH] slub: change an approach of checking node id in\n" - " slab_alloc_node()\n" - "\n" - "Below description borrows from Steven's \"[RFC]slub: Keep page and object\n" - "in sync in slab_alloc_node()\"\n" - "\n" - "In slab_alloc_node(), after the cpu_slab is assigned, if the task is\n" - "preempted and moves to another CPU, there's nothing keeping the page and\n" - "object in sync. The -rt kernel crashed because page was NULL and object\n" - "was not, and the node_match() dereferences page. Even though the crash\n" - "happened on -rt, there's nothing that's keeping this from happening on\n" - "mainline.\n" - "\n" - "To fix this, I add new variable, node, to struct kmem_cache_cpu.\n" - "With it, we can safely compare current cpu slab's node with node of\n" - "allocation request without considering both preemption and irq\n" - "in fast-path of allocation.\n" - "\n" - "This variable is set in slow-path, so this doesn't make much\n" - "performance degration.\n" - "\n" - "Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>\n" - "\n" - "diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h\n" - "index 9db4825..b54dffa 100644\n" - "--- a/include/linux/slub_def.h\n" - "+++ b/include/linux/slub_def.h\n" - "@@ -46,6 +46,9 @@ enum stat_item {\n" - " struct kmem_cache_cpu {\n" - " \tvoid **freelist;\t/* Pointer to next available object */\n" - " \tunsigned long tid;\t/* Globally unique transaction id */\n" - "+#ifdef CONFIG_NUMA\n" - "+\tint node;\n" - "+#endif\n" - " \tstruct page *page;\t/* The slab from which we are allocating */\n" - " \tstruct page *partial;\t/* Partially allocated frozen slabs */\n" - " #ifdef CONFIG_SLUB_STATS\n" - "diff --git a/mm/slub.c b/mm/slub.c\n" - "index ba2ca53..d4d3d07 100644\n" - "--- a/mm/slub.c\n" - "+++ b/mm/slub.c\n" - "@@ -272,6 +272,17 @@ static inline void set_freepointer(struct kmem_cache *s, void *object, void *fp)\n" - " \t*(void **)(object + s->offset) = fp;\n" - " }\n" - " \n" - "+static inline void set_cpuslab(struct kmem_cache_cpu *c, struct page *page)\n" - "+{\n" - "+\tc->page = page;\n" - "+#ifdef CONFIG_NUMA\n" - "+\tif (page)\n" - "+\t\tc->node = page_to_nid(page);\n" - "+\telse\n" - "+\t\tc->node = NUMA_NO_NODE;\n" - "+#endif\n" - "+}\n" - "+\n" - " /* Loop over all objects in a slab */\n" - " #define for_each_object(__p, __s, __addr, __objects) \\\n" - " \tfor (__p = (__addr); __p < (__addr) + (__objects) * (__s)->size;\\\n" - "@@ -1562,7 +1573,7 @@ static void *get_partial_node(struct kmem_cache *s, struct kmem_cache_node *n,\n" - " \t\t\tbreak;\n" - " \n" - " \t\tif (!object) {\n" - "-\t\t\tc->page = page;\n" - "+\t\t\tset_cpuslab(c, page);\n" - " \t\t\tstat(s, ALLOC_FROM_PARTIAL);\n" - " \t\t\tobject = t;\n" - " \t\t\tavailable = page->objects - page->inuse;\n" - "@@ -1993,7 +2004,7 @@ static inline void flush_slab(struct kmem_cache *s, struct kmem_cache_cpu *c)\n" - " \tdeactivate_slab(s, c->page, c->freelist);\n" - " \n" - " \tc->tid = next_tid(c->tid);\n" - "-\tc->page = NULL;\n" - "+\tset_cpuslab(c, NULL);\n" - " \tc->freelist = NULL;\n" - " }\n" - " \n" - "@@ -2038,10 +2049,10 @@ static void flush_all(struct kmem_cache *s)\n" - " * Check if the objects in a per cpu structure fit numa\n" - " * locality expectations.\n" - " */\n" - "-static inline int node_match(struct page *page, int node)\n" - "+static inline int node_match(struct kmem_cache_cpu *c, int node)\n" - " {\n" - " #ifdef CONFIG_NUMA\n" - "-\tif (node != NUMA_NO_NODE && page_to_nid(page) != node)\n" - "+\tif (node != NUMA_NO_NODE && c->node != node)\n" - " \t\treturn 0;\n" - " #endif\n" - " \treturn 1;\n" - "@@ -2136,7 +2147,7 @@ static inline void *new_slab_objects(struct kmem_cache *s, gfp_t flags,\n" - " \t\tpage->freelist = NULL;\n" - " \n" - " \t\tstat(s, ALLOC_SLAB);\n" - "-\t\tc->page = page;\n" - "+\t\tset_cpuslab(c, page);\n" - " \t\t*pc = c;\n" - " \t} else\n" - " \t\tfreelist = NULL;\n" - "@@ -2224,10 +2235,10 @@ static void *__slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,\n" - " \t\tgoto new_slab;\n" - " redo:\n" - " \n" - "-\tif (unlikely(!node_match(page, node))) {\n" - "+\tif (unlikely(!node_match(c, node))) {\n" - " \t\tstat(s, ALLOC_NODE_MISMATCH);\n" - " \t\tdeactivate_slab(s, page, c->freelist);\n" - "-\t\tc->page = NULL;\n" - "+\t\tset_cpuslab(c, NULL);\n" - " \t\tc->freelist = NULL;\n" - " \t\tgoto new_slab;\n" - " \t}\n" - "@@ -2239,7 +2250,7 @@ redo:\n" - " \t */\n" - " \tif (unlikely(!pfmemalloc_match(page, gfpflags))) {\n" - " \t\tdeactivate_slab(s, page, c->freelist);\n" - "-\t\tc->page = NULL;\n" - "+\t\tset_cpuslab(c, NULL);\n" - " \t\tc->freelist = NULL;\n" - " \t\tgoto new_slab;\n" - " \t}\n" - "@@ -2254,7 +2265,7 @@ redo:\n" - " \tfreelist = get_freelist(s, page);\n" - " \n" - " \tif (!freelist) {\n" - "-\t\tc->page = NULL;\n" - "+\t\tset_cpuslab(c, NULL);\n" - " \t\tstat(s, DEACTIVATE_BYPASS);\n" - " \t\tgoto new_slab;\n" - " \t}\n" - "@@ -2276,8 +2287,8 @@ load_freelist:\n" - " new_slab:\n" - " \n" - " \tif (c->partial) {\n" - "-\t\tpage = c->page = c->partial;\n" - "-\t\tc->partial = page->next;\n" - "+\t\tset_cpuslab(c, c->partial);\n" - "+\t\tc->partial = c->partial->next;\n" - " \t\tstat(s, CPU_PARTIAL_ALLOC);\n" - " \t\tc->freelist = NULL;\n" - " \t\tgoto redo;\n" - "@@ -2302,7 +2313,7 @@ new_slab:\n" - " \t\tgoto new_slab;\t/* Slab failed checks. Next slab needed */\n" - " \n" - " \tdeactivate_slab(s, page, get_freepointer(s, freelist));\n" - "-\tc->page = NULL;\n" - "+\tset_cpuslab(c, NULL);\n" - " \tc->freelist = NULL;\n" - " \tlocal_irq_restore(flags);\n" - " \treturn freelist;\n" - "@@ -2323,7 +2334,6 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s,\n" - " {\n" - " \tvoid **object;\n" - " \tstruct kmem_cache_cpu *c;\n" - "-\tstruct page *page;\n" - " \tunsigned long tid;\n" - " \n" - " \tif (slab_pre_alloc_hook(s, gfpflags))\n" - "@@ -2350,8 +2360,7 @@ redo:\n" - " \tbarrier();\n" - " \n" - " \tobject = c->freelist;\n" - "-\tpage = c->page;\n" - "-\tif (unlikely(!object || !node_match(page, node)))\n" - "+\tif (unlikely(!object || !node_match(c, node)))\n" - " \t\tobject = __slab_alloc(s, gfpflags, node, addr, c);\n" - " \n" - " \telse {\n" - "-- \n" - "1.7.9.5\n" - "\n" - "--\n" - "To unsubscribe, send a message with 'unsubscribe linux-mm' in\n" - "the body to majordomo@kvack.org. For more info on Linux MM,\n" - "see: http://www.linux-mm.org/ .\n" - "Don't email: <a href=mailto:\"dont@kvack.org\"> email@kvack.org </a>" + -----------------8<----------------------- -ab265932c2809fa1012051323aa8be6bc006b884b0b56b1dc56038d8c1241b3d +5654186cba8805d7d04190ab3be34e29cabbec8e12cf1fd50edb4bd493637dc6
diff --git a/a/1.txt b/N2/1.txt index 6702aed..cee818a 100644 --- a/a/1.txt +++ b/N2/1.txt @@ -192,9 +192,3 @@ index ba2ca53..d4d3d07 100644 else { -- 1.7.9.5 - --- -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> diff --git a/a/content_digest b/N2/content_digest index 547ffd2..b1d79b8 100644 --- a/a/content_digest +++ b/N2/content_digest @@ -218,12 +218,6 @@ " \n" " \telse {\n" "-- \n" - "1.7.9.5\n" - "\n" - "--\n" - "To unsubscribe, send a message with 'unsubscribe linux-mm' in\n" - "the body to majordomo@kvack.org. For more info on Linux MM,\n" - "see: http://www.linux-mm.org/ .\n" - "Don't email: <a href=mailto:\"dont@kvack.org\"> email@kvack.org </a>" + 1.7.9.5 -ab265932c2809fa1012051323aa8be6bc006b884b0b56b1dc56038d8c1241b3d +1bcb7362defa9d49e5629ec540b0518adf72b1e9eeac03451bd232cd65ff7638
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.