Messages in this thread | | | Date | Mon, 18 Nov 2019 10:46:04 +0100 | From | Jan Kara <> | Subject | Re: [PATCH v5 02/24] mm/gup: factor out duplicate code from four routines |
| |
On Thu 14-11-19 21:53:18, John Hubbard wrote: > There are four locations in gup.c that have a fair amount of code > duplication. This means that changing one requires making the same > changes in four places, not to mention reading the same code four > times, and wondering if there are subtle differences. > > Factor out the common code into static functions, thus reducing the > overall line count and the code's complexity. > > Also, take the opportunity to slightly improve the efficiency of the > error cases, by doing a mass subtraction of the refcount, surrounded > by get_page()/put_page(). > > Also, further simplify (slightly), by waiting until the the successful > end of each routine, to increment *nr. > > Reviewed-by: Jérôme Glisse <jglisse@redhat.com> > Cc: Jan Kara <jack@suse.cz> > Cc: Ira Weiny <ira.weiny@intel.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > Signed-off-by: John Hubbard <jhubbard@nvidia.com> > --- > mm/gup.c | 95 ++++++++++++++++++++++++-------------------------------- > 1 file changed, 40 insertions(+), 55 deletions(-) > > diff --git a/mm/gup.c b/mm/gup.c > index 85caf76b3012..858541ea30ce 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -1969,6 +1969,29 @@ static int __gup_device_huge_pud(pud_t pud, pud_t *pudp, unsigned long addr, > } > #endif > > +static int __record_subpages(struct page *page, unsigned long addr, > + unsigned long end, struct page **pages) > +{ > + int nr = 0; > + int nr_recorded_pages = 0; > + > + do { > + pages[nr] = page; > + nr++; > + page++; > + nr_recorded_pages++; > + } while (addr += PAGE_SIZE, addr != end); > + return nr_recorded_pages;
nr == nr_recorded_pages so no need for both... BTW, structuring this as a for loop would be probably more logical and shorter now:
for (nr = 0; addr != end; addr += PAGE_SIZE) pages[nr++] = page++; return nr;
The rest of the patch looks good to me.
Honza
-- Jan Kara <jack@suse.com> SUSE Labs, CR
| |