linux-kernel - Re: [PATCH v7 09/12] mm/demotion: Demote pages according to allocation fallback order

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 for Android: free password hash cracker in your pocket

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

Date:   Mon, 27 Jun 2022 14:18:26 +1000
From:   Alistair Popple <apopple@...dia.com>
To:     "Aneesh Kumar K.V" <aneesh.kumar@...ux.ibm.com>
Cc:     linux-mm@...ck.org, akpm@...ux-foundation.org,
        Wei Xu <weixugc@...gle.com>, Huang Ying <ying.huang@...el.com>,
        Yang Shi <shy828301@...il.com>,
        Davidlohr Bueso <dave@...olabs.net>,
        Tim C Chen <tim.c.chen@...el.com>,
        Michal Hocko <mhocko@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Hesham Almatary <hesham.almatary@...wei.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Dan Williams <dan.j.williams@...el.com>,
        Jagdish Gediya <jvgediya@...ux.ibm.com>
Subject: Re: [PATCH v7 09/12] mm/demotion: Demote pages according to
 allocation fallback order


"Aneesh Kumar K.V" <aneesh.kumar@...ux.ibm.com> writes:

> Alistair Popple <apopple@...dia.com> writes:
>
>> "Aneesh Kumar K.V" <aneesh.kumar@...ux.ibm.com> writes:
>>
>>> From: Jagdish Gediya <jvgediya@...ux.ibm.com>
>>
>> [...]
>>
>>> -static struct page *alloc_demote_page(struct page *page, unsigned long node)
>>> +static struct page *alloc_demote_page(struct page *page, unsigned long private)
>>>  {
>>> -	struct migration_target_control mtc = {
>>> -		/*
>>> -		 * Allocate from 'node', or fail quickly and quietly.
>>> -		 * When this happens, 'page' will likely just be discarded
>>> -		 * instead of migrated.
>>> -		 */
>>> -		.gfp_mask = (GFP_HIGHUSER_MOVABLE & ~__GFP_RECLAIM) |
>>> -			    __GFP_THISNODE  | __GFP_NOWARN |
>>> -			    __GFP_NOMEMALLOC | GFP_NOWAIT,
>>> -		.nid = node
>>> -	};
>>> +	struct page *target_page;
>>> +	nodemask_t *allowed_mask;
>>> +	struct migration_target_control *mtc;
>>> +
>>> +	mtc = (struct migration_target_control *)private;
>>> +
>>> +	allowed_mask = mtc->nmask;
>>> +	/*
>>> +	 * make sure we allocate from the target node first also trying to
>>> +	 * reclaim pages from the target node via kswapd if we are low on
>>> +	 * free memory on target node. If we don't do this and if we have low
>>> +	 * free memory on the target memtier, we would start allocating pages
>>> +	 * from higher memory tiers without even forcing a demotion of cold
>>> +	 * pages from the target memtier. This can result in the kernel placing
>>> +	 * hotpages in higher memory tiers.
>>> +	 */
>>> +	mtc->nmask = NULL;
>>> +	mtc->gfp_mask |= __GFP_THISNODE;
>>> +	target_page = alloc_migration_target(page, (unsigned long)&mtc);
>>
>> I finally managed to get a system setup to start testing some of this
>> out. However it quickly crashed due to the bad pointer in the above call
>> - you need mtc not &mtc here.
>
> I remember fixing that during earlier testing. I guess I missed to copy
> the change from test to my development. Thanks for testing this. I have
> now also tested the complete series with the above-suggested changes and did
> make sure we are indeed doing demotion by looking at
> /proc/vmstat:pgdemote_kswapd/pgdemote_direct

No worries. I'm still testing but the early results are looking really
promising for some of our use cases so thanks for picking up this work.

 - Alistair

>>
>>> +	if (target_page)
>>> +		return target_page;
>>> +
>>> +	mtc->gfp_mask &= ~__GFP_THISNODE;
>>> +	mtc->nmask = allowed_mask;
>>>
>>>  	return alloc_migration_target(page, (unsigned long)&mtc);
>>
>> And here.
>>
>
> I will fold this changes in and send a v8 after waiting for review
> feedback from others.
>
> -aneesh