Re: loops in get_user_pages() for VM_IO

Andrew Morton
2004-11-17T06:34:21Z

"David S. Miller" <davem@davemloft.net> wrote:
>
> In any event, it is still an open question whether get_user_pages()
> and thus make_pages_present() is meant to be able to handle
> VM_IO areas.

It doesn't make a lot of sense. Andrea says that the only caller of
get_user_pages() which uses a null `pages' arg is mlock(), and mlock of a
VM_IO region is currently causing hangs, and proposes this change:

--- sles/mm/memory.c.~1~ 2004-11-12 12:30:25.000000000 +0100
+++ sles/mm/memory.c 2004-11-16 17:58:02.752131952 +0100
@@ -753,7 +753,7 @@ int get_user_pages(struct task_struct *t
continue;
}

- if (!vma || (pages && (vma->vm_flags & VM_IO))
+ if (!vma || (vma->vm_flags & VM_IO)
|| !(flags & vma->vm_flags))
return i ? : -EFAULT;

which should fix up the sbuslib.c problem.

Although I suspect this change will make mlockall() return -EFAULT or a
short result to userspace if the caller had a VM_IO region mapped, which
doesn't seem appropriate. So perhaps we should silently bale out in the
VM_IO case.

Or, better, simply advance over the VM_IO vma and onto the next one?

--- 25/mm/memory.c~get_user_pages-skip-VM_IO 2004-11-16 22:24:34.470017896 -0800
+++ 25-akpm/mm/memory.c 2004-11-16 22:32:04.890543568 -0800
@@ -761,9 +761,27 @@ int get_user_pages(struct task_struct *t
continue;
}

- if (!vma || (pages && (vma->vm_flags & VM_IO))
- || !(flags & vma->vm_flags))
- return i ? : -EFAULT;
+ if (!vma || !(flags & vma->vm_flags))
+ return i ? i : -EFAULT;
+
+ if (vma->vm_flags & VM_IO) {
+ if (pages) {
+ /*
+ * No, you cannot gather pageframes from VM_IO
+ * regions
+ */
+ return i ? i : -EFAULT;
+ }
+ /*
+ * OK, someone is simply trying to fault in some pages
+ * and they encountered a VM_IO region. mlockall()
+ * can do this. Simply skip the vma
+ */
+ start = vma->vm_end;
+ len -= (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
+ i += (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
+ continue;
+ }

if (is_vm_hugetlb_page(vma)) {
i = follow_hugetlb_page(mm, vma, pages, vmas,
_

(I've probably screwed something up there.)