| Message ID | 20240929032757.404707-1-gxxa03070307@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] mm/cma: print total and used count in cma_alloc() | expand |
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.
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
-----邮件原件----- 发件人: 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.
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."
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
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.
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;