[PATCH] migration: migrate_vmas should check "vma"

[PATCH] migration: migrate_vmas should check "vma"

[Daisuke Nishimura]

migrate_vmas() should check "vma" not "vma->vm_next" for for-loop condition.

Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
---
 mm/migrate.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index 2bb4e1d..a9eff3f 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1129,7 +1129,7 @@ int migrate_vmas(struct mm_struct *mm, const nodemask_t *to,
  	struct vm_area_struct *vma;
  	int err = 0;
 
- 	for(vma = mm->mmap; vma->vm_next && !err; vma = vma->vm_next) {
+	for (vma = mm->mmap; vma && !err; vma = vma->vm_next) {
  		if (vma->vm_ops && vma->vm_ops->migrate) {
  			err = vma->vm_ops->migrate(vma, to, from, flags);
  			if (err)

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] migration: migrate_vmas should check "vma"

[Christoph Lameter]

On Wed, 28 Jan 2009, Daisuke Nishimura wrote:

> migrate_vmas() should check "vma" not "vma->vm_next" for for-loop condition.

The loop condition is checked before vma = vma->vm_next. So the last
iteration of the loop will now be run with vma = NULL?

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] migration: migrate_vmas should check "vma"

[Johannes Weiner]

On Wed, Jan 28, 2009 at 11:42:36AM -0500, Christoph Lameter wrote:
> On Wed, 28 Jan 2009, Daisuke Nishimura wrote:
> 
> > migrate_vmas() should check "vma" not "vma->vm_next" for for-loop condition.
> 
> The loop condition is checked before vma = vma->vm_next. So the last
> iteration of the loop will now be run with vma = NULL?

No, the condition is always checked before the body is executed.  The
assignment to vma->vm_next happens at the end of every body.

Try this:

		int a = 2;

		for (puts("init"); puts("cond"), a; puts("next"))
			a--;

	Hannes

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] migration: migrate_vmas should check "vma"

[Daisuke Nishimura]

On Wed, 28 Jan 2009 17:55:12 +0100, Johannes Weiner <hannes@cmpxchg.org> wrote:
> On Wed, Jan 28, 2009 at 11:42:36AM -0500, Christoph Lameter wrote:
> > On Wed, 28 Jan 2009, Daisuke Nishimura wrote:
> > 
> > > migrate_vmas() should check "vma" not "vma->vm_next" for for-loop condition.
> > 
> > The loop condition is checked before vma = vma->vm_next. So the last
> > iteration of the loop will now be run with vma = NULL?
> 
> No, the condition is always checked before the body is executed.  The
> assignment to vma->vm_next happens at the end of every body.
> 
So, I think in current code the loop body is not executed
about the last vma in the list.


Thanks,
Daisuke Nishimura.

> Try this:
> 
> 		int a = 2;
> 
> 		for (puts("init"); puts("cond"), a; puts("next"))
> 			a--;
> 
> 	Hannes

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] migration: migrate_vmas should check "vma"

[Andrew Morton]

On Thu, 29 Jan 2009 10:16:23 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:

> On Wed, 28 Jan 2009 17:55:12 +0100, Johannes Weiner <hannes@cmpxchg.org> wrote:
> > On Wed, Jan 28, 2009 at 11:42:36AM -0500, Christoph Lameter wrote:
> > > On Wed, 28 Jan 2009, Daisuke Nishimura wrote:
> > > 
> > > > migrate_vmas() should check "vma" not "vma->vm_next" for for-loop condition.
> > > 
> > > The loop condition is checked before vma = vma->vm_next. So the last
> > > iteration of the loop will now be run with vma = NULL?
> > 
> > No, the condition is always checked before the body is executed.  The
> > assignment to vma->vm_next happens at the end of every body.
> > 
> So, I think in current code the loop body is not executed
> about the last vma in the list.
> 

Yep.

Is this serious enough to bother fixing in 2.6.29?

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] migration: migrate_vmas should check "vma"

[Daisuke Nishimura]

On Thu, 29 Jan 2009 00:18:49 -0800, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Thu, 29 Jan 2009 10:16:23 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> 
> > On Wed, 28 Jan 2009 17:55:12 +0100, Johannes Weiner <hannes@cmpxchg.org> wrote:
> > > On Wed, Jan 28, 2009 at 11:42:36AM -0500, Christoph Lameter wrote:
> > > > On Wed, 28 Jan 2009, Daisuke Nishimura wrote:
> > > > 
> > > > > migrate_vmas() should check "vma" not "vma->vm_next" for for-loop condition.
> > > > 
> > > > The loop condition is checked before vma = vma->vm_next. So the last
> > > > iteration of the loop will now be run with vma = NULL?
> > > 
> > > No, the condition is always checked before the body is executed.  The
> > > assignment to vma->vm_next happens at the end of every body.
> > > 
> > So, I think in current code the loop body is not executed
> > about the last vma in the list.
> > 
> 
> Yep.
> 
> Is this serious enough to bother fixing in 2.6.29?
IIUC, there is no user of vm_ops->migrate() now, so this bug causes
no practical problems.

I think it's trival and simple enough to go in .29, but I don't have
any objection if you postpone this patch.


Thanks,
Daisuke Nishimura.

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>