On Wed, Oct 1, 2014 at 8:51 AM, Peter Feiner <pfeiner@xxxxxxxxxx> wrote:
> 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);
Even more strict:
BUG_ON(ret >= nr_pages);
Reviewed-by: Andres Lagar-Cavilla <andreslc@xxxxxxxxxx>
>
> Otherwise, looks good!
>
> Peter
--
Andres Lagar-Cavilla | Google Kernel Team | andreslc@xxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
