Re: [PATCH] mm: unmapped page migration avoid unmap+remap overhead

[Yasuaki Ishimatsu]

(2014/12/01 16:28), Hugh Dickins wrote:
> On Mon, 1 Dec 2014, Yasuaki Ishimatsu wrote:
> > (2014/12/01 13:52), Hugh Dickins wrote:
> > > @@ -798,7 +798,7 @@ static int __unmap_and_move(struct page
> > >    				int force, enum migrate_mode mode)
> > >    {
> > >    	int rc = -EAGAIN;
> > > -	int remap_swapcache = 1;
> > > +	int page_was_mapped = 0;
> > >    	struct anon_vma *anon_vma = NULL;
> > >
> > >    	if (!trylock_page(page)) {
> > > @@ -870,7 +870,6 @@ static int __unmap_and_move(struct page
> > >    			 * migrated but are not remapped when migration
> > >    			 * completes
> > >    			 */
> > > -			remap_swapcache = 0;
> > >    		} else {
> > >    			goto out_unlock;
> > >    		}
> > > @@ -910,13 +909,17 @@ static int __unmap_and_move(struct page
> > >    	}
> > >
> > >    	/* Establish migration ptes or remove ptes */
> >
> > > -	try_to_unmap(page, TTU_MIGRATION|TTU_IGNORE_MLOCK|TTU_IGNORE_ACCESS);
> > > +	if (page_mapped(page)) {
> > > +		try_to_unmap(page,
> > > +			TTU_MIGRATION|TTU_IGNORE_MLOCK|TTU_IGNORE_ACCESS);
> > > +		page_was_mapped = 1;
> > > +	}
> >
> > Is there no possibility that page is swap cache? If page is swap cache,
> > this code changes behavior of move_to_new_page(). Is it O.K.?
>
> Certainly the page may be swap cache, but I don't see how the behavior
> of move_to_new_page() is changed.
>
> Do you mean how I removed that "remap_swapcache = 0;" line above, so that
> it now looks as if move_to_new_page() may be called with page_was_mapped
> 1, where before it was called with remap_swapcache 0?

Yes. I pointed it.

> No: although it cannot be seen from the patch context, that reset
> of remap_swapcache was in a block where we have a PageAnon page, but
> page_get_anon_vma() failed to "get" the anon_vma for it: that means
> that the page was not mapped, so page_was_mapped will be 0 too.
>
> (I was going to add that the page might be faulted back in again by
> the time we reach the page_mapped() test above try_to_unmap(), and
> that yes I'd would be making a change in that case, but it does not
> matter at all to diverge in racy cases.  But actually even that cannot
> happen, since faulting back swap needs page lock which we hold here.)
>
> There is an argument that move_to_new_page() behavior should be
> changed in the case of swap cache: since try_to_unmap() then uses
> the ordinary swap instead of a migration entry, there's not much
> point in going to remove swap entries afterwards; though it would
> be good to make those pages present again.  But I didn't try to
> change that in this patch: this was just a lock contention thing.

Thank you for the explanation.
I understood it.

Thanks,
Yasuaki Ishimatsu


> Hugh
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/