Messages in this thread | | | Subject | Re: [PATCH] staging: gasket: Convert get_user_pages*() --> pin_user_pages*() | From | John Hubbard <> | Date | Fri, 29 May 2020 01:00:22 -0700 |
| |
On 2020-05-29 00:46, Dan Carpenter wrote: > On Fri, May 29, 2020 at 11:57:09AM +0530, Souptick Joarder wrote: >> On Fri, May 29, 2020 at 11:46 AM Souptick Joarder <jrdr.linux@gmail.com> wrote: >>> >>> On Thu, May 28, 2020 at 4:34 PM Dan Carpenter <dan.carpenter@oracle.com> wrote: >>>> >>>> On Thu, May 28, 2020 at 02:32:42AM +0530, Souptick Joarder wrote: >>>>> This code was using get_user_pages_fast(), in a "Case 2" scenario >>>>> (DMA/RDMA), using the categorization from [1]. That means that it's >>>>> time to convert the get_user_pages_fast() + put_page() calls to >>>>> pin_user_pages_fast() + unpin_user_page() calls. >>>> >>>> You are saying that the page is used for DIO and not DMA, but it sure >>>> looks to me like it is used for DMA. >>> >>> No, I was referring to "Case 2" scenario in change log which means it is >>> used for DMA, not DIO. > > You can't use pin_user_pages() for DMA. This was second reason that I > was confused.
OK, now it is getting interesting!
> > mm/gup.c > 2863 /** > 2864 * pin_user_pages_fast() - pin user pages in memory without taking locks > 2865 * > 2866 * @start: starting user address > 2867 * @nr_pages: number of pages from start to pin > 2868 * @gup_flags: flags modifying pin behaviour > 2869 * @pages: array that receives pointers to the pages pinned. > 2870 * Should be at least nr_pages long. > 2871 * > 2872 * Nearly the same as get_user_pages_fast(), except that FOLL_PIN is set. See > 2873 * get_user_pages_fast() for documentation on the function arguments, because > 2874 * the arguments here are identical. > 2875 * > 2876 * FOLL_PIN means that the pages must be released via unpin_user_page(). Please > 2877 * see Documentation/core-api/pin_user_pages.rst for further details. > 2878 * > 2879 * This is intended for Case 1 (DIO) in Documentation/core-api/pin_user_pages.rst. It > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > 2880 * is NOT intended for Case 2 (RDMA: long-term pins). > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
I'm trying to figure out why I wrote that. It seems just wrong, because once the page is dma-pinned, it will work just fine for either Case 1 or Case 2. hmmm, I think this was from a few design ideas ago, when we were still working through the FOLL_LONGTERM and FOLL_PIN thoughts and how the pin_user_pages*() API set should look.
At this point, it's looking very much like a (my) documentation bug: all 4 of the "intended for Case 1 (DIO)" comments in mm/gup.c probably need to be simply deleted. Good catch.
> 2881 */ > 2882 int pin_user_pages_fast(unsigned long start, int nr_pages, > 2883 unsigned int gup_flags, struct page **pages) > 2884 { > 2885 /* FOLL_GET and FOLL_PIN are mutually exclusive. */ > 2886 if (WARN_ON_ONCE(gup_flags & FOLL_GET)) > 2887 return -EINVAL; > 2888 > 2889 gup_flags |= FOLL_PIN; > 2890 return internal_get_user_pages_fast(start, nr_pages, gup_flags, pages); > 2891 } > 2892 EXPORT_SYMBOL_GPL(pin_user_pages_fast); > > regards, > dan carpenter >
thanks, -- John Hubbard NVIDIA
| |