[PATCH 1/2] ksm: clean up page_trans_compound_anon_split

[PATCH 1/2] ksm: clean up page_trans_compound_anon_split

[Bob Liu]

Signed-off-by: Bob Liu <lliubbo@gmail.com>
---
 mm/ksm.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/mm/ksm.c b/mm/ksm.c
index 1925ffb..8e10786 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -817,7 +817,7 @@ out:
 
 static int page_trans_compound_anon_split(struct page *page)
 {
-	int ret = 0;
+	int ret = 1;
 	struct page *transhuge_head = page_trans_compound_anon(page);
 	if (transhuge_head) {
 		/* Get the reference on the head to split it. */
@@ -828,16 +828,8 @@ static int page_trans_compound_anon_split(struct page *page)
 			 */
 			if (PageAnon(transhuge_head))
 				ret = split_huge_page(transhuge_head);
-			else
-				/*
-				 * Retry later if split_huge_page run
-				 * from under us.
-				 */
-				ret = 1;
 			put_page(transhuge_head);
-		} else
-			/* Retry later if split_huge_page run from under us. */
-			ret = 1;
+		}
 	}
 	return ret;
 }
-- 
1.7.0.4


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/2] ksm: cleanup: introduce ksm_check_mm()

[Bob Liu]

There are multi place do the same check.

Signed-off-by: Bob Liu <lliubbo@gmail.com>
---
 mm/ksm.c |   35 ++++++++++++++++++-----------------
 1 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/mm/ksm.c b/mm/ksm.c
index 8e10786..33175af 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -375,11 +375,24 @@ static int break_ksm(struct vm_area_struct *vma, unsigned long addr)
 	return (ret & VM_FAULT_OOM) ? -ENOMEM : 0;
 }
 
