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

[Ingo Molnar]

* Stefan Strogin <s.strogin@xxxxxxxxxxxxxxxxxxx> 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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/