mm/cma.c: remove redundant cma_mutex lock

However, two subsystems (CMA and gigantic
huge pages for example) could attempt operations on the same range. If this happens, one thread may 'undo' the work another thread is doing. This can result in pageblocks being incorrectly left marked as MIGRATE_ISOLATE and therefore not available for page allocation.

The cma_mutex which protects alloc_contig_range() was first appeared in commit 7ee793a62fa8c ("cma: Remove potential deadlock situation"), at that time, there is no guarantee the behavior of concurrency inside alloc_contig_range().

After the commit 2c7452a075d4db2dc
("mm/page_isolation.c: make start_isolate_page_range() fail if already isolated")

Quoted 5 lines

However, two subsystems (CMA and gigantic
huge pages for example) could attempt operations on the same range. If this happens, one thread may 'undo' the work another thread is doing. This can result in pageblocks being incorrectly left marked as MIGRATE_ISOLATE and therefore not available for page allocation.

The concurrency inside alloc_contig_range() was clarified.

Now we can find that hugepage and virtio call alloc_contig_range() without any lock, thus cma_mutex is "redundant" in cma_alloc() now.

Signed-off-by Lecopzer Chen <[email protected]>

---
 mm/cma.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

mm/cma.c +0 -1

index 7f415d7cda9f..3692a34e2353 100644
@@ -38,7 +38,6 @@
 
 struct cma cma_areas[MAX_CMA_AREAS];
 unsigned cma_area_count;
-static DEFINE_MUTEX(cma_mutex);
 
 phys_addr_t cma_get_base(const struct cma *cma)
 {

@@ -454,10 +453,9 @@ struct page *cma_alloc(struct cma *cma, size_t count, unsigned int align,
 		mutex_unlock(&cma->lock);

Quoted 37 lines · Lecopzer Chen

The cma_mutex which protects alloc_contig_range() was first appeared in commit 7ee793a62fa8c ("cma: Remove potential deadlock situation"), at that time, there is no guarantee the behavior of concurrency inside alloc_contig_range().

After the commit 2c7452a075d4db2dc
("mm/page_isolation.c: make start_isolate_page_range() fail if already isolated")

Quoted 5 lines

However, two subsystems (CMA and gigantic
huge pages for example) could attempt operations on the same range. If this happens, one thread may 'undo' the work another thread is doing. This can result in pageblocks being incorrectly left marked as MIGRATE_ISOLATE and therefore not available for page allocation.

The concurrency inside alloc_contig_range() was clarified.

Now we can find that hugepage and virtio call alloc_contig_range() without any lock, thus cma_mutex is "redundant" in cma_alloc() now.

Signed-off-by Lecopzer Chen <[email protected]>

---
 mm/cma.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

mm/cma.c +0 -1

index 7f415d7cda9f..3692a34e2353 100644
@@ -38,7 +38,6 @@
 
 struct cma cma_areas[MAX_CMA_AREAS];
 unsigned cma_area_count;
-static DEFINE_MUTEX(cma_mutex);
 
 phys_addr_t cma_get_base(const struct cma *cma)
 {

@@ -454,10 +453,9 @@ struct page *cma_alloc(struct cma *cma, size_t count, unsigned int align,
 		mutex_unlock(&cma->lock);
 

mm/cma.c struct page *cma_alloc(struct cma *cma, size_t count, unsigned int align, · line 461

Quoted 10 lines

mm/cma.c +1 -2

 		pfn = cma->base_pfn + (bitmap_no << cma->order_per_bit);
-		mutex_lock(&cma_mutex);
 		ret = alloc_contig_range(pfn, pfn + count, MIGRATE_CMA,
 				     GFP_KERNEL | (no_warn ? __GFP_NOWARN : 0));
-		mutex_unlock(&cma_mutex);
+
 		if (ret == 0) {
 			page = pfn_to_page(pfn);
 			break;

I guess this is fine. In case there is a race we return with -EBUSY - which is suboptimal (as it could just be a temporary issue if the other user backs off), but should be good enough for now.

Acked-by David Hildenbrand <[email protected]>