Re: [PATCH 2/4] mm: gup: add get_user_pages_locked and get_user_pages_unlocked

From: Peter Feiner
Date: Wed Oct 01 2014 - 11:52:08 EST


On Wed, Oct 01, 2014 at 10:56:35AM +0200, Andrea Arcangeli wrote:
> +static inline long __get_user_pages_locked(struct task_struct *tsk,
> + struct mm_struct *mm,
> + unsigned long start,
> + unsigned long nr_pages,
> + int write, int force,
> + struct page **pages,
> + struct vm_area_struct **vmas,
> + int *locked,
> + bool notify_drop)
> +{
> + int flags = FOLL_TOUCH;
> + long ret, pages_done;
> + bool lock_dropped;
> +
> + if (locked) {
> + /* if VM_FAULT_RETRY can be returned, vmas become invalid */
> + BUG_ON(vmas);
> + /* check caller initialized locked */
> + BUG_ON(*locked != 1);
> + }
> +
> + if (pages)
> + flags |= FOLL_GET;
> + if (write)
> + flags |= FOLL_WRITE;
> + if (force)
> + flags |= FOLL_FORCE;
> +
> + pages_done = 0;
> + lock_dropped = false;
> + for (;;) {
> + ret = __get_user_pages(tsk, mm, start, nr_pages, flags, pages,
> + vmas, locked);
> + if (!locked)
> + /* VM_FAULT_RETRY couldn't trigger, bypass */
> + return ret;
> +
> + /* VM_FAULT_RETRY cannot return errors */
> + if (!*locked) {
> + BUG_ON(ret < 0);
> + BUG_ON(nr_pages == 1 && ret);

If I understand correctly, this second BUG_ON is asserting that when
__get_user_pages is asked for a single page and it is successfully gets the
page, then it shouldn't have dropped the mmap_sem. If that's the case, then
you could generalize this assertion to

BUG_ON(nr_pages == ret);

Otherwise, looks good!

Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/