| Message ID | 20210904004224.86391-1-jhubbard@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/gup: further simplify __gup_device_huge() | expand |
On Fri 03-09-21 17:42:24, John Hubbard wrote: > commit 6401c4eb57f9 ("mm: gup: fix potential pgmap refcnt leak in > __gup_device_huge()") simplified the return paths, but didn't go quite > far enough, as discussed in [1]. > > Remove the "ret" variable entirely, because there is enough information > already available to provide the return value. > > [1] https://lore.kernel.org/r/CAHk-=wgQTRX=5SkCmS+zfmpqubGHGJvXX_HgnPG8JSpHKHBMeg@mail.gmail.com > > Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> > Cc: Miaohe Lin <linmiaohe@huawei.com> > Cc: Claudio Imbrenda <imbrenda@linux.ibm.com> > Cc: Jan Kara <jack@suse.cz> > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > Signed-off-by: John Hubbard <jhubbard@nvidia.com> Looks sane. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > mm/gup.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/mm/gup.c b/mm/gup.c > index 9935a4480710..6e62f1518c1f 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -2241,7 +2241,6 @@ static int __gup_device_huge(unsigned long pfn, unsigned long addr, > { > int nr_start = *nr; > struct dev_pagemap *pgmap = NULL; > - int ret = 1; > > do { > struct page *page = pfn_to_page(pfn); > @@ -2249,14 +2248,12 @@ static int __gup_device_huge(unsigned long pfn, unsigned long addr, > pgmap = get_dev_pagemap(pfn, pgmap); > if (unlikely(!pgmap)) { > undo_dev_pagemap(nr, nr_start, flags, pages); > - ret = 0; > break; > } > SetPageReferenced(page); > pages[*nr] = page; > if (unlikely(!try_grab_page(page, flags))) { > undo_dev_pagemap(nr, nr_start, flags, pages); > - ret = 0; > break; > } > (*nr)++; > @@ -2264,7 +2261,7 @@ static int __gup_device_huge(unsigned long pfn, unsigned long addr, > } while (addr += PAGE_SIZE, addr != end); > > put_dev_pagemap(pgmap); > - return ret; > + return addr == end; > } > > static int __gup_device_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr, > -- > 2.33.0 >
diff --git a/mm/gup.c b/mm/gup.c index 9935a4480710..6e62f1518c1f 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -2241,7 +2241,6 @@ static int __gup_device_huge(unsigned long pfn, unsigned long addr, { int nr_start = *nr; struct dev_pagemap *pgmap = NULL; - int ret = 1; do { struct page *page = pfn_to_page(pfn); @@ -2249,14 +2248,12 @@ static int __gup_device_huge(unsigned long pfn, unsigned long addr, pgmap = get_dev_pagemap(pfn, pgmap); if (unlikely(!pgmap)) { undo_dev_pagemap(nr, nr_start, flags, pages); - ret = 0; break; } SetPageReferenced(page); pages[*nr] = page; if (unlikely(!try_grab_page(page, flags))) { undo_dev_pagemap(nr, nr_start, flags, pages); - ret = 0; break; } (*nr)++; @@ -2264,7 +2261,7 @@ static int __gup_device_huge(unsigned long pfn, unsigned long addr, } while (addr += PAGE_SIZE, addr != end); put_dev_pagemap(pgmap); - return ret; + return addr == end; } static int __gup_device_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
commit 6401c4eb57f9 ("mm: gup: fix potential pgmap refcnt leak in __gup_device_huge()") simplified the return paths, but didn't go quite far enough, as discussed in [1]. Remove the "ret" variable entirely, because there is enough information already available to provide the return value. [1] https://lore.kernel.org/r/CAHk-=wgQTRX=5SkCmS+zfmpqubGHGJvXX_HgnPG8JSpHKHBMeg@mail.gmail.com Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> Cc: Miaohe Lin <linmiaohe@huawei.com> Cc: Claudio Imbrenda <imbrenda@linux.ibm.com> Cc: Jan Kara <jack@suse.cz> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> Signed-off-by: John Hubbard <jhubbard@nvidia.com> --- mm/gup.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)