[PATCH RFC v3 07/35] mm: cma: Add CMA_RELEASE_{SUCCESS,FAIL} events

Alexandru Elisei posted 35 patches 1 year, 10 months ago
[PATCH RFC v3 07/35] mm: cma: Add CMA_RELEASE_{SUCCESS,FAIL} events
Posted by Alexandru Elisei 1 year, 10 months ago
Similar to the two events that relate to CMA allocations, add the
CMA_RELEASE_SUCCESS and CMA_RELEASE_FAIL events that count when CMA pages
are freed.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---

Changes since rfc v2:

* New patch.

 include/linux/vm_event_item.h | 2 ++
 mm/cma.c                      | 6 +++++-
 mm/vmstat.c                   | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/linux/vm_event_item.h b/include/linux/vm_event_item.h
index 747943bc8cc2..aba5c5bf8127 100644
--- a/include/linux/vm_event_item.h
+++ b/include/linux/vm_event_item.h
@@ -83,6 +83,8 @@ enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT,
 #ifdef CONFIG_CMA
 		CMA_ALLOC_SUCCESS,
 		CMA_ALLOC_FAIL,
+		CMA_RELEASE_SUCCESS,
+		CMA_RELEASE_FAIL,
 #endif
 		UNEVICTABLE_PGCULLED,	/* culled to noreclaim list */
 		UNEVICTABLE_PGSCANNED,	/* scanned for reclaimability */
