linux-kernel - Re: [PATCH v7 3/3] mm/madvise: optimize lazyfreeing with mTHP in madvise

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <3a0d1d1c-97f4-4c9d-bc25-9de3f3659b67@redhat.com>
Date: Tue, 16 Apr 2024 18:52:40 +0200
From: David Hildenbrand <david@...hat.com>
To: Ryan Roberts <ryan.roberts@....com>, Lance Yang <ioworker0@...il.com>,
 akpm@...ux-foundation.org
Cc: 21cnbao@...il.com, mhocko@...e.com, fengwei.yin@...el.com,
 zokeefe@...gle.com, shy828301@...il.com, xiehuan09@...il.com,
 wangkefeng.wang@...wei.com, songmuchun@...edance.com, peterx@...hat.com,
 minchan@...nel.org, linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 3/3] mm/madvise: optimize lazyfreeing with mTHP in
 madvise_free

>> +			nr = madvise_folio_pte_batch(addr, end, folio, pte,
>> +						     ptent, &any_young, &any_dirty);
>> +
>> +			if (nr < folio_nr_pages(folio)) {
>> +				if (folio_likely_mapped_shared(folio))
>> +					continue;
>> +
>> +				arch_leave_lazy_mmu_mode();
>> +				if (madvise_pte_split_folio(mm, pmd, addr,
>> +							    folio, &start_pte, &ptl))
>> +					nr = 0;
>> +				if (!start_pte)
>> +					break;
>> +				pte = start_pte;
>> +				arch_enter_lazy_mmu_mode();
>> +				continue;
>> +			}
>> +
>> +			if (any_young)
>> +				ptent = pte_mkyoung(ptent);
>> +			if (any_dirty)
>> +				ptent = pte_mkdirty(ptent);
>>   		}
>>   
>> +		if (folio_mapcount(folio) != folio_nr_pages(folio))
>> +			continue;
> 
> Why is this here? I thought we had previously concluded to only do this test
> inside the below if statement (where you have it duplicated).

I stumbled over these same while reviewing. It's not exactly duplicate, 
because it's unreliable without the folio lock. It looks more like an 
best-effort early check.

But then, we also add it to cases where we previously wouldn't check the 
mapcount at all: when the folio was added to the swapcache or is already 
dirty.

In that case, we would even see a change for order-0 folios with that 
new check.

-- 
Cheers,

David / dhildenb