Prev: bluetooth: btmrvl_sdio: silence warning
Next: Removing dead CONFIG_DIAGNOSTICS
From: Andi Kleen on 5 Jul 2010 05:50
On Fri, Jul 02, 2010 at 02:47:24PM +0900, Naoya Horiguchi wrote:
> This patch introduces pinning the old page during page migration
> to avoid freeing it before we complete copying.
> This race condition can happen for privately mapped or anonymous hugepage.

Patch looks good.
-Andi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Christoph Lameter on 6 Jul 2010 12:00
On Fri, 2 Jul 2010, Naoya Horiguchi wrote:

> This patch introduces pinning the old page during page migration
> to avoid freeing it before we complete copying.

The old page is already pinned due to the reference count that is taken
when the page is put onto the list of pages to be migrated. See
do_move_pages() f.e.

Huge pages use a different scheme?

> This race condition can happen for privately mapped or anonymous hugepage.

It cannot happen unless you come up with your own scheme of managing pages
to be migrated and bypass migrate_pages(). There you should take the
refcount.

> /*
> + * It's reasonable to pin the old page until unmapping and copying
> + * complete, because when the original page is an anonymous hugepage,
> + * it will be freed in try_to_unmap() due to the fact that
> + * all references of anonymous hugepage come from mapcount.
> + * Although in the other cases no problem comes out without pinning,
> + * it looks logically correct to do it.
> + */
> + get_page(page);
> +
> + /*

Its already pinned. Dont do this. migrate_pages() relies on the caller
having pinned the page already.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Naoya Horiguchi on 7 Jul 2010 02:50
Hi,

Thank you for your reviewing.

On Tue, Jul 06, 2010 at 10:54:38AM -0500, Christoph Lameter wrote:
> On Fri, 2 Jul 2010, Naoya Horiguchi wrote:
>
> > This patch introduces pinning the old page during page migration
> > to avoid freeing it before we complete copying.
>
> The old page is already pinned due to the reference count that is taken
> when the page is put onto the list of pages to be migrated. See
> do_move_pages() f.e.

OK.

> Huge pages use a different scheme?

Different scheme is in soft offline, where the target page is not pinned
before migration. So I should have pinned in soft offline side.
I'll fix it.

> > This race condition can happen for privately mapped or anonymous hugepage.
>
> It cannot happen unless you come up with your own scheme of managing pages
> to be migrated and bypass migrate_pages(). There you should take the
> refcount.

Yes.

> > /*
> > + * It's reasonable to pin the old page until unmapping and copying
> > + * complete, because when the original page is an anonymous hugepage,
> > + * it will be freed in try_to_unmap() due to the fact that
> > + * all references of anonymous hugepage come from mapcount.
> > + * Although in the other cases no problem comes out without pinning,
> > + * it looks logically correct to do it.
> > + */
> > + get_page(page);
> > +
> > + /*
>
> Its already pinned. Dont do this. migrate_pages() relies on the caller
> having pinned the page already.

I agree.

Thanks,
Naoya Horiguchi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: bluetooth: btmrvl_sdio: silence warning
Next: Removing dead CONFIG_DIAGNOSTICS