linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Theodore Ts'o <tytso@mit.edu>
To: Jan Kara <jack@suse.cz>
Cc: Ext4 Developers List <linux-ext4@vger.kernel.org>
Subject: Re: [PATCH 05/12] ext4: pass context information to jbd2__journal_start()
Date: Mon, 11 Feb 2013 15:14:41 -0500	[thread overview]
Message-ID: <20130211201441.GA14005@thunk.org> (raw)
In-Reply-To: <20130211195855.GB14172@quack.suse.cz>

On Mon, Feb 11, 2013 at 08:58:55PM +0100, Jan Kara wrote:
> > Another downside of using a pointer to __FILE__, or some static
> > character pointer, is that it doesn't work for the applications such
> > as perf (and I think ftrace, although I'm not certain about the ftrace
> > tool, since I never use it) can't handle printing the char *, since
> > they use the ring buffer syscall directly instead of using the
> > TP_printk statement.  So all they get is a kernel address, and
> > something which is useful to decode.
>
>   One can overcome this relatively easily by defining the trace
> event to have __array(char, file, 32) and then strcpy() the name to is in
> TP_fast_assign(). We do this in lot of other trace points as well (with
> device names etc.).

True, but then the tradeoff is that we've bloated the size of the
tracepoint in the ring buffer.  Given how many handles can be created
and closed per second, keeping the tracepoint size small is also
desireable.  Of course one thing we could do is use a small integer,
and then have a sysfs interface which maps the integer to the file
name.   That may be overkill, though.

> 
> > But for users who are using the /sys/kernel/debug/tracing interface
> > via echo and cat, I do agree that using a static char *JBD_FILE so
> > that /sys/kernel/debug/trace has real file names would be a bit more
> > convenient for them.
>   Convenience of use is one thing, having programmer properly assign new
> handle types (or use old ones) is another. And you also have questions like
> - there are three call sites with this handle type but I cannot figure out
> which of them is causing problems.

Well, that's what the line number is for (so you can distinguish the
call site).  The other reason why using a type separate from the file
name is sometimes the best way to classify different types of handle
operations isn't necessarily by file name.  As far as the effort of
programmers to assign new handle types, I don't really anticipate that
this will be a common thing; it's pretty rare that we introduce code
which requires new calls to ext4_journal_start/ext4_journal_stop.  And
when programmers do need to do this, the question of which handle type
to use or whether to create a new handle type is a pretty minor issue.
The much greater difficulty is figuring out how many journal credits
to reserve.  :-)

						- Ted

  reply	other threads:[~2013-02-11 20:14 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-09 21:53 [PATCH 00/12] jbd2 optimization and bug fixes Theodore Ts'o
2013-02-09 21:53 ` [PATCH 01/12] jbd2: track request delay statistics Theodore Ts'o
2013-02-11 15:57   ` Jan Kara
2013-02-09 21:53 ` [PATCH 02/12] jbd2: revert "jbd2: add COW fields to struct jbd2_journal_handle" Theodore Ts'o
2013-02-11 15:58   ` Jan Kara
2013-02-09 21:53 ` [PATCH 03/12] jbd2: add tracepoints which provide per-handle statistics Theodore Ts'o
2013-02-09 21:53 ` [PATCH 04/12] ext4: move the jbd2 wrapper functions out of super.c Theodore Ts'o
2013-02-09 21:53 ` [PATCH 05/12] ext4: pass context information to jbd2__journal_start() Theodore Ts'o
2013-02-11 16:16   ` Jan Kara
2013-02-11 18:13     ` Theodore Ts'o
2013-02-11 19:58       ` Jan Kara
2013-02-11 20:14         ` Theodore Ts'o [this message]
2013-02-09 21:53 ` [PATCH 06/12] ext4: grab page before starting transaction handle in write_begin() Theodore Ts'o
2013-02-11 16:35   ` Jan Kara
2013-02-09 21:53 ` [PATCH 07/12] ext4: start handle at the last possible moment in ext4_unlink() Theodore Ts'o
2013-02-11 16:21   ` Jan Kara
2013-02-09 21:53 ` [PATCH 08/12] ext4: start handle at the last possible moment in ext4_rmdir() Theodore Ts'o
2013-02-11 16:22   ` Jan Kara
2013-02-09 21:53 ` [PATCH 09/12] ext4: fix the number of credits needed for ext4_ext_migrate() Theodore Ts'o
2013-02-11 16:26   ` Jan Kara
2013-02-09 21:53 ` [PATCH 10/12] ext4: fix the number of credits needed for ext4_unlink() and ext4_rmdir() Theodore Ts'o
2013-02-11 16:28   ` Jan Kara
2013-02-11 18:30     ` Theodore Ts'o
2013-02-11 19:30       ` Jan Kara
2013-02-09 21:53 ` [PATCH 11/12] ext4: fix the number of credits needed for acl ops with inline data Theodore Ts'o
2013-02-10 13:42   ` Tao Ma
2013-02-10 18:15     ` Shentino
2013-02-10 19:43       ` Theodore Ts'o
2013-02-11 16:16         ` Andreas Dilger
2013-02-11 16:30   ` Jan Kara
2013-02-09 21:53 ` [PATCH 12/12] ext4: start handle at the last possible moment when creating inodes Theodore Ts'o
2013-02-11  1:47   ` Theodore Ts'o
2013-02-11 16:00   ` Andreas Dilger
2013-02-11 15:52 ` [PATCH 00/12] jbd2 optimization and bug fixes Jan Kara

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=20130211201441.GA14005@thunk.org \
    --to=tytso@mit.edu \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).