+static int ksm_check_mm(struct mm_struct *mm, struct vm_area_struct *vma,
+		unsigned long addr)
+{
+	if (ksm_test_exit(mm))
+		return 0;
+	vma = find_vma(mm, addr);
+	if (!vma || vma->vm_start > addr)
+		return 0;
+	if (!(vma->vm_flags & VM_MERGEABLE) || !vma->anon_vma)
+		return 0;
+	return 1;
+}
+
 static void break_cow(struct rmap_item *rmap_item)
 {
 	struct mm_struct *mm = rmap_item->mm;
 	unsigned long addr = rmap_item->address;
-	struct vm_area_struct *vma;
+	struct vm_area_struct *vma = NULL;
 
 	/*
 	 * It is not an accident that whenever we want to break COW
@@ -388,15 +401,8 @@ static void break_cow(struct rmap_item *rmap_item)
 	put_anon_vma(rmap_item->anon_vma);
 
 	down_read(&mm->mmap_sem);
-	if (ksm_test_exit(mm))
-		goto out;
-	vma = find_vma(mm, addr);
-	if (!vma || vma->vm_start > addr)
-		goto out;
-	if (!(vma->vm_flags & VM_MERGEABLE) || !vma->anon_vma)
-		goto out;
-	break_ksm(vma, addr);
-out:
+	if (ksm_check_mm(mm, vma, addr))
+		break_ksm(vma, addr);
 	up_read(&mm->mmap_sem);
 }
 
@@ -418,16 +424,11 @@ static struct page *get_mergeable_page(struct rmap_item *rmap_item)
 {
 	struct mm_struct *mm = rmap_item->mm;
 	unsigned long addr = rmap_item->address;
-	struct vm_area_struct *vma;
+	struct vm_area_struct *vma = NULL;
 	struct page *page;
 
 	down_read(&mm->mmap_sem);
-	if (ksm_test_exit(mm))
-		goto out;
-	vma = find_vma(mm, addr);
-	if (!vma || vma->vm_start > addr)
-		goto out;
-	if (!(vma->vm_flags & VM_MERGEABLE) || !vma->anon_vma)
+	if (!ksm_check_mm(mm, vma, addr))
 		goto out;
 
 	page = follow_page(vma, addr, FOLL_GET);
-- 
1.7.0.4


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/2] ksm: cleanup: introduce ksm_check_mm()

[Andrew Morton]

On Thu, 1 Mar 2012 17:32:54 +0800
Bob Liu <lliubbo@gmail.com> wrote:

> +static int ksm_check_mm(struct mm_struct *mm, struct vm_area_struct *vma,
> +		unsigned long addr)
> +{
> +	if (ksm_test_exit(mm))
> +		return 0;
> +	vma = find_vma(mm, addr);
> +	if (!vma || vma->vm_start > addr)
> +		return 0;
> +	if (!(vma->vm_flags & VM_MERGEABLE) || !vma->anon_vma)
> +		return 0;
> +	return 1;
> +}

Can we please think of a suitable name for this check, other than
"check"?  IOW, give the function a meaningful name which describes what
it is checking?

And it's not checking the mm, is it?  It is checking the address: to
see whether it lies within a mergeable anon vma.

So maybe

--- a/mm/ksm.c~ksm-cleanup-introduce-ksm_check_mm-fix
+++ a/mm/ksm.c
@@ -375,17 +375,17 @@ static int break_ksm(struct vm_area_stru
 	return (ret & VM_FAULT_OOM) ? -ENOMEM : 0;
 }
 
-static int ksm_check_mm(struct mm_struct *mm, struct vm_area_struct *vma,
-		unsigned long addr)
+static bool in_mergeable_anon_vma(struct mm_struct *mm,
+				 struct vm_area_struct *vma, unsigned long addr)
 {
 	if (ksm_test_exit(mm))
-		return 0;
+		return false;
 	vma = find_vma(mm, addr);
 	if (!vma || vma->vm_start > addr)
-		return 0;
+		return false;
 	if (!(vma->vm_flags & VM_MERGEABLE) || !vma->anon_vma)
-		return 0;
-	return 1;
+		return false;
+	return true;
 }
 
 static void break_cow(struct rmap_item *rmap_item)
@@ -401,7 +401,7 @@ static void break_cow(struct rmap_item *
 	put_anon_vma(rmap_item->anon_vma);
 
 	down_read(&mm->mmap_sem);
-	if (ksm_check_mm(mm, vma, addr))
+	if (in_mergeable_anon_vma(mm, vma, addr))
 		break_ksm(vma, addr);
 	up_read(&mm->mmap_sem);
 }
@@ -428,7 +428,7 @@ static struct page *get_mergeable_page(s
 	struct page *page;
 
 	down_read(&mm->mmap_sem);
-	if (!ksm_check_mm(mm, vma, addr))
+	if (!in_mergeable_anon_vma(mm, vma, addr))
 		goto out;
 
 	page = follow_page(vma, addr, FOLL_GET);
_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/2] ksm: cleanup: introduce ksm_check_mm()

[Bob Liu]

Hi Andrew,

On Fri, Mar 2, 2012 at 4:51 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Thu, 1 Mar 2012 17:32:54 +0800
> Bob Liu <lliubbo@gmail.com> wrote:
>
>> +static int ksm_check_mm(struct mm_struct *mm, struct vm_area_struct *vma,
>> +             unsigned long addr)
>> +{
>> +     if (ksm_test_exit(mm))
>> +             return 0;
>> +     vma = find_vma(mm, addr);
>> +     if (!vma || vma->vm_start > addr)
>> +             return 0;
>> +     if (!(vma->vm_flags & VM_MERGEABLE) || !vma->anon_vma)
>> +             return 0;
>> +     return 1;
>> +}
>
> Can we please think of a suitable name for this check, other than
> "check"?  IOW, give the function a meaningful name which describes what
> it is checking?
>
> And it's not checking the mm, is it?  It is checking the address: to
> see whether it lies within a mergeable anon vma.
>
> So maybe
>
> --- a/mm/ksm.c~ksm-cleanup-introduce-ksm_check_mm-fix
> +++ a/mm/ksm.c
> @@ -375,17 +375,17 @@ static int break_ksm(struct vm_area_stru
>        return (ret & VM_FAULT_OOM) ? -ENOMEM : 0;
>  }
>
> -static int ksm_check_mm(struct mm_struct *mm, struct vm_area_struct *vma,
> -               unsigned long addr)
> +static bool in_mergeable_anon_vma(struct mm_struct *mm,
> +                                struct vm_area_struct *vma, unsigned long addr)
>  {
>        if (ksm_test_exit(mm))
> -               return 0;
> +               return false;
>        vma = find_vma(mm, addr);
>        if (!vma || vma->vm_start > addr)
> -               return 0;
> +               return false;
>        if (!(vma->vm_flags & VM_MERGEABLE) || !vma->anon_vma)
> -               return 0;
> -       return 1;
> +               return false;
> +       return true;
>  }
>
>  static void break_cow(struct rmap_item *rmap_item)
> @@ -401,7 +401,7 @@ static void break_cow(struct rmap_item *
>        put_anon_vma(rmap_item->anon_vma);
>
>        down_read(&mm->mmap_sem);
> -       if (ksm_check_mm(mm, vma, addr))
> +       if (in_mergeable_anon_vma(mm, vma, addr))
>                break_ksm(vma, addr);
>        up_read(&mm->mmap_sem);
>  }
> @@ -428,7 +428,7 @@ static struct page *get_mergeable_page(s
>        struct page *page;
>
>        down_read(&mm->mmap_sem);
> -       if (!ksm_check_mm(mm, vma, addr))
> +       if (!in_mergeable_anon_vma(mm, vma, addr))
>                goto out;
>
>        page = follow_page(vma, addr, FOLL_GET);
> _
>

Yeah, It looks much better than mine, Thanks!

-- 
Regards,
--Bob

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/2] ksm: cleanup: introduce ksm_check_mm()

[Hugh Dickins]

On Thu, 1 Mar 2012, Bob Liu wrote:

> There are multi place do the same check.
> 
> Signed-off-by: Bob Liu <lliubbo@gmail.com>

Combining two sets of alike changes into one function, fair enough.

But am I imagining it, or is vma going to be NULL in the callers forever
after, and KSM badly broken?  And for what is vma passed to the function?

I think you want to redo this with

static struct vm_area_struct *find_mergeable_vma(struct mm_struct *mm,
                                                 unsigned long addr);

The anon_vma aspect that Andrew latched on to: that's rather a red
herring.  It's not so much looking for an anon_vma, it just knows that
if an anon_vma has not (yet) been instantiated there, then it's a
waste of time to look for an Anon or KSM page in that area.

Hugh

> ---
>  mm/ksm.c |   35 ++++++++++++++++++-----------------
>  1 files changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/mm/ksm.c b/mm/ksm.c
> index 8e10786..33175af 100644
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
> @@ -375,11 +375,24 @@ static int break_ksm(struct vm_area_struct *vma, unsigned long addr)
>  	return (ret & VM_FAULT_OOM) ? -ENOMEM : 0;
>  }
>  
> +static int ksm_check_mm(struct mm_struct *mm, struct vm_area_struct *vma,
> +		unsigned long addr)
> +{
> +	if (ksm_test_exit(mm))
> +		return 0;
> +	vma = find_vma(mm, addr);
> +	if (!vma || vma->vm_start > addr)
> +		return 0;
> +	if (!(vma->vm_flags & VM_MERGEABLE) || !vma->anon_vma)
> +		return 0;
> +	return 1;
> +}
> +
>  static void break_cow(struct rmap_item *rmap_item)
>  {
>  	struct mm_struct *mm = rmap_item->mm;
>  	unsigned long addr = rmap_item->address;
> -	struct vm_area_struct *vma;
> +	struct vm_area_struct *vma = NULL;
>  
>  	/*
>  	 * It is not an accident that whenever we want to break COW
> @@ -388,15 +401,8 @@ static void break_cow(struct rmap_item *rmap_item)
>  	put_anon_vma(rmap_item->anon_vma);
>  
>  	down_read(&mm->mmap_sem);
> -	if (ksm_test_exit(mm))
> -		goto out;
> -	vma = find_vma(mm, addr);
> -	if (!vma || vma->vm_start > addr)
> -		goto out;
> -	if (!(vma->vm_flags & VM_MERGEABLE) || !vma->anon_vma)
> -		goto out;
> -	break_ksm(vma, addr);
> -out:
> +	if (ksm_check_mm(mm, vma, addr))
> +		break_ksm(vma, addr);
>  	up_read(&mm->mmap_sem);
>  }
>  
> @@ -418,16 +424,11 @@ static struct page *get_mergeable_page(struct rmap_item *rmap_item)
>  {
>  	struct mm_struct *mm = rmap_item->mm;
>  	unsigned long addr = rmap_item->address;
> -	struct vm_area_struct *vma;
> +	struct vm_area_struct *vma = NULL;
>  	struct page *page;
>  
>  	down_read(&mm->mmap_sem);
> -	if (ksm_test_exit(mm))
> -		goto out;
> -	vma = find_vma(mm, addr);
> -	if (!vma || vma->vm_start > addr)
> -		goto out;
> -	if (!(vma->vm_flags & VM_MERGEABLE) || !vma->anon_vma)
> +	if (!ksm_check_mm(mm, vma, addr))
>  		goto out;
>  
>  	page = follow_page(vma, addr, FOLL_GET);
> -- 
> 1.7.0.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] ksm: clean up page_trans_compound_anon_split

[Hugh Dickins]

On Thu, 1 Mar 2012, Bob Liu wrote:

> Signed-off-by: Bob Liu <lliubbo@gmail.com>

I agree it looks very much nicer: a patch on these lines would be good.

But you've lost the comment about a return of 1 meaning "Retry later if
split_huge_page run from under us", which I think was a helpful comment.

And you've not commented on the functional change which you made:
if page_trans_compound_anon() returns NULL, then _split() now returns
1 where before it returned 0.  I suspect that's a reasonable change
in a rare case, and better left simple as you have it, than slavishly
reproduce the earlier behaviour; but I'd like to have an Ack from the
author before we commit your modification.

But you didn't Cc Andrea whose code this is, and who understands THP
and its races better than anybody: now Cc'ed.

Hugh

> ---
>  mm/ksm.c |   12 ++----------
>  1 files changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/ksm.c b/mm/ksm.c
> index 1925ffb..8e10786 100644
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
> @@ -817,7 +817,7 @@ out:
>  
>  static int page_trans_compound_anon_split(struct page *page)
>  {
> -	int ret = 0;
> +	int ret = 1;
>  	struct page *transhuge_head = page_trans_compound_anon(page);
>  	if (transhuge_head) {
>  		/* Get the reference on the head to split it. */
> @@ -828,16 +828,8 @@ static int page_trans_compound_anon_split(struct page *page)
>  			 */
>  			if (PageAnon(transhuge_head))
>  				ret = split_huge_page(transhuge_head);
> -			else
> -				/*
> -				 * Retry later if split_huge_page run
> -				 * from under us.
> -				 */
> -				ret = 1;
>  			put_page(transhuge_head);
> -		} else
> -			/* Retry later if split_huge_page run from under us. */
> -			ret = 1;
> +		}
>  	}
>  	return ret;
>  }
> -- 
> 1.7.0.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] ksm: clean up page_trans_compound_anon_split

