Re: [PATCH v7 02/24] mm/gup: factor out duplicate code from four routines

From: John Hubbard
Date: Thu Nov 21 2019 - 03:32:51 EST


On 11/21/19 12:03 AM, Christoph Hellwig wrote:
On Wed, Nov 20, 2019 at 11:13:32PM -0800, 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.

Any reason for the spurious underscore in the function name?

argghh, I just fixed that, but applied the fix to the wrong patch! So now
patch 17 ("mm/gup: track FOLL_PIN pages") is improperly renaming it, instead
of this patch naming it correctly in the first place. Will fix.


Otherwise this looks fine and might be a worthwhile cleanup to feed
Andrew for 5.5 independent of the gut of the changes.

Reviewed-by: Christoph Hellwig <hch@xxxxxx>


Thanks for the reviews! Say, it sounds like your view here is that this
series should be targeted at 5.6 (not 5.5), is that what you have in mind?
And get the preparatory patches (1-9, and maybe even 10-16) into 5.5?

thanks,
--
John Hubbard
NVIDIA