[PATCH v2] mm/cma: print total and used count in cma_alloc()

[PATCH v2] mm/cma: print total and used count in cma_alloc()

[Xiang Gao]

From: gaoxiang17 <gaoxiang17@xiaomi.com>

before:
[   24.407814] cma: cma_alloc(cma (____ptrval____), name: reserved, count 1, align 0)
[   24.413397] cma: cma_alloc(cma (____ptrval____), name: reserved, count 1, align 0)
[   24.415886] cma: cma_alloc(cma (____ptrval____), name: reserved, count 1, align 0)

after:
[   24.069738] cma: cma_alloc(cma (____ptrval____), name: reserved, total pages: 16384, used pages: 64, request pages: 1, align 0)
[   24.075317] cma: cma_alloc(cma (____ptrval____), name: reserved, total pages: 16384, used pages: 65, request pages: 1, align 0)
[   24.078455] cma: cma_alloc(cma (____ptrval____), name: reserved, total pages: 16384, used pages: 66, request pages: 1, align 0)

Signed-off-by: gaoxiang17 <gaoxiang17@xiaomi.com>
---
 mm/cma.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/mm/cma.c b/mm/cma.c
index 2d9fae939283..90b3fdbac19c 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -403,6 +403,17 @@ static void cma_debug_show_areas(struct cma *cma)
 	spin_unlock_irq(&cma->lock);
 }
 
