On 11/19/20 11:57 AM, Oscar Salvador wrote:
From: Naoya Horiguchi <naoya.horigu...@nec.com>

The call to get_user_pages_fast is only to get the pointer to a struct
page of a given address, pinning it is memory-poisoning handler's job,
so drop the refcount grabbed by get_user_pages_fast().

Note that the target page is still pinned after this put_page() because
the current process should have refcount from mapping.

Well, but can't it go away due to reclaim, migration or whatever?

Signed-off-by: Naoya Horiguchi <naoya.horigu...@nec.com>
Signed-off-by: Oscar Salvador <osalva...@suse.de>
---
  mm/madvise.c | 19 +++++++++++--------
  1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/mm/madvise.c b/mm/madvise.c
index c6b5524add58..7a0f64b93635 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -900,20 +900,23 @@ static int madvise_inject_error(int behavior,
                 */
                size = page_size(compound_head(page));
+ /*
+                * The get_user_pages_fast() is just to get the pfn of the
+                * given address, and the refcount has nothing to do with
+                * what we try to test, so it should be released immediately.
+                * This is racy but it's intended because the real hardware
+                * errors could happen at any moment and memory error handlers
+                * must properly handle the race.

Sure they have to. We might just be unexpectedly messing with other process' memory. Or does anything else prevent that?

+                */
+               put_page(page);
+
                if (behavior == MADV_SOFT_OFFLINE) {
                        pr_info("Soft offlining pfn %#lx at process virtual address 
%#lx\n",
                                 pfn, start);
-                       ret = soft_offline_page(pfn, MF_COUNT_INCREASED);
+                       ret = soft_offline_page(pfn, 0);
                } else {
                        pr_info("Injecting memory failure for pfn %#lx at process 
virtual address %#lx\n",
                                 pfn, start);
-                       /*
-                        * Drop the page reference taken by 
get_user_pages_fast(). In
-                        * the absence of MF_COUNT_INCREASED the 
memory_failure()
-                        * routine is responsible for pinning the page to 
prevent it
-                        * from being released back to the page allocator.
-                        */
-                       put_page(page);
                        ret = memory_failure(pfn, 0);
                }

Reply via email to