On 25.03.21 17:56, Mike Kravetz wrote:
On 3/25/21 3:22 AM, Michal Hocko wrote:
On Thu 25-03-21 10:56:38, David Hildenbrand wrote:
On 25.03.21 01:28, Mike Kravetz wrote:
From: Roman Gushchin <guro@xxxxxx>
cma_release() has to lock the cma_lock mutex to clear the cma bitmap.
It makes it a blocking function, which complicates its usage from
non-blocking contexts. For instance, hugetlbfs code is temporarily
dropping the hugetlb_lock spinlock to call cma_release().
This patch introduces a non-blocking cma_release_nowait(), which
postpones the cma bitmap clearance. It's done later from a work
context. The first page in the cma allocation is used to store
the work struct. Because CMA allocations and de-allocations are
usually not that frequent, a single global workqueue is used.
To make sure that subsequent cma_alloc() call will pass, cma_alloc()
flushes the cma_release_wq workqueue. To avoid a performance
regression in the case when only cma_release() is used, gate it
by a per-cma area flag, which is set by the first call
of cma_release_nowait().
Signed-off-by: Roman Gushchin <guro@xxxxxx>
[mike.kravetz@xxxxxxxxxx: rebased to v5.12-rc3-mmotm-2021-03-17-22-24]
Signed-off-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx>
---
1. Is there a real reason this is a mutex and not a spin lock? It seems to
only protect the bitmap. Are bitmaps that huge that we spend a significant
amount of time in there?
Good question. Looking at the code it doesn't seem that there is any
blockable operation or any heavy lifting done under the lock.
7ee793a62fa8 ("cma: Remove potential deadlock situation") has introduced
the lock and there was a simple bitmat protection back then. I suspect
the patch just followed the cma_mutex lead and used the same type of the
lock. cma_mutex used to protect alloc_contig_range which is sleepable.
This all suggests that there is no real reason to use a sleepable lock
at all and we do not need all this heavy lifting.
When Roman first proposed these patches, I brought up the same issue:
https://lore.kernel.org/linux-mm/20201022023352.GC300658@xxxxxxxxxxxxxxxxxxxxxxxxxxx/
Previously, Roman proposed replacing the mutex with a spinlock but
Joonsoo was opposed.
Adding Joonsoo on Cc:
There has to be a good reason not to. And if there is a good reason,
lockless clearing might be one feasible alternative.
--
Thanks,
David / dhildenb
