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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <547C1CC9.7080100@jp.fujitsu.com>
Date:	Mon, 1 Dec 2014 16:46:17 +0900
From:	Yasuaki Ishimatsu <isimatu.yasuaki@...fujitsu.com>
To:	Hugh Dickins <hughd@...gle.com>
CC:	Andrew Morton <akpm@...ux-foundation.org>,
	Davidlohr Bueso <dave@...olabs.net>, <linux-mm@...ck.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mm: unmapped page migration avoid unmap+remap overhead

(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@...r.kernel.org
> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/