[PATCH 2/2] get_user_pages: minor code tweaks

[PATCH 2/2] get_user_pages: minor code tweaks

[Oleg Nesterov]

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 <oleg@tv-sign.ru>

--- 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);

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

[Oleg Nesterov]

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.

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

[Nick Piggin]

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.