Re: [PATCH v4 1/5] mm: cma: add trace events to debug physically-contiguous memory allocations

[Ingo Molnar <mingo@kernel.org>]

* Stefan Strogin <s.strogin@partner.samsung.com> wrote:

> > +TRACE_EVENT(cma_alloc,
> > +
> > +	TP_PROTO(struct cma *cma, struct page *page, int count),
> > +
> > +	TP_ARGS(cma, page, count),
> > +
> > +	TP_STRUCT__entry(
> > +		__field(struct page *, page)
> > +		__field(unsigned long, count)
> > +	),
> > +
> > +	TP_fast_assign(
> > +		__entry->page = page;
> > +		__entry->count = count;
> > +	),
> > +
> > +	TP_printk("page=%p pfn=%lu count=%lu",
> > +		  __entry->page,
> > +		  __entry->page ? page_to_pfn(__entry->page) : 0,
> > +		  __entry->count)

So I'm wondering, the fast-assign side is not equivalent to the 
TP_printk() side:

> > +		__entry->page = page;
> > +		  __entry->page ? page_to_pfn(__entry->page) : 0,

to me it seems it would be useful if MM tracing standardized on pfn 
printing. Just like you did for trace_cma_release().

Also:

> > +		  __entry->page ? page_to_pfn(__entry->page) : 0,

pfn 0 should probably be reserved for the true 0th pfn - those exist 
in some machines. Returning -1ll could be the 'no such pfn' condition?

> > +	TP_STRUCT__entry(
> > +		__field(unsigned long, pfn)

Btw., does pfn always fit into 32 bits on 32-bit platforms?

> > +		__field(unsigned long, count)

Does this have to be 64-bit on 64-bit platforms?

> > +	),
> > +
> > +	TP_fast_assign(
> > +		__entry->pfn = pfn;
> > +		__entry->count = count;
> > +	),
> > +
> > +	TP_printk("pfn=%lu page=%p count=%lu",
> > +		  __entry->pfn,
> > +		  pfn_to_page(__entry->pfn),
> > +		  __entry->count)

So here you print more in the TP_printk() line than in the fast-assign 
side.

Again I'd double check the various boundary conditions.

Thanks,

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>