[PATCH v2 3/7] mm/gup: Fix the outdated code comments above get_user_pages_unlocked()

Baoquan He posted 7 patches 8 months, 3 weeks ago
There is a newer version of this series
[PATCH v2 3/7] mm/gup: Fix the outdated code comments above get_user_pages_unlocked()
Posted by Baoquan He 8 months, 3 weeks ago
Since commit f0818f472d8d ("mm: gup: add get_user_pages_locked
and get_user_pages_unlocked"), get_user_pages() doesn't need to have
mmap_lock held anymore. It calls __get_user_pages_locked() which
can acquire and drop the mmap_lock internaly.

Hence remove the incorrect code comments now.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 mm/gup.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/mm/gup.c b/mm/gup.c
index f9bce14ed3cd..a15317cf6641 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2702,19 +2702,9 @@ long get_user_pages(unsigned long start, unsigned long nr_pages,
 EXPORT_SYMBOL(get_user_pages);
 
 /*
- * get_user_pages_unlocked() is suitable to replace the form:
- *
- *      mmap_read_lock(mm);
- *      get_user_pages(mm, ..., pages, NULL);
- *      mmap_read_unlock(mm);
- *
- *  with:
- *
- *      get_user_pages_unlocked(mm, ..., pages);
- *
- * It is functionally equivalent to get_user_pages_fast so
- * get_user_pages_fast should be used instead if specific gup_flags
- * (e.g. FOLL_FORCE) are not required.
+ * get_user_pages_unlocked() is functionally equivalent to
+ * get_user_pages_fast so get_user_pages_fast should be used instead
+ * if specific gup_flags (e.g. FOLL_FORCE) are not required.
  */
 long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
 			     struct page **pages, unsigned int gup_flags)
-- 
2.41.0
Re: [PATCH v2 3/7] mm/gup: Fix the outdated code comments above get_user_pages_unlocked()
Posted by Oscar Salvador 8 months, 3 weeks ago
On Mon, Mar 31, 2025 at 04:13:23PM +0800, Baoquan He wrote:
> Since commit f0818f472d8d ("mm: gup: add get_user_pages_locked
> and get_user_pages_unlocked"), get_user_pages() doesn't need to have
> mmap_lock held anymore. It calls __get_user_pages_locked() which
> can acquire and drop the mmap_lock internaly.
> 
> Hence remove the incorrect code comments now.

Uhm, I think this one should be dropped according to 

https://lore.kernel.org/linux-mm/20250330121718.175815-4-bhe@redhat.com/T/#m1aa161016ab98a6d85bcb657a729e01cb98763ec

?


-- 
Oscar Salvador
SUSE Labs
Re: [PATCH v2 3/7] mm/gup: Fix the outdated code comments above get_user_pages_unlocked()
Posted by Baoquan He 8 months, 3 weeks ago
On 04/01/25 at 03:51pm, Oscar Salvador wrote:
> On Mon, Mar 31, 2025 at 04:13:23PM +0800, Baoquan He wrote:
> > Since commit f0818f472d8d ("mm: gup: add get_user_pages_locked
> > and get_user_pages_unlocked"), get_user_pages() doesn't need to have
> > mmap_lock held anymore. It calls __get_user_pages_locked() which
> > can acquire and drop the mmap_lock internaly.
> > 
> > Hence remove the incorrect code comments now.
> 
> Uhm, I think this one should be dropped according to 

Yeah, this v2 series was sent earlier than your comment in v1. Sorry
about the mess.

> 
> https://lore.kernel.org/linux-mm/20250330121718.175815-4-bhe@redhat.com/T/#m1aa161016ab98a6d85bcb657a729e01cb98763ec
> 
> ?
> 
> 
> -- 
> Oscar Salvador
> SUSE Labs
>
Re: [PATCH v2 3/7] mm/gup: Fix the outdated code comments above get_user_pages_unlocked()
Posted by David Hildenbrand 8 months, 3 weeks ago
On 31.03.25 10:13, Baoquan He wrote:
> Since commit f0818f472d8d ("mm: gup: add get_user_pages_locked
> and get_user_pages_unlocked"), get_user_pages() doesn't need to have
> mmap_lock held anymore. It calls __get_user_pages_locked() which
> can acquire and drop the mmap_lock internaly.

s/internaly/internally/

But your statement is wrong. get_user_pages() must be called with the 
mmap_lock held, because it sets "int locked = 1;" when calling 
__get_user_pages_locked().

-- 
Cheers,

David / dhildenb
Re: [PATCH v2 3/7] mm/gup: Fix the outdated code comments above get_user_pages_unlocked()
Posted by Baoquan He 8 months, 3 weeks ago
On 04/01/25 at 10:14am, David Hildenbrand wrote:
> On 31.03.25 10:13, Baoquan He wrote:
> > Since commit f0818f472d8d ("mm: gup: add get_user_pages_locked
> > and get_user_pages_unlocked"), get_user_pages() doesn't need to have
> > mmap_lock held anymore. It calls __get_user_pages_locked() which
> > can acquire and drop the mmap_lock internaly.
> 
> s/internaly/internally/
> 
> But your statement is wrong. get_user_pages() must be called with the
> mmap_lock held, because it sets "int locked = 1;" when calling
> __get_user_pages_locked().

You are right. I missed that line. Oscar pointed that out in v1 thread.
I will drop this patch. Thanks.