Re: [PATCH v12 04/22] mm: devmap: refactor 1-based refcounting for ZONE_DEVICE pages

[Christoph Hellwig]

On Tue, Jan 07, 2020 at 02:45:40PM -0800, John Hubbard wrote:
> An upcoming patch changes and complicates the refcounting and
> especially the "put page" aspects of it. In order to keep
> everything clean, refactor the devmap page release routines:
> 
> * Rename put_devmap_managed_page() to page_is_devmap_managed(),
>   and limit the functionality to "read only": return a bool,
>   with no side effects.
> 
> * Add a new routine, put_devmap_managed_page(), to handle
>   decrementing the refcount for ZONE_DEVICE pages.
> 
> * Change callers (just release_pages() and put_page()) to check
>   page_is_devmap_managed() before calling the new
>   put_devmap_managed_page() routine. This is a performance
>   point: put_page() is a hot path, so we need to avoid non-
>   inline function calls where possible.
> 
> * Rename __put_devmap_managed_page() to free_devmap_managed_page(),
>   and limit the functionality to unconditionally freeing a devmap
>   page.
> 
> This is originally based on a separate patch by Ira Weiny, which
> applied to an early version of the put_user_page() experiments.
> Since then, Jérôme Glisse suggested the refactoring described above.

I'm really not sold on this scheme.  Note that I think it is
particularly bad, but it also doesn't seem any better than what
we had before, and it introduced quite a bit more code.