Messages in this thread | | | Date | Mon, 14 Sep 2020 11:39:43 -0700 | From | Chris Goldsworthy <> | 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
| |