public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Andrew Morton <akpm@linux-foundation.org>, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] mm: trace filemap add and del
Date: Fri, 23 Nov 2012 11:15:35 +1100	[thread overview]
Message-ID: <20121123001535.GZ2591@dastard> (raw)
In-Reply-To: <87vccx500r.fsf@free.fr>

On Thu, Nov 22, 2012 at 12:51:00PM +0100, Robert Jarzmik wrote:
> Dave Chinner <david@fromorbit.com> writes:
> 
> > We actually have an informal convention for formating filesystem
> > trace events, and that is to use the device number....
> >
> >> 
> >> > +	),
> >> > +
> >> > +	TP_printk("page=%p pfn=%lu blk=%d:%d inode+ofs=%lu+%lu",
> >
> > ... and to prefix messages like:
> >
> > 	TP_printk("dev %d:%d ino 0x%llx ....
> > 		  MAJOR(__entry->dev), MINOR(__entry->dev),
> >
> > i.e. the start of the event message has all the identifying
> > information where it is easy to grep for and get all the events for
> > a specific dev/inode combination without even having to think about
> > it.
> 
> I cross-checked your proposition.
> The "ino 0x%llx" looks wrong to me, because :
>  - i_ino is "unsigned long", not "(unsigned) long long"

I just copied it from the XFS code, where the inode number is always
64 bits, even on 32 bit platforms.

>  - triggers a printk where "ino" looks really awfull (on a 32bits LE arm)
> >  mm_filemap_add_to_page_cache: dev 0:2 ino 0xc05186e000000000 page=000a0737
> >  pfn=0 ofs=3283861504

Then use the correct format specifier for a long. i.e. 0x%lx....

>  - why print the inode number in hexadecimal format ???

Because they are fair easier to parse and correlate by eye than
large decimal numbers.  Hexadecimal is frequently used because
boundaries in kernel and filesystem code typically fall on
powers-of-2 rather than powers-of-ten and hexadecimal makes it simple
to see those power-of-2 boundaries. Decimal tends to obfuscate
those boundaries, especially for large numbers....

e.g. with XFS inode numbers I can convert the inode number in
hex directly to it's location on disk in my head. e.g. decimal inode
numbers 137438980146 and 206158719668 don't look to have any real
meaning - they just look like large numbers. Convert them to hex and
you get 0x2000006802 and 0x3000046a14 repsectively.

For me, pattern recognition kicks in immediately and tells me that
the first inode is inode #2 at block offset 0x6800 in AG 32, and the
second is inode #20 at block 0x46a00 in AG 48, and I can immediately
correlate that to the filesystem geometry and storage layout and
know physically how far apart those inodes are.

IOWs, the inode numbers in hex are way more meaningful than in
decimal, and when I'm looking for needles in a haystack of
multi-gigabyte event traces (that's something I do every day), not
having to think about the relationship between two inodes is
extremely important....

>    Doing a "ls -i" returns decimal format, "debugfs" returns decimal. What is
>    the rational behind hexadecimal ?
> 
> I'd rather have : "dev %d:%d ino %lu page=0x%p pfn=%lu ofs=%lu".

Tracing is for debugging, therefore the format should be determined
by what makes life easier for those that are using the event traces
every day for debugging. Hence for anything that has defined power
of 2 boundaries like inode numbers, file offsets in the page cache,
etc, hexadecimal is much more meaningful to a programmer for
debugging purposes....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2012-11-23  0:15 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-08 19:54 [RFC PATCH] mm: trace filemap add and del Robert Jarzmik
2012-11-20 23:57 ` Andrew Morton
2012-11-21  3:59   ` Dave Chinner
2012-11-21 10:38     ` Robert Jarzmik
2012-11-22 11:51     ` Robert Jarzmik
2012-11-23  0:15       ` Dave Chinner [this message]
2012-11-21 10:36   ` Robert Jarzmik
2012-11-21  5:07 ` Hugh Dickins
2012-11-21 10:41   ` Robert Jarzmik
2012-11-23 14:10 ` [PATCH v2] " Robert Jarzmik
2012-11-23 23:34   ` [PATCH v3] " Robert Jarzmik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121123001535.GZ2591@dastard \
    --to=david@fromorbit.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robert.jarzmik@free.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox