2008-06-21 13:39:16

by Oleg Nesterov

Subject: [PATCH 2/2] get_user_pages: minor code tweaks

No functional changes.

1. Change the code from

if (len <= 0)
return;
do {
} while (len);

to
for (; len > 0; ) {
}

(I wonder what is the right behavior when len < 0. Currently we hide the
caller's error and return 0. Perhaps it is better to test "len != 0" or
turn this argument into "unsigned int").

2. Move "foll_flags |= FOLL_WRITE;" from the inner loop up.

Signed-off-by: Oleg Nesterov <[email protected]>

--- 26-rc2/mm/memory.c~2_GUP_CLEANUP 2008-06-21 15:56:58.000000000 +0400
+++ 26-rc2/mm/memory.c 2008-06-21 17:26:06.000000000 +0400
@@ -1035,17 +1035,14 @@ int get_user_pages(struct task_struct *t
int i;
unsigned int vm_flags;

- if (len <= 0)
- return 0;
- /*
+ /*
* Require read or write permissions.
* If 'force' is set, we only require the "MAY" flags.
*/
vm_flags = write ? (VM_WRITE | VM_MAYWRITE) : (VM_READ | VM_MAYREAD);
vm_flags &= force ? (VM_MAYREAD | VM_MAYWRITE) : (VM_READ | VM_WRITE);
- i = 0;

- do {
+ for (i = 0; len > 0; ) {
struct vm_area_struct *vma;
unsigned int foll_flags;

@@ -1102,8 +1099,10 @@ int get_user_pages(struct task_struct *t
foll_flags = FOLL_TOUCH;
if (pages)
foll_flags |= FOLL_GET;
- if (!write && !(vma->vm_flags & VM_LOCKED) &&
- (!vma->vm_ops || !vma->vm_ops->fault))
+ if (write)
+ foll_flags |= FOLL_WRITE;
+ else if (!(vma->vm_flags & VM_LOCKED) &&
+ (!vma->vm_ops || !vma->vm_ops->fault))
foll_flags |= FOLL_ANON;

do {
@@ -1117,9 +1116,6 @@ int get_user_pages(struct task_struct *t
if (unlikely(test_tsk_thread_flag(tsk, TIF_MEMDIE)))
return i ? i : -ENOMEM;

- if (write)
- foll_flags |= FOLL_WRITE;
-
cond_resched();
while (!(page = follow_page(vma, start, foll_flags))) {
int ret;
@@ -1163,7 +1159,7 @@ int get_user_pages(struct task_struct *t
start += PAGE_SIZE;
len--;
} while (len && start < vma->vm_end);
- } while (len);
+ }
return i;
}
EXPORT_SYMBOL(get_user_pages);

2008-06-21 14:05:32

by Oleg Nesterov

Subject: Re: [PATCH 2/2] get_user_pages: minor code tweaks

On 06/21, Oleg Nesterov wrote:
>
> No functional changes.

Heh.

> 2. Move "foll_flags |= FOLL_WRITE;" from the inner loop up.

Please ignore this patch, it is wrong. FOLL_WRITE can be cleared after
handle_mm_fault().

Oleg.

2008-06-23 02:44:56

by Nick Piggin

Subject: Re: [PATCH 2/2] get_user_pages: minor code tweaks

On Sat, Jun 21, 2008 at 06:07:41PM +0400, Oleg Nesterov wrote:
> On 06/21, Oleg Nesterov wrote:
> >
> > No functional changes.
>
> Heh.
>
> > 2. Move "foll_flags |= FOLL_WRITE;" from the inner loop up.
>
> Please ignore this patch, it is wrong. FOLL_WRITE can be cleared after
> handle_mm_fault().

Patch 1/2 is good, though. Please get it upstream.