On Fri 31-01-20 19:40:19, John Hubbard wrote:
> An upcoming patch requires reusing the implementation of
> get_user_pages_remote(). Split up get_user_pages_remote() into an outer
> routine that checks flags, and an implementation routine that will be
> reused. This makes subsequent changes much easier to understand.
>
> There should be no change in behavior due to this patch.
>
> Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
Looks good to me. You can add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> mm/gup.c | 56 +++++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 33 insertions(+), 23 deletions(-)
>
> diff --git a/mm/gup.c b/mm/gup.c
> index e13f4d211475..228aa7059018 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -1557,6 +1557,37 @@ static __always_inline long __gup_longterm_locked(struct task_struct *tsk,
> }
> #endif /* CONFIG_FS_DAX || CONFIG_CMA */
>
> +#ifdef CONFIG_MMU
> +static long __get_user_pages_remote(struct task_struct *tsk,
> + struct mm_struct *mm,
> + unsigned long start, unsigned long nr_pages,
> + unsigned int gup_flags, struct page **pages,
> + struct vm_area_struct **vmas, int *locked)
> +{
> + /*
> + * Parts of FOLL_LONGTERM behavior are incompatible with
> + * FAULT_FLAG_ALLOW_RETRY because of the FS DAX check requirement on
> + * vmas. However, this only comes up if locked is set, and there are
> + * callers that do request FOLL_LONGTERM, but do not set locked. So,
> + * allow what we can.
> + */
> + if (gup_flags & FOLL_LONGTERM) {
> + if (WARN_ON_ONCE(locked))
> + return -EINVAL;
> + /*
> + * This will check the vmas (even if our vmas arg is NULL)
> + * and return -ENOTSUPP if DAX isn't allowed in this case:
> + */
> + return __gup_longterm_locked(tsk, mm, start, nr_pages, pages,
> + vmas, gup_flags | FOLL_TOUCH |
> + FOLL_REMOTE);
> + }
> +
> + return __get_user_pages_locked(tsk, mm, start, nr_pages, pages, vmas,
> + locked,
> + gup_flags | FOLL_TOUCH | FOLL_REMOTE);
> +}
> +
> /*
> * get_user_pages_remote() - pin user pages in memory
> * @tsk: the task_struct to use for page fault accounting, or
> @@ -1619,7 +1650,6 @@ static __always_inline long __gup_longterm_locked(struct task_struct *tsk,
> * should use get_user_pages because it cannot pass
> * FAULT_FLAG_ALLOW_RETRY to handle_mm_fault.
> */
> -#ifdef CONFIG_MMU
> long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
> unsigned long start, unsigned long nr_pages,
> unsigned int gup_flags, struct page **pages,
> @@ -1632,28 +1662,8 @@ long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
> if (WARN_ON_ONCE(gup_flags & FOLL_PIN))
> return -EINVAL;
>
> - /*
> - * Parts of FOLL_LONGTERM behavior are incompatible with
> - * FAULT_FLAG_ALLOW_RETRY because of the FS DAX check requirement on
> - * vmas. However, this only comes up if locked is set, and there are
> - * callers that do request FOLL_LONGTERM, but do not set locked. So,
> - * allow what we can.
> - */
> - if (gup_flags & FOLL_LONGTERM) {
> - if (WARN_ON_ONCE(locked))
> - return -EINVAL;
> - /*
> - * This will check the vmas (even if our vmas arg is NULL)
> - * and return -ENOTSUPP if DAX isn't allowed in this case:
> - */
> - return __gup_longterm_locked(tsk, mm, start, nr_pages, pages,
> - vmas, gup_flags | FOLL_TOUCH |
> - FOLL_REMOTE);
> - }
> -
> - return __get_user_pages_locked(tsk, mm, start, nr_pages, pages, vmas,
> - locked,
> - gup_flags | FOLL_TOUCH | FOLL_REMOTE);
> + return __get_user_pages_remote(tsk, mm, start, nr_pages, gup_flags,
> + pages, vmas, locked);
> }
> EXPORT_SYMBOL(get_user_pages_remote);
>
> --
> 2.25.0
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
