Skip to contentSkip to conversation

HWPOSION, hugetlb: lock_page/unlock_page does not match for handling a free hugepage

PATCH In review Chen Yucong 3 participants 3 messages May 10, 2014 – May 12, 2014 1 message not in the archive

Subsystems (2)
Memory management HWPoison memory failure handling
lore
Share & tools
lore

Chen Yucong

author
#0 lore raw

PATCH 1/1 HWPOSION, hugetlb: lock_page/unlock_page does not match for handling a free hugepage

For handling a free hugepage in memory failure, the race will happen if another thread hwpoisoned this hugepage concurrently. So we need to check PageHWPoison instead of !PageHWPoison.

If hwpoison_filter(p) returns true or a race happens, then we need to unlock_page(hpage).

Signed-off-by Chen Yucong <[email protected]>
Reviewed-by Naoya Horiguchi <[email protected]>

---
mm/memory-failure.c | 15 ++++++++-------

1 file changed, 8 insertions(+), 7 deletions(-)
mm/memory-failure.c +0 -0
index 35ef28a..dbf8922 100644
@@ -1081,15 +1081,16 @@ int memory_failure(unsigned long pfn, int

trapno, int flags)
return 0;
} else if (PageHuge(hpage)) {
/*
- * Check "just unpoisoned", "filter hit", and
- * "race with other subpage."
+ * Check "filter hit" and "race with other subpage."
*/
lock_page(hpage);
- if (!PageHWPoison(hpage)

-			    || (hwpoison_filter(p) && TestClearPageHWPoison(p))
-			    || (p != hpage && TestSetPageHWPoison(hpage))) {

- atomic_long_sub(nr_pages, &num_poisoned_pages);
- return 0;
+ if (PageHWPoison(hpage)) {
+ if ((hwpoison_filter(p) && TestClearPageHWPoison(p))
+ || (p != hpage && TestSetPageHWPoison(hpage))) {
+ atomic_long_sub(nr_pages, &num_poisoned_pages);
+ unlock_page(hpage);
+ return 0;
+ }
} set_page_hwpoison_huge_page(hpage); res = dequeue_hwpoisoned_huge_page(hpage);

Signature · 12 lines
1.7.10.4






--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Tested-by on v1 1/1
#1 lore raw

(Cced: Andrew)

Quoted 9 lines · Chen Yucong

For handling a free hugepage in memory failure, the race will happen if another thread hwpoisoned this hugepage concurrently. So we need to check PageHWPoison instead of !PageHWPoison.

If hwpoison_filter(p) returns true or a race happens, then we need to unlock_page(hpage).

Signed-off-by Chen Yucong <[email protected]>
Reviewed-by Naoya Horiguchi <[email protected]>

I tested this patch on latest linux-next, and confirmed that memory error on a tail page of a free hugepage is properly handled.

Tested-by Naoya Horiguchi <[email protected]>

And I think this patch should go into all recent stable trees, since this bug exists since 2.6.36 (because of my patch, sorry.)

mm/memory-failure.c int memory_failure(unsigned long pfn, int · line 1081

Quoted 10 lines

---
mm/memory-failure.c | 15 ++++++++-------

1 file changed, 8 insertions(+), 7 deletions(-)
mm/memory-failure.c +0 -0
index 35ef28a..dbf8922 100644
@@ -1081,15 +1081,16 @@ int memory_failure(unsigned long pfn, int
 trapno, int flags)

This linebreak breaks patch format. I guess it's done by your email client or copy and paste. If it's true, git-send-email might be helpful to avoid such errors.

Thanks,
Naoya

Quoted 22 lines
mm/memory-failure.c +8 -7
 			return 0;
 		} else if (PageHuge(hpage)) {
 			/*
-			 * Check "just unpoisoned", "filter hit", and
-			 * "race with other subpage."
+			 * Check "filter hit" and "race with other subpage."
 			 */
 			lock_page(hpage);
-			if (!PageHWPoison(hpage)
-			    || (hwpoison_filter(p) && TestClearPageHWPoison(p))
-			    || (p != hpage && TestSetPageHWPoison(hpage))) {
-				atomic_long_sub(nr_pages, &num_poisoned_pages);
-				return 0;
+			if (PageHWPoison(hpage)) {
+				if ((hwpoison_filter(p) && TestClearPageHWPoison(p))
+				    || (p != hpage && TestSetPageHWPoison(hpage))) {
+					atomic_long_sub(nr_pages, &num_poisoned_pages);
+					unlock_page(hpage);
+					return 0;
+				}
 			}
 			set_page_hwpoison_huge_page(hpage);
Quoted 15 lines

res = dequeue_hwpoisoned_huge_page(hpage);

Signature · 12 lines
1.7.10.4






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

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

A message that is not in the archive (sent off-list or not yet received): 1399926246-nyqrlpg9@[email protected]

Reviewed-by
#3 lore raw
Quoted 7 lines · Naoya Horiguchi

(Cced: Andrew)

Quoted 4 lines · Chen Yucong

For handling a free hugepage in memory failure, the race will happen if another thread hwpoisoned this hugepage concurrently. So we need to check PageHWPoison instead of !PageHWPoison.

Quoted 8 lines
Quoted 5 lines

If hwpoison_filter(p) returns true or a race happens, then we need to unlock_page(hpage).

Signed-off-by Chen Yucong <[email protected]>
Reviewed-by Naoya Horiguchi <[email protected]>

I tested this patch on latest linux-next, and confirmed that memory error on a tail page of a free hugepage is properly handled.

Patch looks good to me too.

Reviewed-by Andi Kleen <[email protected]>

-Andi

Quoted 62 lines
Tested-by Naoya Horiguchi <[email protected]>

And I think this patch should go into all recent stable trees, since this bug exists since 2.6.36 (because of my patch, sorry.)

mm/memory-failure.c int memory_failure(unsigned long pfn, int · line 1081

Quoted 10 lines

---
mm/memory-failure.c | 15 ++++++++-------

1 file changed, 8 insertions(+), 7 deletions(-)
mm/memory-failure.c +0 -0
index 35ef28a..dbf8922 100644
@@ -1081,15 +1081,16 @@ int memory_failure(unsigned long pfn, int
 trapno, int flags)

This linebreak breaks patch format. I guess it's done by your email client or copy and paste. If it's true, git-send-email might be helpful to avoid such errors.

Thanks,
Naoya

Quoted 37 lines
mm/memory-failure.c +8 -7
 			return 0;
 		} else if (PageHuge(hpage)) {
 			/*
-			 * Check "just unpoisoned", "filter hit", and
-			 * "race with other subpage."
+			 * Check "filter hit" and "race with other subpage."
 			 */
 			lock_page(hpage);
-			if (!PageHWPoison(hpage)
-			    || (hwpoison_filter(p) && TestClearPageHWPoison(p))
-			    || (p != hpage && TestSetPageHWPoison(hpage))) {
-				atomic_long_sub(nr_pages, &num_poisoned_pages);
-				return 0;
+			if (PageHWPoison(hpage)) {
+				if ((hwpoison_filter(p) && TestClearPageHWPoison(p))
+				    || (p != hpage && TestSetPageHWPoison(hpage))) {
+					atomic_long_sub(nr_pages, &num_poisoned_pages);
+					unlock_page(hpage);
+					return 0;
+				}
 			}
 			set_page_hwpoison_huge_page(hpage);

res = dequeue_hwpoisoned_huge_page(hpage);

Signature · 12 lines
1.7.10.4






--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Signature · 7 lines
[email protected] -- Speaking for myself only

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