[Andrea Arcangeli]

On Tue, Mar 06, 2012 at 03:28:43PM -0800, Hugh Dickins wrote:
> On Thu, 1 Mar 2012, Bob Liu wrote:
> 
> > Signed-off-by: Bob Liu <lliubbo@gmail.com>
> 
> I agree it looks very much nicer: a patch on these lines would be good.
> 
> But you've lost the comment about a return of 1 meaning "Retry later if
> split_huge_page run from under us", which I think was a helpful comment.
> 
> And you've not commented on the functional change which you made:
> if page_trans_compound_anon() returns NULL, then _split() now returns
> 1 where before it returned 0.  I suspect that's a reasonable change
> in a rare case, and better left simple as you have it, than slavishly
> reproduce the earlier behaviour; but I'd like to have an Ack from the
> author before we commit your modification.

Yes, it's not a "noop", I just read the patch through the -mm flow a
few sec after reading the above.

> But you didn't Cc Andrea whose code this is, and who understands THP
> and its races better than anybody: now Cc'ed.

Thanks for CCing me. Returning 1 when page_trans_compound_anon returns
NULL, should still be safe, because 1 triggers the bail out path, so
it won't harm. It should be fully equivalent too because it would bail
out later in the PageAnon check if page_trans_compound_anon returned 0
(the function was invoked only on compound pages in the first place).

