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