Re:Re: [PATCH] mm: gup: fix comment of __get_user_pages()

[Liu Xiang]

At 2019-10-24 03:28:14, "John Hubbard" <jhubbard@xxxxxxxxxx> wrote:
>On 10/23/19 6:51 AM, Liu Xiang wrote:
>> Because nr_pages is unsigned long, it can not be negative.
>> 
>> Signed-off-by: Liu Xiang <liuxiang_1999@xxxxxxx>
>> ---
>>  mm/gup.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/mm/gup.c b/mm/gup.c
>> index 8f236a3..0236954 100644
>> --- a/mm/gup.c
>> +++ b/mm/gup.c
>> @@ -735,10 +735,10 @@ static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags)
>>   * @nonblocking: whether waiting for disk IO or mmap_sem contention
>>   *
>>   * Returns number of pages pinned. This may be fewer than the number
>> - * requested. If nr_pages is 0 or negative, returns 0. If no pages
>> - * were pinned, returns -errno. Each page returned must be released
>> - * with a put_page() call when it is finished with. vmas will only
>> - * remain valid while mmap_sem is held.
>> + * requested. If nr_pages is 0, returns 0. If no pages were pinned,
>> + * returns -errno. Each page returned must be released with a
>> + * put_page() call when it is finished with. vmas will only remain
>> + * valid while mmap_sem is held.
>>   *
>>   * Must be called with mmap_sem held.  It may be released.  See below.
>>   *
>> 
>
>Hi Liu,
>
>Thanks for fixing the documentation! As long as you're there...for the actual 
>wording, can we please do it as shown below? This also addresses David's 
>feedback, and it makes this read a lot better overall:
>
>diff --git a/mm/gup.c b/mm/gup.c
>index 8f236a335ae9..5816876fee51 100644
>--- a/mm/gup.c
>+++ b/mm/gup.c
>@@ -734,11 +734,17 @@ static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags)
>  *             Or NULL if the caller does not require them.
>  * @nonblocking: whether waiting for disk IO or mmap_sem contention
>  *
>- * Returns number of pages pinned. This may be fewer than the number
>- * requested. If nr_pages is 0 or negative, returns 0. If no pages
>- * were pinned, returns -errno. Each page returned must be released
>- * with a put_page() call when it is finished with. vmas will only
>- * remain valid while mmap_sem is held.
>+ * Returns either number of pages pinned (which may be less than the number
>+ * requested), or an error. Details about the return value:
>+ *
>+ *      -- If nr_pages is 0, returns 0.
>+ *      -- If nr_pages is >0, but no pages were pinned, returns -errno.
>+ *      -- If nr_pages is >0, and some pages were pinned, returns the number of
>+ *         pages pinned. Again, this may be less than nr_pages.
>+ *
>+ * The caller is responsible for releasing returned @pages, via put_page().
>+ *
>+ * @vmas are valid only as long as mmap_sem is held.
>  *
>  * Must be called with mmap_sem held.  It may be released.  See below.
>  *
>
>
>Patch ordering: I'll have to change the above as part of my upcoming series, to
>make it refer to "put_page() or put_user_page(), depending on FOLL_PIN", 
>approximately. (Just more of a note to self than anything else, here.)
>
>thanks,
>
>John Hubbard
>NVIDIA


Hi John,

Thanks for your suggestion. I will send a new patch.

Regards

Liu Xiang