migrate_vmas() should check "vma" not "vma->vm_next" for for-loop condition.
Signed-off-by: Daisuke Nishimura <[email protected]>
---
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)
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?
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
On Wed, 28 Jan 2009 17:55:12 +0100, Johannes Weiner <[email protected]> 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
On Thu, 29 Jan 2009 10:16:23 +0900 Daisuke Nishimura <[email protected]> wrote:
> On Wed, 28 Jan 2009 17:55:12 +0100, Johannes Weiner <[email protected]> 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?
On Thu, 29 Jan 2009 00:18:49 -0800, Andrew Morton <[email protected]> wrote:
> On Thu, 29 Jan 2009 10:16:23 +0900 Daisuke Nishimura <[email protected]> wrote:
>
> > On Wed, 28 Jan 2009 17:55:12 +0100, Johannes Weiner <[email protected]> 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.