So it looks fine.

Andrea

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] ksm: clean up page_trans_compound_anon_split

[Andrea Arcangeli]

On Wed, Mar 07, 2012 at 01:11:48AM +0100, Andrea Arcangeli wrote:
> (the function was invoked only on compound pages in the first place).

BTW, most certainly I did at some point this change:

-	if (page_trans_compound_anon_split(page))
+	if (PageTransCompound(page) && page_trans_compound_anon_split(page))

Before doing this change, the "cleaned up" version would have been
broken.

The original idea was to return 1 only in real error condition when a
THP splitting failure was encountered. So it had to be neutral and not
error out if split_huge_page wasn't needed.

In short the cleaned up version of page_trans_compound_anon_split is a
bit less generic but it being a static and only used here I don't mind
too much.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] ksm: clean up page_trans_compound_anon_split

[Bob Liu]

Hi Andrea,

On Wed, Mar 7, 2012 at 8:26 AM, Andrea Arcangeli <aarcange@redhat.com> wrote:
> On Wed, Mar 07, 2012 at 01:11:48AM +0100, Andrea Arcangeli wrote:
>> (the function was invoked only on compound pages in the first place).
>
> BTW, most certainly I did at some point this change:
>
> -       if (page_trans_compound_anon_split(page))
> +       if (PageTransCompound(page) && page_trans_compound_anon_split(page))
>
> Before doing this change, the "cleaned up" version would have been
> broken.
>

