On 11/18/19 1:46 AM, Jan Kara wrote:
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;
Nice touch, I've applied it.
thanks,