Re: [PATCH] staging: gasket: Convert get_user_pages*() --> pin_user_pages*()

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

 



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@xxxxxxxxx> wrote:

On Thu, May 28, 2020 at 4:34 PM Dan Carpenter <dan.carpenter@xxxxxxxxxx> 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
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux