| Message ID | 20191017142123.24245-15-osalvador@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Hwpoison rework {hard,soft}-offline | expand |
On Thu, Oct 17, 2019 at 04:21:21PM +0200, Oscar Salvador wrote: > Currently, there is an inconsistency when calling soft-offline from > different paths on a page that is already poisoned. > > 1) madvise: > > madvise_inject_error skips any poisoned page and continues > the loop. > If that was the only page to madvise, it returns 0. > > 2) /sys/devices/system/memory/: > > Whe calling soft_offline_page_store()->soft_offline_page(), > we return -EBUSY in case the page is already poisoned. > This is inconsistent with a) the above example and b) > memory_failure, where we return 0 if the page was poisoned. > > Fix this by dropping the PageHWPoison() check in madvise_inject_error, > and let soft_offline_page return 0 if it finds the page already poisoned. > > Please, note that this represents an user-api change, since now the return > error when calling soft_offline_page_store()->soft_offline_page() will be different. > > Signed-off-by: Oscar Salvador <osalvador@suse.de> Looks good to me. Acked-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> > --- > mm/madvise.c | 3 --- > mm/memory-failure.c | 4 ++-- > 2 files changed, 2 insertions(+), 5 deletions(-) > > diff --git a/mm/madvise.c b/mm/madvise.c > index 8a0b1f901d72..9ca48345ce45 100644 > --- a/mm/madvise.c > +++ b/mm/madvise.c > @@ -887,9 +887,6 @@ static int madvise_inject_error(int behavior, > */ > put_page(page); > > - if (PageHWPoison(page)) > - continue; > - > if (behavior == MADV_SOFT_OFFLINE) { > pr_info("Soft offlining pfn %#lx at process virtual address %#lx\n", > pfn, start); > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index 3d491c0d3f91..c038896bedf0 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -1767,7 +1767,7 @@ static int __soft_offline_page(struct page *page) > unlock_page(page); > put_page(page); > pr_info("soft offline: %#lx page already poisoned\n", pfn); > - return -EBUSY; > + return 0; > } > > if (!PageHuge(page)) > @@ -1866,7 +1866,7 @@ int soft_offline_page(struct page *page) > > if (PageHWPoison(page)) { > pr_info("soft offline: %#lx page already poisoned\n", pfn); > - return -EBUSY; > + return 0; > } > > get_online_mems(); > -- > 2.12.3 > >
diff --git a/mm/madvise.c b/mm/madvise.c index 8a0b1f901d72..9ca48345ce45 100644 --- a/mm/madvise.c +++ b/mm/madvise.c @@ -887,9 +887,6 @@ static int madvise_inject_error(int behavior, */ put_page(page); - if (PageHWPoison(page)) - continue; - if (behavior == MADV_SOFT_OFFLINE) { pr_info("Soft offlining pfn %#lx at process virtual address %#lx\n", pfn, start); diff --git a/mm/memory-failure.c b/mm/memory-failure.c index 3d491c0d3f91..c038896bedf0 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -1767,7 +1767,7 @@ static int __soft_offline_page(struct page *page) unlock_page(page); put_page(page); pr_info("soft offline: %#lx page already poisoned\n", pfn); - return -EBUSY; + return 0; } if (!PageHuge(page)) @@ -1866,7 +1866,7 @@ int soft_offline_page(struct page *page) if (PageHWPoison(page)) { pr_info("soft offline: %#lx page already poisoned\n", pfn); - return -EBUSY; + return 0; } get_online_mems();
Currently, there is an inconsistency when calling soft-offline from different paths on a page that is already poisoned. 1) madvise: madvise_inject_error skips any poisoned page and continues the loop. If that was the only page to madvise, it returns 0. 2) /sys/devices/system/memory/: Whe calling soft_offline_page_store()->soft_offline_page(), we return -EBUSY in case the page is already poisoned. This is inconsistent with a) the above example and b) memory_failure, where we return 0 if the page was poisoned. Fix this by dropping the PageHWPoison() check in madvise_inject_error, and let soft_offline_page return 0 if it finds the page already poisoned. Please, note that this represents an user-api change, since now the return error when calling soft_offline_page_store()->soft_offline_page() will be different. Signed-off-by: Oscar Salvador <osalvador@suse.de> --- mm/madvise.c | 3 --- mm/memory-failure.c | 4 ++-- 2 files changed, 2 insertions(+), 5 deletions(-)