mm/memory-failure: Fix redundant updates for already poisoned pages
Subsystems (3)
PATCH v2 1/1 mm/memory-failure: Fix redundant updates for already poisoned pages
Duplicate memory errors can be reported by multiple sources.
Passing an already poisoned page to action_result() causes issues:
* The amount of hardware corrupted memory is incorrectly updated.
* Per NUMA node MF stats are incorrectly updated.
* Redundant "already poisoned" messages are printed.
Avoid those issues by:
* Skipping hardware corrupted memory updates for already poisoned pages.
* Skipping per NUMA node MF stats updates for already poisoned pages.
* Dropping redundant "already poisoned" messages.
Make MF_MSG_ALREADY_POISONED consistent with other action_page_types and make calls to action_result() consistent for already poisoned normal pages and huge pages.
---
v1 -> v2:
* Continue passing poisoned pages to action_result() with MF_FAILED but don't
update anything.
* https://lore.kernel.org/all/[email protected]
--- mm/memory-failure.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
mm/memory-failure.c +6 -7
@@ -956,7 +956,7 @@ static const char * const action_page_types[] = { [MF_MSG_BUDDY] = "free buddy page", [MF_MSG_DAX] = "dax page", [MF_MSG_UNSPLIT_THP] = "unsplit thp", - [MF_MSG_ALREADY_POISONED] = "already poisoned", + [MF_MSG_ALREADY_POISONED] = "already poisoned page", [MF_MSG_UNKNOWN] = "unknown page", };
@@ -1349,9 +1349,10 @@ static int action_result(unsigned long pfn, enum mf_action_page_type type, {trace_memory_failure_event (pfn, type, result); -num_poisoned_pages_inc (pfn); - -update_per_node_mf_stats (pfn, result); +if (type !=MF_MSG_ALREADY_POISONED ) { +num_poisoned_pages_inc (pfn); +update_per_node_mf_stats (pfn, result); + }pr_err ("%#lx: recovery action for %s: %s\n" , pfn, action_page_types[type], action_name[result]);
@@ -2094,12 +2095,11 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb *hugetlb =0 ;return 0 ; }else if (res == -EHWPOISON ) { -pr_err ("%#lx: already hardware poisoned\n" , pfn);if (flags &MF_ACTION_REQUIRED ) { folio =page_folio (p); res =kill_accessing_process (current,folio_pfn (folio), flags); -action_result (pfn,MF_MSG_ALREADY_POISONED ,MF_FAILED ); } +action_result (pfn,MF_MSG_ALREADY_POISONED ,MF_FAILED );return res; }else if (res == -EBUSY ) {if (!(flags &MF_NO_RETRY )) {
@@ -2285,7 +2285,6 @@ int memory_failure(unsigned long pfn, int flags) goto unlock_mutex;if (TestSetPageHWPoison (p)) { -pr_err ("%#lx: already hardware poisoned\n" , pfn); res = -EHWPOISON ;if (flags &MF_ACTION_REQUIRED ) res =kill_accessing_process (current, pfn, flags);
Signature · 1 line
2.50.1
Quoted 46 lines · Kyle Meyer
Duplicate memory errors can be reported by multiple sources.
Passing an already poisoned page to
action_result()causes issues:* The amount of hardware corrupted memory is incorrectly updated.
* Per NUMA node MF stats are incorrectly updated.
* Redundant "already poisoned" messages are printed.Avoid those issues by:
* Skipping hardware corrupted memory updates for already poisoned pages.
* Skipping per NUMA node MF stats updates for already poisoned pages.
* Dropping redundant "already poisoned" messages.Make
MF_MSG_ALREADY_POISONEDconsistent with otheraction_page_typesand make calls toaction_result()consistent for already poisoned normal pages and huge pages.Fixes b8b9488d50b7 ("mm/memory-failure: improve memory failure action_result messages")Signed-off-by Kyle Meyer <[email protected]>---
v1 -> v2:
* Continue passing poisoned pages toaction_result()withMF_FAILEDbut don't
update anything.
* https://lore.kernel.org/all/[email protected]--- mm/memory-failure.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)mm/memory-failure.c +1
-1@@ -956,7 +956,7 @@ static const char * const action_page_types[] = { [MF_MSG_BUDDY] = "free buddy page", [MF_MSG_DAX] = "dax page", [MF_MSG_UNSPLIT_THP] = "unsplit thp", - [MF_MSG_ALREADY_POISONED] = "already poisoned", + [MF_MSG_ALREADY_POISONED] = "already poisoned page", [MF_MSG_UNKNOWN] = "unknown page", };
mm/memory-failure.c static int action_result(unsigned long pfn, enum mf_action_page_type type, · line 1355
Quoted 10 lines
mm/memory-failure.c +4
-3@@ -1349,9 +1349,10 @@ static int action_result(unsigned long pfn, enum mf_action_page_type type, {trace_memory_failure_event (pfn, type, result); -num_poisoned_pages_inc (pfn); - -update_per_node_mf_stats (pfn, result); +if (type !=MF_MSG_ALREADY_POISONED ) { +num_poisoned_pages_inc (pfn); +update_per_node_mf_stats (pfn, result); + }
Thanks Kyle, the code looks good to me. While you wait for Miaohe's review/ack...
Quoted 28 lines
mm/memory-failure.c +1
-4pr_err ("%#lx: recovery action for %s: %s\n" , pfn, action_page_types[type], action_name[result]);@@ -2094,12 +2095,11 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb *hugetlb =0 ;return 0 ; }else if (res == -EHWPOISON ) { -pr_err ("%#lx: already hardware poisoned\n" , pfn);if (flags &MF_ACTION_REQUIRED ) { folio =page_folio (p); res =kill_accessing_process (current,folio_pfn (folio), flags); -action_result (pfn,MF_MSG_ALREADY_POISONED ,MF_FAILED ); } +action_result (pfn,MF_MSG_ALREADY_POISONED ,MF_FAILED );return res; }else if (res == -EBUSY ) {if (!(flags &MF_NO_RETRY )) {@@ -2285,7 +2285,6 @@ int memory_failure(unsigned long pfn, int flags) goto unlock_mutex;if (TestSetPageHWPoison (p)) { -pr_err ("%#lx: already hardware poisoned\n" , pfn); res = -EHWPOISON ;if (flags &MF_ACTION_REQUIRED ) res =kill_accessing_process (current, pfn, flags); --2.50.1
Quoted 59 lines · Kyle Meyer
Duplicate memory errors can be reported by multiple sources.
Passing an already poisoned page to
action_result()causes issues:* The amount of hardware corrupted memory is incorrectly updated.
* Per NUMA node MF stats are incorrectly updated.
* Redundant "already poisoned" messages are printed.Avoid those issues by:
* Skipping hardware corrupted memory updates for already poisoned pages.
* Skipping per NUMA node MF stats updates for already poisoned pages.
* Dropping redundant "already poisoned" messages.Make
MF_MSG_ALREADY_POISONEDconsistent with otheraction_page_typesand make calls toaction_result()consistent for already poisoned normal pages and huge pages.Fixes b8b9488d50b7 ("mm/memory-failure: improve memory failure action_result messages")Signed-off-by Kyle Meyer <[email protected]>---
v1 -> v2:
* Continue passing poisoned pages toaction_result()withMF_FAILEDbut don't
update anything.
* https://lore.kernel.org/all/[email protected]--- mm/memory-failure.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)mm/memory-failure.c +5
-4@@ -956,7 +956,7 @@ static const char * const action_page_types[] = { [MF_MSG_BUDDY] = "free buddy page", [MF_MSG_DAX] = "dax page", [MF_MSG_UNSPLIT_THP] = "unsplit thp", - [MF_MSG_ALREADY_POISONED] = "already poisoned", + [MF_MSG_ALREADY_POISONED] = "already poisoned page", [MF_MSG_UNKNOWN] = "unknown page", };@@ -1349,9 +1349,10 @@ static int action_result(unsigned long pfn, enum mf_action_page_type type, {trace_memory_failure_event (pfn, type, result); -num_poisoned_pages_inc (pfn); - -update_per_node_mf_stats (pfn, result); +if (type !=MF_MSG_ALREADY_POISONED ) { +num_poisoned_pages_inc (pfn); +update_per_node_mf_stats (pfn, result); + }pr_err ("%#lx: recovery action for %s: %s\n" , pfn, action_page_types[type], action_name[result]);
mm/memory-failure.c static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb · line 2102
Quoted 10 lines
mm/memory-failure.c +1
-2@@ -2094,12 +2095,11 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb *hugetlb =0 ;return 0 ; }else if (res == -EHWPOISON ) { -pr_err ("%#lx: already hardware poisoned\n" , pfn);if (flags &MF_ACTION_REQUIRED ) { folio =page_folio (p); res =kill_accessing_process (current,folio_pfn (folio), flags); -action_result (pfn,MF_MSG_ALREADY_POISONED ,MF_FAILED ); } +action_result (pfn,MF_MSG_ALREADY_POISONED ,MF_FAILED );
Was briefly confused by that change, but action_result() now essentially only traces + pr_err() with MF_MSG_ALREADY_POISONED.
Signature · 3 lines
Cheers David / dhildenb
Quoted 73 lines · Kyle Meyer
Duplicate memory errors can be reported by multiple sources.
Passing an already poisoned page to
action_result()causes issues:* The amount of hardware corrupted memory is incorrectly updated.
* Per NUMA node MF stats are incorrectly updated.
* Redundant "already poisoned" messages are printed.Avoid those issues by:
* Skipping hardware corrupted memory updates for already poisoned pages.
* Skipping per NUMA node MF stats updates for already poisoned pages.
* Dropping redundant "already poisoned" messages.Make
MF_MSG_ALREADY_POISONEDconsistent with otheraction_page_typesand make calls toaction_result()consistent for already poisoned normal pages and huge pages.Fixes b8b9488d50b7 ("mm/memory-failure: improve memory failure action_result messages")Signed-off-by Kyle Meyer <[email protected]>---
v1 -> v2:
* Continue passing poisoned pages toaction_result()withMF_FAILEDbut don't
update anything.
* https://lore.kernel.org/all/[email protected]--- mm/memory-failure.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)mm/memory-failure.c +6
-6@@ -956,7 +956,7 @@ static const char * const action_page_types[] = { [MF_MSG_BUDDY] = "free buddy page", [MF_MSG_DAX] = "dax page", [MF_MSG_UNSPLIT_THP] = "unsplit thp", - [MF_MSG_ALREADY_POISONED] = "already poisoned", + [MF_MSG_ALREADY_POISONED] = "already poisoned page", [MF_MSG_UNKNOWN] = "unknown page", };@@ -1349,9 +1349,10 @@ static int action_result(unsigned long pfn, enum mf_action_page_type type, {trace_memory_failure_event (pfn, type, result); -num_poisoned_pages_inc (pfn); - -update_per_node_mf_stats (pfn, result); +if (type !=MF_MSG_ALREADY_POISONED ) { +num_poisoned_pages_inc (pfn); +update_per_node_mf_stats (pfn, result); + }pr_err ("%#lx: recovery action for %s: %s\n" , pfn, action_page_types[type], action_name[result]);@@ -2094,12 +2095,11 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb *hugetlb =0 ;return 0 ; }else if (res == -EHWPOISON ) { -pr_err ("%#lx: already hardware poisoned\n" , pfn);if (flags &MF_ACTION_REQUIRED ) { folio =page_folio (p); res =kill_accessing_process (current,folio_pfn (folio), flags); -action_result (pfn,MF_MSG_ALREADY_POISONED ,MF_FAILED ); } +action_result (pfn,MF_MSG_ALREADY_POISONED ,MF_FAILED );return res; }else if (res == -EBUSY ) {if (!(flags &MF_NO_RETRY )) {
mm/memory-failure.c int memory_failure(unsigned long pfn, int flags) · line 2290
Quoted 7 lines
mm/memory-failure.c +0
-1@@ -2285,7 +2285,6 @@ int memory_failure(unsigned long pfn, int flags) goto unlock_mutex;if (TestSetPageHWPoison (p)) { -pr_err ("%#lx: already hardware poisoned\n" , pfn); res = -EHWPOISON ;if (flags &MF_ACTION_REQUIRED ) res =kill_accessing_process (current, pfn, flags);
Looks good, thanks!
-jane
Quoted 14 lines · Kyle Meyer
Duplicate memory errors can be reported by multiple sources.
Passing an already poisoned page to
action_result()causes issues:* The amount of hardware corrupted memory is incorrectly updated.
* Per NUMA node MF stats are incorrectly updated.
* Redundant "already poisoned" messages are printed.Avoid those issues by:
* Skipping hardware corrupted memory updates for already poisoned pages.
* Skipping per NUMA node MF stats updates for already poisoned pages.
* Dropping redundant "already poisoned" messages.
Quoted 6 lines
Make
MF_MSG_ALREADY_POISONEDconsistent with otheraction_page_typesand make calls toaction_result()consistent for already poisoned normal pages and huge pages.Fixes b8b9488d50b7 ("mm/memory-failure: improve memory failure action_result messages")Signed-off-by Kyle Meyer <[email protected]>
Thanks for your patch. This patch looks good to me.
Thanks.
.
Quoted 61 lines
---
v1 -> v2:
* Continue passing poisoned pages toaction_result()withMF_FAILEDbut don't
update anything.
* https://lore.kernel.org/all/[email protected]--- mm/memory-failure.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)mm/memory-failure.c +6
-7@@ -956,7 +956,7 @@ static const char * const action_page_types[] = { [MF_MSG_BUDDY] = "free buddy page", [MF_MSG_DAX] = "dax page", [MF_MSG_UNSPLIT_THP] = "unsplit thp", - [MF_MSG_ALREADY_POISONED] = "already poisoned", + [MF_MSG_ALREADY_POISONED] = "already poisoned page", [MF_MSG_UNKNOWN] = "unknown page", };@@ -1349,9 +1349,10 @@ static int action_result(unsigned long pfn, enum mf_action_page_type type, {trace_memory_failure_event (pfn, type, result); -num_poisoned_pages_inc (pfn); - -update_per_node_mf_stats (pfn, result); +if (type !=MF_MSG_ALREADY_POISONED ) { +num_poisoned_pages_inc (pfn); +update_per_node_mf_stats (pfn, result); + }pr_err ("%#lx: recovery action for %s: %s\n" , pfn, action_page_types[type], action_name[result]);@@ -2094,12 +2095,11 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb *hugetlb =0 ;return 0 ; }else if (res == -EHWPOISON ) { -pr_err ("%#lx: already hardware poisoned\n" , pfn);if (flags &MF_ACTION_REQUIRED ) { folio =page_folio (p); res =kill_accessing_process (current,folio_pfn (folio), flags); -action_result (pfn,MF_MSG_ALREADY_POISONED ,MF_FAILED ); } +action_result (pfn,MF_MSG_ALREADY_POISONED ,MF_FAILED );return res; }else if (res == -EBUSY ) {if (!(flags &MF_NO_RETRY )) {@@ -2285,7 +2285,6 @@ int memory_failure(unsigned long pfn, int flags) goto unlock_mutex;if (TestSetPageHWPoison (p)) { -pr_err ("%#lx: already hardware poisoned\n" , pfn); res = -EHWPOISON ;if (flags &MF_ACTION_REQUIRED ) res =kill_accessing_process (current, pfn, flags);