From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752561AbbCXNSC (ORCPT ); Tue, 24 Mar 2015 09:18:02 -0400 Received: from mail-pd0-f182.google.com ([209.85.192.182]:34568 "EHLO mail-pd0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752424AbbCXNSA (ORCPT ); Tue, 24 Mar 2015 09:18:00 -0400 Date: Tue, 24 Mar 2015 22:17:08 +0900 From: Namhyung Kim To: Ingo Molnar Cc: Joonsoo Kim , Arnaldo Carvalho de Melo , Peter Zijlstra , Jiri Olsa , LKML , David Ahern , Minchan Kim Subject: Re: [PATCH 2/5] perf kmem: Analyze page allocator events also Message-ID: <20150324131708.GB14900@danjae.kornet> References: <1427092244-22764-1-git-send-email-namhyung@kernel.org> <1427092244-22764-3-git-send-email-namhyung@kernel.org> <20150324001828.GJ2782@sejong> <20150324070802.GB28190@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20150324070802.GB28190@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Ingo, On Tue, Mar 24, 2015 at 08:08:03AM +0100, Ingo Molnar wrote: > * Joonsoo Kim wrote: > > How about following change and making 'perf kmem' print pfn? > > If we store pfn on the trace buffer, we can print $debugfs/tracing/trace > > as is and 'perf kmem' can also print pfn. > > > > Thanks. > > > > diff --git a/include/trace/events/kmem.h b/include/trace/events/kmem.h > > index 4ad10ba..9dcfd0b 100644 > > --- a/include/trace/events/kmem.h > > +++ b/include/trace/events/kmem.h > > @@ -199,22 +199,22 @@ TRACE_EVENT(mm_page_alloc, > > TP_ARGS(page, order, gfp_flags, migratetype), > > > > TP_STRUCT__entry( > > - __field( struct page *, page ) > > + __field( unsigned long, pfn ) > > __field( unsigned int, order ) > > __field( gfp_t, gfp_flags ) > > __field( int, migratetype ) > > ), > > > > TP_fast_assign( > > - __entry->page = page; > > + __entry->pfn = page ? page_to_pfn(page) : -1; > > __entry->order = order; > > __entry->gfp_flags = gfp_flags; > > __entry->migratetype = migratetype; > > ), > > > > TP_printk("page=%p pfn=%lu order=%d migratetype=%d gfp_flags=%s", > > - __entry->page, > > - __entry->page ? page_to_pfn(__entry->page) : 0, > > + __entry->pfn != -1 ? pfn_to_page(__entry->pfn) : NULL, > > + __entry->pfn != -1 ? __entry->pfn : 0, > > __entry->order, > > __entry->migratetype, > > show_gfp_flags(__entry->gfp_flags)) > > Acked-by: Ingo Molnar > > It would be very nice to make all the other page granular tracepoints > output pfn (which is a physical address that can be resolved to 'node' > and other properties), not 'struct page *' (which is a kernel resource > with little meaning to user-space tooling). > > I.e. the following tracepoints: > > triton:~/tip> git grep -E '__field.*struct page *' include/trace/ > include/trace/events/filemap.h: __field(struct page *, page) > include/trace/events/kmem.h: __field( struct page *, page ) > include/trace/events/kmem.h: __field( struct page *, page ) > include/trace/events/kmem.h: __field( struct page *, page ) > include/trace/events/kmem.h: __field( struct page *, page ) > include/trace/events/kmem.h: __field( struct page *, page ) > include/trace/events/pagemap.h: __field(struct page *, page ) > include/trace/events/pagemap.h: __field(struct page *, page ) > include/trace/events/vmscan.h: __field(struct page *, page) Okay, will do. > > there's very little breakage I can imagine: they have traced pointers > to 'struct page', which is a pretty opaque page identifier to > user-space, and they'll trace pfn's in the future, which still serves > as a page identifier. Agreed. > > One thing would be important: to do all these changes at once, to make > sure that the various page identifiers can be compared. OK > > Also, we might keep the 'page' field name if anything relies on that - > but 'pfn' is even better. Another option is to keep page field and add new pfn field. The events in pagemap.h already do this way. This will minimize the possible breakage but increase the trace size somewhat. Thanks, Namhyung