On Tue, Mar 30, 2021 at 4:18 PM Michal Hocko [off-list ref] wrote:

On Tue 30-03-21 16:08:36, Muchun Song wrote:

quoted

On Tue, Mar 30, 2021 at 4:01 PM Michal Hocko [off-list ref] wrote:

quoted

On Mon 29-03-21 16:23:55, Mike Kravetz wrote:

quoted

Ideally, cma_release could be called from any context.  However, that is
not possible because a mutex is used to protect the per-area bitmap.
Change the bitmap to an irq safe spinlock.

I would phrase the changelog slightly differerent
"
cma_release is currently a sleepable operatation because the bitmap
manipulation is protected by cma->lock mutex. Hugetlb code which relies
on cma_release for CMA backed (giga) hugetlb pages, however, needs to be
irq safe.

The lock doesn't protect any sleepable operation so it can be changed to
a (irq aware) spin lock. The bitmap processing should be quite fast in
typical case but if cma sizes grow to TB then we will likely need to
replace the lock by a more optimized bitmap implementation.
"

it seems that you are overusing irqsave variants even from context which
are never called from the IRQ context so they do not need storing flags.

[...]

quoted

@@ -391,8 +391,9 @@ static void cma_debug_show_areas(struct cma *cma)
      unsigned long start = 0;
      unsigned long nr_part, nr_total = 0;
      unsigned long nbits = cma_bitmap_maxno(cma);
+     unsigned long flags;

-     mutex_lock(&cma->lock);
+     spin_lock_irqsave(&cma->lock, flags);

spin_lock_irq should be sufficient. This is only called from the
allocation context and that is never called from IRQ context.

This makes me think more. I think that spin_lock should be
sufficient. Right?

Nope. Think of the following scenario
        spin_lock(cma->lock);
        <IRQ>
        put_page
          __free_huge_page
            cma_release
              spin_lock_irqsave() DEADLOCK

Got it. Thanks.

--
Michal Hocko
SUSE Labs