On Tue, Jan 17, 2023 at 11:58:34AM -0400, Jason Gunthorpe wrote:
> The GUP family of functions have a complex, but fairly well defined, set
> of invariants for their arguments. Currently these are sprinkled about,
> sometimes in duplicate through many functions.
>
> Internally we don't follow all the invariants that the external interface
> has to follow, so place these checks directly at the exported
> interface. This ensures the internal functions never reach a violated
> invariant.
>
> Remove the duplicated invariant checks.
>
> The end result is to make these functions fully internal:
> __get_user_pages_locked()
> internal_get_user_pages_fast()
> __gup_longterm_locked()
>
> And all the other functions call directly into one of these.
>
> Suggested-by: John Hubbard <jhubbard@xxxxxxxxxx>
> Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> ---
> mm/gup.c | 150 +++++++++++++++++++++++------------------------
> mm/huge_memory.c | 10 ----
> 2 files changed, 75 insertions(+), 85 deletions(-)
>
> diff --git a/mm/gup.c b/mm/gup.c
> index 2c833f862d0354..9e332e3f6ea8e2 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
...
> -static bool is_valid_gup_flags(unsigned int gup_flags)
> +/*
> + * Check that the given flags are valid for the exported gup/pup interface, and
> + * update them with the required flags that the caller must have set.
> + */
> +static bool is_valid_gup_args(struct page **pages, struct vm_area_struct **vmas,
> + int *locked, unsigned int *gup_flags_p,
> + unsigned int to_set)
> {
> + unsigned int gup_flags = *gup_flags_p;
> +
> /*
> - * FOLL_PIN must only be set internally by the pin_user_pages*() APIs,
> - * never directly by the caller, so enforce that with an assertion:
> + * These flags not allowed to be specified externally to the gup
> + * interfaces:
> + * - FOLL_PIN/FOLL_TRIED/FOLL_FAST_ONLY is internal only
^ are?
> + * - FOLL_REMOTE is internal only and used on follow_page()
> */
> - if (WARN_ON_ONCE(gup_flags & FOLL_PIN))
> + if (WARN_ON_ONCE(gup_flags & (FOLL_PIN | FOLL_TRIED |
> + FOLL_REMOTE | FOLL_FAST_ONLY)))
> return false;
> +
> + gup_flags |= to_set;
> +
> + /* FOLL_GET and FOLL_PIN are mutually exclusive. */
> + if (WARN_ON_ONCE((gup_flags & (FOLL_PIN | FOLL_GET)) ==
> + (FOLL_PIN | FOLL_GET)))
> + return false;
> +
--
Sincerely yours,
Mike.
