lkml.org 
[lkml]   [2004]   [Nov]   [17]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
Patches in this message
/
DateTue, 16 Nov 2004 22:33:38 -0800
FromAndrew Morton <>
SubjectRe: loops in get_user_pages() for VM_IO
"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.)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:08    [from the cache]
©2003-2008