+static unsigned long cma_get_used_pages(struct cma *cma)
+{
+	unsigned long used;
+
+	spin_lock_irq(&cma->lock);
+	used = bitmap_weight(cma->bitmap, (int)cma_bitmap_maxno(cma));
+	spin_unlock_irq(&cma->lock);
+
+	return used << cma->order_per_bit;
+}
+
 static struct page *__cma_alloc(struct cma *cma, unsigned long count,
 				unsigned int align, gfp_t gfp)
 {
@@ -420,8 +431,8 @@ static struct page *__cma_alloc(struct cma *cma, unsigned long count,
 	if (!cma || !cma->count || !cma->bitmap)
 		return page;
 
-	pr_debug("%s(cma %p, name: %s, count %lu, align %d)\n", __func__,
-		(void *)cma, cma->name, count, align);
+	pr_debug("%s(cma %p, name: %s, total pages: %lu, used pages: %lu, request pages: %lu, align %d)\n", __func__,
+		(void *)cma, cma->name, cma->count, cma_get_used_pages(cma), count, align);
 
 	if (!count)
 		return page;
-- 
2.34.1

Re: [PATCH v2] mm/cma: print total and used count in cma_alloc()

[David Hildenbrand]

On 29.09.24 05:27, Xiang Gao wrote:
> From: gaoxiang17 <gaoxiang17@xiaomi.com>
> 

We should add here

"To debug CMA allocations (especially failing ones), it is valuable to 
know the state of CMA: how many pages out of the total ones are 
allocated, and how many were requested to be allocated. Let's print
some more information."

I assume Andrew can fix that up when applying.

> before:
> [   24.407814] cma: cma_alloc(cma (____ptrval____), name: reserved, count 1, align 0)
> [   24.413397] cma: cma_alloc(cma (____ptrval____), name: reserved, count 1, align 0)
> [   24.415886] cma: cma_alloc(cma (____ptrval____), name: reserved, count 1, align 0)
> 
> after:
> [   24.069738] cma: cma_alloc(cma (____ptrval____), name: reserved, total pages: 16384, used pages: 64, request pages: 1, align 0)
> [   24.075317] cma: cma_alloc(cma (____ptrval____), name: reserved, total pages: 16384, used pages: 65, request pages: 1, align 0)
> [   24.078455] cma: cma_alloc(cma (____ptrval____), name: reserved, total pages: 16384, used pages: 66, request pages: 1, align 0)
> 
> Signed-off-by: gaoxiang17 <gaoxiang17@xiaomi.com>
> ---
>   mm/cma.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/cma.c b/mm/cma.c
> index 2d9fae939283..90b3fdbac19c 100644
> --- a/mm/cma.c
> +++ b/mm/cma.c
> @@ -403,6 +403,17 @@ static void cma_debug_show_areas(struct cma *cma)
>   	spin_unlock_irq(&cma->lock);
>   }
>   
> +static unsigned long cma_get_used_pages(struct cma *cma)
> +{
> +	unsigned long used;
> +
> +	spin_lock_irq(&cma->lock);
> +	used = bitmap_weight(cma->bitmap, (int)cma_bitmap_maxno(cma));
> +	spin_unlock_irq(&cma->lock);

This adds overhead to each allocation, even if debug outputs are ignored 
I assume?

I wonder if we'd want to print these details only when our allocation 
failed?

Alternatively, we could actually track how many pages are allocated in 
the cma, so we don't have to traverse the complete bitmap on every 
allocation.


-- 
Cheers,

David / dhildenb

Re: [PATCH v2] mm/cma: print total and used count in cma_alloc()

[Frank van der Linden]

On Mon, Sep 30, 2024 at 2:34 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 29.09.24 05:27, Xiang Gao wrote:
> > From: gaoxiang17 <gaoxiang17@xiaomi.com>
> >
>
> We should add here
>
> "To debug CMA allocations (especially failing ones), it is valuable to
> know the state of CMA: how many pages out of the total ones are
> allocated, and how many were requested to be allocated. Let's print
> some more information."
>
> I assume Andrew can fix that up when applying.
>
> > before:
> > [   24.407814] cma: cma_alloc(cma (____ptrval____), name: reserved, count 1, align 0)
> > [   24.413397] cma: cma_alloc(cma (____ptrval____), name: reserved, count 1, align 0)
> > [   24.415886] cma: cma_alloc(cma (____ptrval____), name: reserved, count 1, align 0)
> >
> > after:
> > [   24.069738] cma: cma_alloc(cma (____ptrval____), name: reserved, total pages: 16384, used pages: 64, request pages: 1, align 0)
> > [   24.075317] cma: cma_alloc(cma (____ptrval____), name: reserved, total pages: 16384, used pages: 65, request pages: 1, align 0)
> > [   24.078455] cma: cma_alloc(cma (____ptrval____), name: reserved, total pages: 16384, used pages: 66, request pages: 1, align 0)
> >
> > Signed-off-by: gaoxiang17 <gaoxiang17@xiaomi.com>
> > ---
> >   mm/cma.c | 15 +++++++++++++--
> >   1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/cma.c b/mm/cma.c
> > index 2d9fae939283..90b3fdbac19c 100644
> > --- a/mm/cma.c
> > +++ b/mm/cma.c
> > @@ -403,6 +403,17 @@ static void cma_debug_show_areas(struct cma *cma)
> >       spin_unlock_irq(&cma->lock);
> >   }
> >
> > +static unsigned long cma_get_used_pages(struct cma *cma)
> > +{
> > +     unsigned long used;
> > +
> > +     spin_lock_irq(&cma->lock);
> > +     used = bitmap_weight(cma->bitmap, (int)cma_bitmap_maxno(cma));
> > +     spin_unlock_irq(&cma->lock);
>
> This adds overhead to each allocation, even if debug outputs are ignored
> I assume?
>
> I wonder if we'd want to print these details only when our allocation
> failed?
>
> Alternatively, we could actually track how many pages are allocated in
> the cma, so we don't have to traverse the complete bitmap on every
> allocation.
>

Yep, that's what I did as part of
https://lore.kernel.org/all/20240724124845.614c03ad39f8af3729cebee6@linux-foundation.org/T/

That patch didn't make it in (yet). I'm happy for it to be combined
with this one if that's easier.

- Frank

回复: [External Mail]Re: [PATCH v2] mm/cma: print total and used count in cma_alloc()

[高翔]

-----邮件原件-----
发件人: Frank van der Linden <fvdl@google.com> 
发送时间: 2024年10月3日 3:50
收件人: David Hildenbrand <david@redhat.com>
抄送: Xiang Gao <gxxa03070307@gmail.com>; akpm@linux-foundation.org; linux-mm@kvack.org; linux-kernel@vger.kernel.org; 高翔 <gaoxiang17@xiaomi.com>
主题: [External Mail]Re: [PATCH v2] mm/cma: print total and used count in cma_alloc()

[外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给misec@xiaomi.com进行反馈

On Mon, Sep 30, 2024 at 2:34 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 29.09.24 05:27, Xiang Gao wrote:
> > From: gaoxiang17 <gaoxiang17@xiaomi.com>
> >
>
> We should add here
>
> "To debug CMA allocations (especially failing ones), it is valuable to 
> know the state of CMA: how many pages out of the total ones are 
> allocated, and how many were requested to be allocated. Let's print 
> some more information."
>
> I assume Andrew can fix that up when applying.
>
> > before:
> > [   24.407814] cma: cma_alloc(cma (____ptrval____), name: reserved, count 1, align 0)
> > [   24.413397] cma: cma_alloc(cma (____ptrval____), name: reserved, count 1, align 0)
> > [   24.415886] cma: cma_alloc(cma (____ptrval____), name: reserved, count 1, align 0)
> >
> > after:
> > [   24.069738] cma: cma_alloc(cma (____ptrval____), name: reserved, total pages: 16384, used pages: 64, request pages: 1, align 0)
> > [   24.075317] cma: cma_alloc(cma (____ptrval____), name: reserved, total pages: 16384, used pages: 65, request pages: 1, align 0)
> > [   24.078455] cma: cma_alloc(cma (____ptrval____), name: reserved, total pages: 16384, used pages: 66, request pages: 1, align 0)
> >
> > Signed-off-by: gaoxiang17 <gaoxiang17@xiaomi.com>
> > ---
> >   mm/cma.c | 15 +++++++++++++--
> >   1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/cma.c b/mm/cma.c
> > index 2d9fae939283..90b3fdbac19c 100644
> > --- a/mm/cma.c
> > +++ b/mm/cma.c
> > @@ -403,6 +403,17 @@ static void cma_debug_show_areas(struct cma *cma)
> >       spin_unlock_irq(&cma->lock);
> >   }
> >
> > +static unsigned long cma_get_used_pages(struct cma *cma) {
> > +     unsigned long used;
> > +
> > +     spin_lock_irq(&cma->lock);
> > +     used = bitmap_weight(cma->bitmap, (int)cma_bitmap_maxno(cma));
> > +     spin_unlock_irq(&cma->lock);
>
> This adds overhead to each allocation, even if debug outputs are 
> ignored I assume?
>
> I wonder if we'd want to print these details only when our allocation 
> failed?
>
> Alternatively, we could actually track how many pages are allocated in 
> the cma, so we don't have to traverse the complete bitmap on every 
> allocation.
>

Yep, that's what I did as part of
https://lore.kernel.org/all/20240724124845.614c03ad39f8af3729cebee6@linux-foundation.org/T/

That patch didn't make it in (yet). I'm happy for it to be combined with this one if that's easier.

- Frank

Yes, I think it's simpler to add new members to the cma structure to manage. 
This does not add much overhead to the used count per print.

If this is possible, we can also add "caller" members to troubleshoot problems.

Re: 回复: [External Mail]Re: [PATCH v2] mm/cma: print total and used count in cma_alloc()

[Andrew Morton]

On Fri, 4 Oct 2024 12:23:30 +0000 高翔 <gaoxiang17@xiaomi.com> wrote:

> > > +static unsigned long cma_get_used_pages(struct cma *cma) {
> > > +     unsigned long used;
> > > +
> > > +     spin_lock_irq(&cma->lock);
> > > +     used = bitmap_weight(cma->bitmap, (int)cma_bitmap_maxno(cma));
> > > +     spin_unlock_irq(&cma->lock);
> >
> > This adds overhead to each allocation, even if debug outputs are 
> > ignored I assume?
> >
> > I wonder if we'd want to print these details only when our allocation 
> > failed?
> >
> > Alternatively, we could actually track how many pages are allocated in 
> > the cma, so we don't have to traverse the complete bitmap on every 
> > allocation.
> >
> 
> Yep, that's what I did as part of
> https://lore.kernel.org/all/20240724124845.614c03ad39f8af3729cebee6@linux-foundation.org/T/
> 
> That patch didn't make it in (yet). I'm happy for it to be combined with this one if that's easier.

That patch has been forgotten about.  As I asked in July,
"I suggest a resend, and add some Cc:s for likely reviewers."

Re: 回复: [External Mail]Re: [PATCH v2] mm/cma: print total and used count in cma_alloc()

[Frank van der Linden]

On Fri, Oct 4, 2024 at 3:39 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Fri, 4 Oct 2024 12:23:30 +0000 高翔 <gaoxiang17@xiaomi.com> wrote:
>
> > > > +static unsigned long cma_get_used_pages(struct cma *cma) {
> > > > +     unsigned long used;
> > > > +
> > > > +     spin_lock_irq(&cma->lock);
> > > > +     used = bitmap_weight(cma->bitmap, (int)cma_bitmap_maxno(cma));
> > > > +     spin_unlock_irq(&cma->lock);
> > >
> > > This adds overhead to each allocation, even if debug outputs are
> > > ignored I assume?
> > >
> > > I wonder if we'd want to print these details only when our allocation
> > > failed?
> > >
> > > Alternatively, we could actually track how many pages are allocated in
> > > the cma, so we don't have to traverse the complete bitmap on every
> > > allocation.
> > >
> >
> > Yep, that's what I did as part of
> > https://lore.kernel.org/all/20240724124845.614c03ad39f8af3729cebee6@linux-foundation.org/T/
> >
> > That patch didn't make it in (yet). I'm happy for it to be combined with this one if that's easier.
>
> That patch has been forgotten about.  As I asked in July,
> "I suggest a resend, and add some Cc:s for likely reviewers."

Indeed - I certainly wasn't suggesting that anyone else forgot about
it, it's up to me to follow up here, and I haven't yet.

- Frank

答复: 回复: [External Mail]Re: [PATCH v2] mm/cma: print total and used count in cma_alloc()

[高翔]

[-- Attachment #1: Type: text/plain, Size: 1936 bytes --]


________________________________
发件人: Frank van der Linden <fvdl@google.com>
发送时间: 2024年10月5日 6:55
收件人: Andrew Morton
抄送: 高翔; David Hildenbrand; Xiang Gao; linux-mm@kvack.org; linux-kernel@vger.kernel.org
主题: Re: 回复: [External Mail]Re: [PATCH v2] mm/cma: print total and used count in cma_alloc()

[外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给misec@xiaomi.com进行反馈

On Fri, Oct 4, 2024 at 3:39 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Fri, 4 Oct 2024 12:23:30 +0000 高翔 <gaoxiang17@xiaomi.com> wrote:
>
> > > > +static unsigned long cma_get_used_pages(struct cma *cma) {
> > > > +     unsigned long used;
> > > > +
> > > > +     spin_lock_irq(&cma->lock);
> > > > +     used = bitmap_weight(cma->bitmap, (int)cma_bitmap_maxno(cma));
> > > > +     spin_unlock_irq(&cma->lock);
> > >
> > > This adds overhead to each allocation, even if debug outputs are
> > > ignored I assume?
> > >
> > > I wonder if we'd want to print these details only when our allocation
> > > failed?
> > >
> > > Alternatively, we could actually track how many pages are allocated in
> > > the cma, so we don't have to traverse the complete bitmap on every
> > > allocation.
> > >
> >
> > Yep, that's what I did as part of
> > https://lore.kernel.org/all/20240724124845.614c03ad39f8af3729cebee6@linux-foundation.org/T/
> >
> > That patch didn't make it in (yet). I'm happy for it to be combined with this one if that's easier.
>
> That patch has been forgotten about.  As I asked in July,
> "I suggest a resend, and add some Cc:s for likely reviewers."

> Indeed - I certainly wasn't suggesting that anyone else forgot about
> it, it's up to me to follow up here, and I haven't yet.


Do I need to resend it after adding "used_count" member to the "struct cma" as V3?

[-- Attachment #2: Type: text/html, Size: 3788 bytes --]

Re: 答复: 回复: [External Mail]Re: [PATCH v2] mm/cma: print total and used count in cma_alloc()

[Andrew Morton]

On Thu, 10 Oct 2024 08:48:55 +0000 高翔 <gaoxiang17@xiaomi.com> wrote:

> 
> ________________________________
> 发件人: Frank van der Linden <fvdl@google.com>
> 发送时间: 2024年10月5日 6:55
> 收件人: Andrew Morton
> 抄送: 高翔; David Hildenbrand; Xiang Gao; linux-mm@kvack.org; linux-kernel@vger.kernel.org
> 主题: Re: 回复: [External Mail]Re: [PATCH v2] mm/cma: print total and used count in cma_alloc()
> 
> [外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给misec@xiaomi.com进行反馈
> 
> On Fri, Oct 4, 2024 at 3:39 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > On Fri, 4 Oct 2024 12:23:30 +0000 高翔 <gaoxiang17@xiaomi.com> wrote:
> >
> > > > > +static unsigned long cma_get_used_pages(struct cma *cma) {
> > > > > +     unsigned long used;
> > > > > +
> > > > > +     spin_lock_irq(&cma->lock);
> > > > > +     used = bitmap_weight(cma->bitmap, (int)cma_bitmap_maxno(cma));
> > > > > +     spin_unlock_irq(&cma->lock);
> > > >
> > > > This adds overhead to each allocation, even if debug outputs are
> > > > ignored I assume?
> > > >
> > > > I wonder if we'd want to print these details only when our allocation
> > > > failed?
> > > >
> > > > Alternatively, we could actually track how many pages are allocated in
> > > > the cma, so we don't have to traverse the complete bitmap on every
> > > > allocation.
> > > >
> > >
> > > Yep, that's what I did as part of
> > > https://lore.kernel.org/all/20240724124845.614c03ad39f8af3729cebee6@linux-foundation.org/T/
> > >
> > > That patch didn't make it in (yet). I'm happy for it to be combined with this one if that's easier.
> >
> > That patch has been forgotten about.  As I asked in July,
> > "I suggest a resend, and add some Cc:s for likely reviewers."
> 
> > Indeed - I certainly wasn't suggesting that anyone else forgot about
> > it, it's up to me to follow up here, and I haven't yet.
> 
> 
> Do I need to resend it after adding "used_count" member to the "struct cma" as V3?

I don't know what is being asked here.  What is this used_count?  Some
other patch, presumably?  Which?

The preferable thing to do is to send a patch against latest
mm-unstable or linux-next.

答复: 答复: 回复: [External Mail]Re: [PATCH v2] mm/cma: print total and used count in cma_alloc()

[高翔]

[-- Attachment #1: Type: text/plain, Size: 2834 bytes --]


________________________________
发件人: Andrew Morton <akpm@linux-foundation.org>
发送时间: 2024年10月11日 5:17
收件人: 高翔
抄送: Frank van der Linden; David Hildenbrand; Xiang Gao; linux-mm@kvack.org; linux-kernel@vger.kernel.org
主题: Re: 答复: 回复: [External Mail]Re: [PATCH v2] mm/cma: print total and used count in cma_alloc()

[外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给misec@xiaomi.com进行反馈

On Thu, 10 Oct 2024 08:48:55 +0000 高翔 <gaoxiang17@xiaomi.com> wrote:

>
> ________________________________
> 发件人: Frank van der Linden <fvdl@google.com>
> 发送时间: 2024年10月5日 6:55
> 收件人: Andrew Morton
> 抄送: 高翔; David Hildenbrand; Xiang Gao; linux-mm@kvack.org; linux-kernel@vger.kernel.org
> 主题: Re: 回复: [External Mail]Re: [PATCH v2] mm/cma: print total and used count in cma_alloc()
>
> [外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给misec@xiaomi.com进行反馈
>
> On Fri, Oct 4, 2024 at 3:39 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > On Fri, 4 Oct 2024 12:23:30 +0000 高翔 <gaoxiang17@xiaomi.com> wrote:
> >
> > > > > +static unsigned long cma_get_used_pages(struct cma *cma) {
> > > > > +     unsigned long used;
> > > > > +
> > > > > +     spin_lock_irq(&cma->lock);
> > > > > +     used = bitmap_weight(cma->bitmap, (int)cma_bitmap_maxno(cma));
> > > > > +     spin_unlock_irq(&cma->lock);
> > > >
> > > > This adds overhead to each allocation, even if debug outputs are
> > > > ignored I assume?
> > > >
> > > > I wonder if we'd want to print these details only when our allocation
> > > > failed?
> > > >
> > > > Alternatively, we could actually track how many pages are allocated in
> > > > the cma, so we don't have to traverse the complete bitmap on every
> > > > allocation.
> > > >
> > >
> > > Yep, that's what I did as part of
> > > https://lore.kernel.org/all/20240724124845.614c03ad39f8af3729cebee6@linux-foundation.org/T/
> > >
> > > That patch didn't make it in (yet). I'm happy for it to be combined with this one if that's easier.
> >
> > That patch has been forgotten about.  As I asked in July,
> > "I suggest a resend, and add some Cc:s for likely reviewers."
>
> > Indeed - I certainly wasn't suggesting that anyone else forgot about
> > it, it's up to me to follow up here, and I haven't yet.
>
>
> Do I need to resend it after adding "used_count" member to the "struct cma" as V3?

> I don't know what is being asked here.  What is this used_count?  Some
> other patch, presumably?  Which?

> The preferable thing to do is to send a patch against latest
> mm-unstable or linux-next.

OK.

[-- Attachment #2: Type: text/html, Size: 5751 bytes --]