Messages in this thread Patches in this message | | | Date | Tue, 16 Nov 2004 22:33:38 -0800 | From | Andrew Morton <> | Subject | Re: 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/
| |