Re: [PATCH v1 1/1] mm/gup: skip pinnable check for refs==1

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/31/22 23:35, John Hubbard wrote:
On 1/31/22 23:28, Minchan Kim wrote:
...
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -135,7 +135,7 @@ struct page *try_grab_compound_head(struct page *page,
            * right zone, so fail and let the caller fall back to the slow
            * path.
            */
-        if (unlikely((flags & FOLL_LONGTERM) &&
+        if (refs > 1 && unlikely((flags & FOLL_LONGTERM) &&
                    !is_pinnable_page(page)))
               return NULL;

...but are you really sure that this is the best way to "fix" the
problem? This trades correctness for "bug-for-bug compatibility" with
the previous code. It says, "it's OK to violate the pinnable and
longterm checks, as long as you do it one page at a time, rather than in
larger chunks.

Wouldn't it be better to try to fix up the calling code so that it's
not in violation of these zone rules?

I think the problem is before pin_user_pages can work with CMA pages
in the fallback path but now it doesn't work with CMA page. Driver

Actually, it "worked" only if the caller did it one page at a time.
(See how the above attempted fix restores the "make it work for
refs == 1.)

couldn't know whether it will work with CMA page or not in advance.

pin_user_pages
   __get_user_pages_locked
     follow_page_mask
       follow_page_pte
         try_grab_page
           !is_pinnable_page(page)
             return NULL;
         return ERR_PTR(-ENOMEM);
      return -ENOMEM without faultin_page

Yes, that's all clear.
...oh, but I guess you're pointing out that it's always going to be
page-at-a-time this deep in the pin_user_pages() call path. Which is true.

I hadn't worked through how to fix this yet, my initial reaction was
that allowing single refs to go through, while prohibiting multiple refs,
was clearly *not* the way to go.

thanks,
--
John Hubbard
NVIDIA





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux