Messages in this thread Patch in this message | | | From | Michal Nazarewicz <> | Subject | Re: [PATCH 3/3] mm: fix return value in __alloc_contig_migrate_range() | Date | Mon, 16 Jul 2012 19:40:56 +0200 |
| |
Joonsoo Kim <js1304@gmail.com> writes:
> migrate_pages() would return positive value in some failure case, > so 'ret > 0 ? 0 : ret' may be wrong. > This fix it and remove one dead statement. > > Signed-off-by: Joonsoo Kim <js1304@gmail.com> > Cc: Michal Nazarewicz <mina86@mina86.com> > Cc: Marek Szyprowski <m.szyprowski@samsung.com> > Cc: Minchan Kim <minchan@kernel.org> > Cc: Christoph Lameter <cl@linux.com>
Have you actually encountered this problem? If migrate_pages() fails with a positive value, the code that you are removing kicks in and -EBUSY is assigned to ret (now that I look at it, I think that in the current code the "return ret > 0 ? 0 : ret;" statement could be reduced to "return ret;"). Your code seems to be cleaner, but the commit message does not look accurate to me.
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 4403009..02d4519 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -5673,7 +5673,6 @@ static int __alloc_contig_migrate_range(unsigned long start, unsigned long end) > } > tries = 0; > } else if (++tries == 5) { > - ret = ret < 0 ? ret : -EBUSY; > break; > } > > @@ -5683,7 +5682,7 @@ static int __alloc_contig_migrate_range(unsigned long start, unsigned long end) > } > > putback_lru_pages(&cc.migratepages); > - return ret > 0 ? 0 : ret; > + return ret <= 0 ? ret : -EBUSY; > } > > /*
-- Best regards, _ _ .o. | Liege of Serenly Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michal "mina86" Nazarewicz (o o) ooo +-<mina86-mina86.com>-<jid:mina86-jabber.org>--ooO--(_)--Ooo-- [unhandled content-type:application/pgp-signature] | |