Re: [PATCH v2] tools/mm/page_owner_sort: free per-record allocations

[Vishal Moola <vishal.moola@gmail.com>]

On Thu, Jun 11, 2026 at 09:34:45AM +0100, Vishal Moola wrote:
> On Thu, Jun 11, 2026 at 10:34:11AM +0800, Yichong Chen wrote:
> > add_list() allocates comm and txt for each page owner record, but the
> > cleanup path only frees the outer list array. This leaks both buffers for
> > every retained record.
> > 
> > Free discarded records during culling and free the retained records on
> > exit. Also unwind comm when allocating txt fails.
> > 
> > Signed-off-by: Yichong Chen <chenyichong@uniontech.com>
> > ---
> > Changes in v2:
> > - Wrap commit message lines to approximately 75 columns.
> > - Use "Yichong Chen" as the author name.
> 
> Thanks. Also, in the future send new versions as new threads :)
> 
> The patch looks fine, just see below for my comment you might have
> missed last time.

Sashiko also caught that is_need() calls get_comm(). That looks like it
should be handling the NULL return case as well. It appears to be an
existing issue, but it could fit into this patch if respinning for v3.

> > @@ -480,9 +489,15 @@ static bool add_list(char *buf, int len, char *ext_buf)
> >  	list[list_size].pid = get_pid(buf);
> >  	list[list_size].tgid = get_tgid(buf);
> >  	list[list_size].comm = get_comm(buf);
> > +	if (!list[list_size].comm) {
> > +		fprintf(stderr, "Out of memory\n");
> > +		return false;
> > +	}
> >  	list[list_size].txt = malloc(len+1);
> >  	if (!list[list_size].txt) {
> >  		fprintf(stderr, "Out of memory\n");
> > +		free(list[list_size].comm);
> > +		list[list_size].comm = NULL;
> >  		return false;
> 
> Returning false here sends us back to the error handling path in main()
> where you end up calling your free_block_list() anyway. So we don't need
> this here, right?
> 
> >  	}
> >  	memcpy(list[list_size].txt, buf, len);
>