diff --git a/mm/cma.c b/mm/cma.c
index dbf7fe8cb1bd..543bb6b3be8e 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -562,8 +562,10 @@ bool cma_release(struct cma *cma, const struct page *pages,
 {
 	unsigned long pfn;
 
-	if (!cma_pages_valid(cma, pages, count))
+	if (!cma_pages_valid(cma, pages, count)) {
+		count_vm_events(CMA_RELEASE_FAIL, count);
 		return false;
+	}
 
 	pr_debug("%s(page %p, count %lu)\n", __func__, (void *)pages, count);
 
@@ -575,6 +577,8 @@ bool cma_release(struct cma *cma, const struct page *pages,
 	cma_clear_bitmap(cma, pfn, count);
 	trace_cma_release(cma->name, pfn, pages, count);
 
+	count_vm_events(CMA_RELEASE_SUCCESS, count);
+
 	return true;
 }
 
diff --git a/mm/vmstat.c b/mm/vmstat.c
index db79935e4a54..eebfd5c6c723 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1340,6 +1340,8 @@ const char * const vmstat_text[] = {
 #ifdef CONFIG_CMA
 	"cma_alloc_success",
 	"cma_alloc_fail",
+	"cma_release_success",
+	"cma_release_fail",
 #endif
 	"unevictable_pgs_culled",
 	"unevictable_pgs_scanned",
-- 
2.43.0
Re: [PATCH RFC v3 07/35] mm: cma: Add CMA_RELEASE_{SUCCESS,FAIL} events
Posted by Anshuman Khandual 1 year, 10 months ago

On 1/25/24 22:12, Alexandru Elisei wrote:
> Similar to the two events that relate to CMA allocations, add the
> CMA_RELEASE_SUCCESS and CMA_RELEASE_FAIL events that count when CMA pages
> are freed.

How is this is going to be beneficial towards analyzing CMA alloc/release
behaviour - particularly with respect to this series. OR just adding this
from parity perspective with CMA alloc side counters ? Regardless this
CMA change too could be discussed separately.

> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
> 
> Changes since rfc v2:
> 
> * New patch.
> 
>  include/linux/vm_event_item.h | 2 ++
>  mm/cma.c                      | 6 +++++-
>  mm/vmstat.c                   | 2 ++
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/vm_event_item.h b/include/linux/vm_event_item.h
> index 747943bc8cc2..aba5c5bf8127 100644
> --- a/include/linux/vm_event_item.h
> +++ b/include/linux/vm_event_item.h
> @@ -83,6 +83,8 @@ enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT,
>  #ifdef CONFIG_CMA
>  		CMA_ALLOC_SUCCESS,
>  		CMA_ALLOC_FAIL,
> +		CMA_RELEASE_SUCCESS,
> +		CMA_RELEASE_FAIL,
>  #endif
>  		UNEVICTABLE_PGCULLED,	/* culled to noreclaim list */
>  		UNEVICTABLE_PGSCANNED,	/* scanned for reclaimability */
> diff --git a/mm/cma.c b/mm/cma.c
> index dbf7fe8cb1bd..543bb6b3be8e 100644
> --- a/mm/cma.c
> +++ b/mm/cma.c
> @@ -562,8 +562,10 @@ bool cma_release(struct cma *cma, const struct page *pages,
>  {
>  	unsigned long pfn;
>  
> -	if (!cma_pages_valid(cma, pages, count))
> +	if (!cma_pages_valid(cma, pages, count)) {
> +		count_vm_events(CMA_RELEASE_FAIL, count);
>  		return false;
> +	}
>  
>  	pr_debug("%s(page %p, count %lu)\n", __func__, (void *)pages, count);
>  
> @@ -575,6 +577,8 @@ bool cma_release(struct cma *cma, const struct page *pages,
>  	cma_clear_bitmap(cma, pfn, count);
>  	trace_cma_release(cma->name, pfn, pages, count);
>  
> +	count_vm_events(CMA_RELEASE_SUCCESS, count);
> +
>  	return true;
>  }
>  
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> index db79935e4a54..eebfd5c6c723 100644
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -1340,6 +1340,8 @@ const char * const vmstat_text[] = {
>  #ifdef CONFIG_CMA
>  	"cma_alloc_success",
>  	"cma_alloc_fail",
> +	"cma_release_success",
> +	"cma_release_fail",
>  #endif
>  	"unevictable_pgs_culled",
>  	"unevictable_pgs_scanned",
Re: [PATCH RFC v3 07/35] mm: cma: Add CMA_RELEASE_{SUCCESS,FAIL} events
Posted by Alexandru Elisei 1 year, 10 months ago
Hi,

On Mon, Jan 29, 2024 at 03:01:24PM +0530, Anshuman Khandual wrote:
> 
> 
> On 1/25/24 22:12, Alexandru Elisei wrote:
> > Similar to the two events that relate to CMA allocations, add the
> > CMA_RELEASE_SUCCESS and CMA_RELEASE_FAIL events that count when CMA pages
> > are freed.
> 
> How is this is going to be beneficial towards analyzing CMA alloc/release
> behaviour - particularly with respect to this series. OR just adding this
> from parity perspective with CMA alloc side counters ? Regardless this
> CMA change too could be discussed separately.

Added for parity and because it's useful for this series (see my reply to
the previous patch where I discuss how I've used the counters).

Thanks,
Alex

> 
> > 
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > ---
> > 
> > Changes since rfc v2:
> > 
> > * New patch.
> > 
> >  include/linux/vm_event_item.h | 2 ++
> >  mm/cma.c                      | 6 +++++-
> >  mm/vmstat.c                   | 2 ++
> >  3 files changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/vm_event_item.h b/include/linux/vm_event_item.h
> > index 747943bc8cc2..aba5c5bf8127 100644
> > --- a/include/linux/vm_event_item.h
> > +++ b/include/linux/vm_event_item.h
> > @@ -83,6 +83,8 @@ enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT,
> >  #ifdef CONFIG_CMA
> >  		CMA_ALLOC_SUCCESS,
> >  		CMA_ALLOC_FAIL,
> > +		CMA_RELEASE_SUCCESS,
> > +		CMA_RELEASE_FAIL,
> >  #endif
> >  		UNEVICTABLE_PGCULLED,	/* culled to noreclaim list */
> >  		UNEVICTABLE_PGSCANNED,	/* scanned for reclaimability */
> > diff --git a/mm/cma.c b/mm/cma.c
> > index dbf7fe8cb1bd..543bb6b3be8e 100644
> > --- a/mm/cma.c
> > +++ b/mm/cma.c
> > @@ -562,8 +562,10 @@ bool cma_release(struct cma *cma, const struct page *pages,
> >  {
> >  	unsigned long pfn;
> >  
> > -	if (!cma_pages_valid(cma, pages, count))
> > +	if (!cma_pages_valid(cma, pages, count)) {
> > +		count_vm_events(CMA_RELEASE_FAIL, count);
> >  		return false;
> > +	}
> >  
> >  	pr_debug("%s(page %p, count %lu)\n", __func__, (void *)pages, count);
> >  
> > @@ -575,6 +577,8 @@ bool cma_release(struct cma *cma, const struct page *pages,
> >  	cma_clear_bitmap(cma, pfn, count);
> >  	trace_cma_release(cma->name, pfn, pages, count);
> >  
> > +	count_vm_events(CMA_RELEASE_SUCCESS, count);
> > +
> >  	return true;
> >  }
> >  
> > diff --git a/mm/vmstat.c b/mm/vmstat.c
> > index db79935e4a54..eebfd5c6c723 100644
> > --- a/mm/vmstat.c
> > +++ b/mm/vmstat.c
> > @@ -1340,6 +1340,8 @@ const char * const vmstat_text[] = {
> >  #ifdef CONFIG_CMA
> >  	"cma_alloc_success",
> >  	"cma_alloc_fail",
> > +	"cma_release_success",
> > +	"cma_release_fail",
> >  #endif
> >  	"unevictable_pgs_culled",
> >  	"unevictable_pgs_scanned",
Re: [PATCH RFC v3 07/35] mm: cma: Add CMA_RELEASE_{SUCCESS,FAIL} events
Posted by Anshuman Khandual 1 year, 10 months ago

On 1/29/24 17:23, Alexandru Elisei wrote:
> Hi,
> 
> On Mon, Jan 29, 2024 at 03:01:24PM +0530, Anshuman Khandual wrote:
>>
>> On 1/25/24 22:12, Alexandru Elisei wrote:
>>> Similar to the two events that relate to CMA allocations, add the
>>> CMA_RELEASE_SUCCESS and CMA_RELEASE_FAIL events that count when CMA pages
>>> are freed.
>> How is this is going to be beneficial towards analyzing CMA alloc/release
>> behaviour - particularly with respect to this series. OR just adding this
>> from parity perspective with CMA alloc side counters ? Regardless this
>> CMA change too could be discussed separately.
> Added for parity and because it's useful for this series (see my reply to
> the previous patch where I discuss how I've used the counters).

As mentioned earlier, a new CONFIG_CMA_SYSFS element 'cma->nr_freed_pages'
could be instrumented in cma_release()'s success path for this purpose.
But again the failure path is not of much value as it could only happen
when there is an invalid input from the caller i.e when cma_pages_valid()
check fails.