linux-kernel - Re: [PATCH v2] mm: cma: indefinitely retry allocations in cma

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: <75058e054256df7fddf0bfc251b43aa1@codeaurora.org>
Date:   Mon, 14 Sep 2020 11:39:43 -0700
From:   Chris Goldsworthy <cgoldswo@...eaurora.org>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     linux-kernel@...r.kernel.org,
        Vinayak Menon <vinmenon@...eaurora.org>,
        linux-kernel-owner@...r.kernel.org
Subject: Re: [PATCH v2] mm: cma: indefinitely retry allocations in cma_alloc

On 2020-09-11 14:37, Florian Fainelli wrote:
> On 9/11/2020 1:54 PM, Chris Goldsworthy wrote:
>> CMA allocations will fail if 'pinned' pages are in a CMA area, since 
>> we
>> cannot migrate pinned pages. The _refcount of a struct page being 
>> greater
>> than _mapcount for that page can cause pinning for anonymous pages.  
>> This
>> is because try_to_unmap(), which (1) is called in the CMA allocation 
>> path,
>> and (2) decrements both _refcount and _mapcount for a page, will stop
>> unmapping a page from VMAs once the _mapcount for a page reaches 0.  
>> This
>> implies that after try_to_unmap() has finished successfully for a page
>> where _recount > _mapcount, that _refcount will be greater than 0.  
>> Later
>> in the CMA allocation path in migrate_page_move_mapping(), we will 
>> have one
>> more reference count than intended for anonymous pages, meaning the
>> allocation will fail for that page.
>> 
>> One example of where _refcount can be greater than _mapcount for a 
>> page we
>> would not expect to be pinned is inside of copy_one_pte(), which is 
>> called
>> during a fork. For ptes for which pte_present(pte) == true, 
>> copy_one_pte()
>> will increment the _refcount field followed by the  _mapcount field of 
>> a
>> page. If the process doing copy_one_pte() is context switched out 
>> after
>> incrementing _refcount but before incrementing _mapcount, then the 
>> page
>> will be temporarily pinned.
>> 
>> So, inside of cma_alloc(), instead of giving up when 
>> alloc_contig_range()
>> returns -EBUSY after having scanned a whole CMA-region bitmap, perform
>> retries indefinitely, with sleeps, to give the system an opportunity 
>> to
>> unpin any pinned pages.
> 
> I am by no means an authoritative CMA person but this behavior does
> not seem acceptable, there is no doubt the existing one is sub-optimal
> under specific circumstances, but an indefinite retry, as well as a
> 100ms sleep appear to be arbitrary at best. How about you introduce a
> parameter that allows the tuning of the number of retries and/or delay
> between retries?
> 

Apologies Florian, I messed up on the threading and there are 
discussions that aren't reference here.  The original version of this 
patch was doing a finite number of retires.  Also, this e-mail was just 
sent out to LKML so I could debug some issues I was facing with git 
send-email.  The actual thread is now here, which summarizes the 
discussions w.r.t. this patch so far:  
https://lkml.org/lkml/2020/9/14/1097

Thanks,

Chris.

-- 
The Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,
a Linux Foundation Collaborative Project