| Message ID | 20250331081327.256412-4-bhe@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/gup: Minor fix, cleanup and improvements | expand |
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().
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 ?
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.
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 >
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)
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(-)