I think this patch may still break the origin meaning.

In case PageTransCompound(page) but !PageAnon(head) after this cleanup,
page_trans_compound_anon_split(page) will return 1 instead of 0 which
will cause following
PageAnon check to a compounded page.

So please just ignore this cleanup. Sorry for my noise.


Hugh,  Thank you for your review also.

-- 
Regards,
--Bob

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] ksm: clean up page_trans_compound_anon_split

[Andrea Arcangeli]

Hi Bob,

On Wed, Mar 07, 2012 at 06:39:12PM +0800, Bob Liu wrote:
> Hi Andrea,
> 
> I think this patch may still break the origin meaning.
> 
> In case PageTransCompound(page) but !PageAnon(head) after this cleanup,
> page_trans_compound_anon_split(page) will return 1 instead of 0 which
> will cause following
> PageAnon check to a compounded page.

It won't check PageAnon if you return 1 in that case. Returning 1 will
bail out immediately so it's always safe (simply it would become
dangerous to call the page_trans_compound_anon_split on a page that
wasn't PageTransCompound after the cleanup). The only downside is not
a runtime one but a theoretical one: it makes the function less
generic as it errors out even for regular pages now so it must only be
called on compound pages after the cleanup (but it was already called
only for compound pages so I couldn't argue against the cleanup, but
hey I also feel like the original version was more generic).

> 
> So please just ignore this cleanup. Sorry for my noise.

No problem, ok to drop it if you also like the current semantics more.

Thanks,
Andrea

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] ksm: clean up page_trans_compound_anon_split

[Hugh Dickins]

On Wed, 7 Mar 2012, Andrea Arcangeli wrote:
> On Tue, Mar 06, 2012 at 03:28:43PM -0800, Hugh Dickins wrote:
> > On Thu, 1 Mar 2012, Bob Liu wrote:
> > 
> > > Signed-off-by: Bob Liu <lliubbo@gmail.com>
> > 
> > I agree it looks very much nicer: a patch on these lines would be good.
> > 
> > But you've lost the comment about a return of 1 meaning "Retry later if
> > split_huge_page run from under us", which I think was a helpful comment.
> > 
> > And you've not commented on the functional change which you made:
> > if page_trans_compound_anon() returns NULL, then _split() now returns
> > 1 where before it returned 0.  I suspect that's a reasonable change
> > in a rare case, and better left simple as you have it, than slavishly
> > reproduce the earlier behaviour; but I'd like to have an Ack from the
> > author before we commit your modification.
> 
> Yes, it's not a "noop", I just read the patch through the -mm flow a
> few sec after reading the above.
> 
> > But you didn't Cc Andrea whose code this is, and who understands THP
> > and its races better than anybody: now Cc'ed.
> 
> Thanks for CCing me. Returning 1 when page_trans_compound_anon returns
> NULL, should still be safe, because 1 triggers the bail out path, so
> it won't harm. It should be fully equivalent too because it would bail
> out later in the PageAnon check if page_trans_compound_anon returned 0
> (the function was invoked only on compound pages in the first place).
> 
> So it looks fine.

Thanks, Andrea, that's good.  So Bob, please resubmit with comment on
return value 1 reinstated, and in the commit description explain how
the slight change in operation is benign.

Thanks,
Hugh

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>