2012-01-12 21:01:15

by Seiji Aguchi

[permalink] [raw]
Subject: [PATCH][RESEND]tracepoint: add drop_transaction/update_superblock_end to jbd2

Hello Ted,

Please review this patch adding jbd2 tracepoints(and hopefully apply to your tree).
This has already been accepted by Lukas and Jan.

Seiji

---
This patch adds trace_jbd2_drop_transaction and
trace_jbd2_update_superblock_end because there are similar tracepoints in jbd and
they are needed in jbd2 as well.


Signed-off-by: Seiji Aguchi <[email protected]>
Reviewed-by: Lukas Czerner <[email protected]>
Acked-by: Jan Kara <[email protected]>

---
fs/jbd2/checkpoint.c | 2 ++
fs/jbd2/journal.c | 2 ++
include/trace/events/jbd2.h | 28 ++++++++++++++++++++++++++++
3 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
index 16a698b..2bfd8b0 100644
--- a/fs/jbd2/checkpoint.c
+++ b/fs/jbd2/checkpoint.c
@@ -797,5 +797,7 @@ void __jbd2_journal_drop_transaction(journal_t *journal, transaction_t *transact
J_ASSERT(journal->j_committing_transaction != transaction);
J_ASSERT(journal->j_running_transaction != transaction);

+ trace_jbd2_drop_transaction(journal, transaction);
+
jbd_debug(1, "Dropping transaction %d, all done\n", transaction->t_tid);
}
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index f24df13..5953b3d 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1185,6 +1185,8 @@ void jbd2_journal_update_superblock(journal_t *journal, int wait)
} else
write_dirty_buffer(bh, WRITE);

+ trace_jbd2_update_superblock_end(journal, wait);
+
out:
/* If we have just flushed the log (by marking s_start==0), then
* any future commit will have to be careful to update the
diff --git a/include/trace/events/jbd2.h b/include/trace/events/jbd2.h
index 7596441..ae59bc2 100644
--- a/include/trace/events/jbd2.h
+++ b/include/trace/events/jbd2.h
@@ -81,6 +81,13 @@ DEFINE_EVENT(jbd2_commit, jbd2_commit_logging,
TP_ARGS(journal, commit_transaction)
);

+DEFINE_EVENT(jbd2_commit, jbd2_drop_transaction,
+
+ TP_PROTO(journal_t *journal, transaction_t *commit_transaction),
+
+ TP_ARGS(journal, commit_transaction)
+);
+
TRACE_EVENT(jbd2_end_commit,
TP_PROTO(journal_t *journal, transaction_t *commit_transaction),

@@ -229,6 +236,27 @@ TRACE_EVENT(jbd2_cleanup_journal_tail,
__entry->block_nr, __entry->freed)
);

+TRACE_EVENT(jbd2_update_superblock_end,
+
+ TP_PROTO(journal_t *journal, int wait),
+
+ TP_ARGS(journal, wait),
+
+ TP_STRUCT__entry(
+ __field( dev_t, dev )
+ __field( int, wait )
+ ),
+
+ TP_fast_assign(
+ __entry->dev = journal->j_fs_dev->bd_dev;
+ __entry->wait = wait;
+ ),
+
+ TP_printk("dev %d,%d wait %d",
+ MAJOR(__entry->dev), MINOR(__entry->dev),
+ __entry->wait)
+);
+
#endif /* _TRACE_JBD2_H */

/* This part must be outside protection */
--
1.7.1



2012-01-19 16:08:43

by Seiji Aguchi

[permalink] [raw]
Subject: RE: [PATCH][RESEND]tracepoint: add drop_transaction/update_superblock_end to jbd2

Ping?
________________________________________

Hello Ted,

Please review this patch adding jbd2 tracepoints(and hopefully apply to your tree).
This has already been accepted by Lukas and Jan.

Seiji

---
This patch adds trace_jbd2_drop_transaction and
trace_jbd2_update_superblock_end because there are similar tracepoints in jbd and
they are needed in jbd2 as well.


Signed-off-by: Seiji Aguchi <[email protected]>
Reviewed-by: Lukas Czerner <[email protected]>
Acked-by: Jan Kara <[email protected]>

---
fs/jbd2/checkpoint.c | 2 ++
fs/jbd2/journal.c | 2 ++
include/trace/events/jbd2.h | 28 ++++++++++++++++++++++++++++
3 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
index 16a698b..2bfd8b0 100644
--- a/fs/jbd2/checkpoint.c
+++ b/fs/jbd2/checkpoint.c
@@ -797,5 +797,7 @@ void __jbd2_journal_drop_transaction(journal_t *journal, transaction_t *transact
J_ASSERT(journal->j_committing_transaction != transaction);
J_ASSERT(journal->j_running_transaction != transaction);

+ trace_jbd2_drop_transaction(journal, transaction);
+
jbd_debug(1, "Dropping transaction %d, all done\n", transaction->t_tid);
}
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index f24df13..5953b3d 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1185,6 +1185,8 @@ void jbd2_journal_update_superblock(journal_t *journal, int wait)
} else
write_dirty_buffer(bh, WRITE);

+ trace_jbd2_update_superblock_end(journal, wait);
+
out:
/* If we have just flushed the log (by marking s_start==0), then
* any future commit will have to be careful to update the
diff --git a/include/trace/events/jbd2.h b/include/trace/events/jbd2.h
index 7596441..ae59bc2 100644
--- a/include/trace/events/jbd2.h
+++ b/include/trace/events/jbd2.h
@@ -81,6 +81,13 @@ DEFINE_EVENT(jbd2_commit, jbd2_commit_logging,
TP_ARGS(journal, commit_transaction)
);

+DEFINE_EVENT(jbd2_commit, jbd2_drop_transaction,
+
+ TP_PROTO(journal_t *journal, transaction_t *commit_transaction),
+
+ TP_ARGS(journal, commit_transaction)
+);
+
TRACE_EVENT(jbd2_end_commit,
TP_PROTO(journal_t *journal, transaction_t *commit_transaction),

@@ -229,6 +236,27 @@ TRACE_EVENT(jbd2_cleanup_journal_tail,
__entry->block_nr, __entry->freed)
);

+TRACE_EVENT(jbd2_update_superblock_end,
+
+ TP_PROTO(journal_t *journal, int wait),
+
+ TP_ARGS(journal, wait),
+
+ TP_STRUCT__entry(
+ __field( dev_t, dev )
+ __field( int, wait )
+ ),
+
+ TP_fast_assign(
+ __entry->dev = journal->j_fs_dev->bd_dev;
+ __entry->wait = wait;
+ ),
+
+ TP_printk("dev %d,%d wait %d",
+ MAJOR(__entry->dev), MINOR(__entry->dev),
+ __entry->wait)
+);
+
#endif /* _TRACE_JBD2_H */

/* This part must be outside protection */
--
1.7.1


2012-01-24 07:08:44

by Lukas Czerner

[permalink] [raw]
Subject: Re: [PATCH][RESEND]tracepoint: add drop_transaction/update_superblock_end to jbd2

On Thu, 12 Jan 2012, Seiji Aguchi wrote:

> Hello Ted,
>
> Please review this patch adding jbd2 tracepoints(and hopefully apply to your tree).
> This has already been accepted by Lukas and Jan.
>
> Seiji

Hi Ted,

can we get this thing in ? It has ACK and review so I do not thing there
is a reason to hold it back. Or do you have any problems with it ?

Thanks!
-Lukas

>
> ---
> This patch adds trace_jbd2_drop_transaction and
> trace_jbd2_update_superblock_end because there are similar tracepoints in jbd and
> they are needed in jbd2 as well.
>
>
> Signed-off-by: Seiji Aguchi <[email protected]>
> Reviewed-by: Lukas Czerner <[email protected]>
> Acked-by: Jan Kara <[email protected]>
>
> ---
> fs/jbd2/checkpoint.c | 2 ++
> fs/jbd2/journal.c | 2 ++
> include/trace/events/jbd2.h | 28 ++++++++++++++++++++++++++++
> 3 files changed, 32 insertions(+), 0 deletions(-)
>
> diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
> index 16a698b..2bfd8b0 100644
> --- a/fs/jbd2/checkpoint.c
> +++ b/fs/jbd2/checkpoint.c
> @@ -797,5 +797,7 @@ void __jbd2_journal_drop_transaction(journal_t *journal, transaction_t *transact
> J_ASSERT(journal->j_committing_transaction != transaction);
> J_ASSERT(journal->j_running_transaction != transaction);
>
> + trace_jbd2_drop_transaction(journal, transaction);
> +
> jbd_debug(1, "Dropping transaction %d, all done\n", transaction->t_tid);
> }
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index f24df13..5953b3d 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -1185,6 +1185,8 @@ void jbd2_journal_update_superblock(journal_t *journal, int wait)
> } else
> write_dirty_buffer(bh, WRITE);
>
> + trace_jbd2_update_superblock_end(journal, wait);
> +
> out:
> /* If we have just flushed the log (by marking s_start==0), then
> * any future commit will have to be careful to update the
> diff --git a/include/trace/events/jbd2.h b/include/trace/events/jbd2.h
> index 7596441..ae59bc2 100644
> --- a/include/trace/events/jbd2.h
> +++ b/include/trace/events/jbd2.h
> @@ -81,6 +81,13 @@ DEFINE_EVENT(jbd2_commit, jbd2_commit_logging,
> TP_ARGS(journal, commit_transaction)
> );
>
> +DEFINE_EVENT(jbd2_commit, jbd2_drop_transaction,
> +
> + TP_PROTO(journal_t *journal, transaction_t *commit_transaction),
> +
> + TP_ARGS(journal, commit_transaction)
> +);
> +
> TRACE_EVENT(jbd2_end_commit,
> TP_PROTO(journal_t *journal, transaction_t *commit_transaction),
>
> @@ -229,6 +236,27 @@ TRACE_EVENT(jbd2_cleanup_journal_tail,
> __entry->block_nr, __entry->freed)
> );
>
> +TRACE_EVENT(jbd2_update_superblock_end,
> +
> + TP_PROTO(journal_t *journal, int wait),
> +
> + TP_ARGS(journal, wait),
> +
> + TP_STRUCT__entry(
> + __field( dev_t, dev )
> + __field( int, wait )
> + ),
> +
> + TP_fast_assign(
> + __entry->dev = journal->j_fs_dev->bd_dev;
> + __entry->wait = wait;
> + ),
> +
> + TP_printk("dev %d,%d wait %d",
> + MAJOR(__entry->dev), MINOR(__entry->dev),
> + __entry->wait)
> +);
> +
> #endif /* _TRACE_JBD2_H */
>
> /* This part must be outside protection */
>

--

2012-01-30 06:21:39

by Lukas Czerner

[permalink] [raw]
Subject: Re: [PATCH][RESEND]tracepoint: add drop_transaction/update_superblock_end to jbd2

On Tue, 24 Jan 2012, Lukas Czerner wrote:

> On Thu, 12 Jan 2012, Seiji Aguchi wrote:
>
> > Hello Ted,
> >
> > Please review this patch adding jbd2 tracepoints(and hopefully apply to your tree).
> > This has already been accepted by Lukas and Jan.
> >
> > Seiji
>
> Hi Ted,
>
> can we get this thing in ? It has ACK and review so I do not thing there
> is a reason to hold it back. Or do you have any problems with it ?
>
> Thanks!
> -Lukas

Ping!

-Lukas

>
> >
> > ---
> > This patch adds trace_jbd2_drop_transaction and
> > trace_jbd2_update_superblock_end because there are similar tracepoints in jbd and
> > they are needed in jbd2 as well.
> >
> >
> > Signed-off-by: Seiji Aguchi <[email protected]>
> > Reviewed-by: Lukas Czerner <[email protected]>
> > Acked-by: Jan Kara <[email protected]>
> >
> > ---
> > fs/jbd2/checkpoint.c | 2 ++
> > fs/jbd2/journal.c | 2 ++
> > include/trace/events/jbd2.h | 28 ++++++++++++++++++++++++++++
> > 3 files changed, 32 insertions(+), 0 deletions(-)
> >
> > diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
> > index 16a698b..2bfd8b0 100644
> > --- a/fs/jbd2/checkpoint.c
> > +++ b/fs/jbd2/checkpoint.c
> > @@ -797,5 +797,7 @@ void __jbd2_journal_drop_transaction(journal_t *journal, transaction_t *transact
> > J_ASSERT(journal->j_committing_transaction != transaction);
> > J_ASSERT(journal->j_running_transaction != transaction);
> >
> > + trace_jbd2_drop_transaction(journal, transaction);
> > +
> > jbd_debug(1, "Dropping transaction %d, all done\n", transaction->t_tid);
> > }
> > diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> > index f24df13..5953b3d 100644
> > --- a/fs/jbd2/journal.c
> > +++ b/fs/jbd2/journal.c
> > @@ -1185,6 +1185,8 @@ void jbd2_journal_update_superblock(journal_t *journal, int wait)
> > } else
> > write_dirty_buffer(bh, WRITE);
> >
> > + trace_jbd2_update_superblock_end(journal, wait);
> > +
> > out:
> > /* If we have just flushed the log (by marking s_start==0), then
> > * any future commit will have to be careful to update the
> > diff --git a/include/trace/events/jbd2.h b/include/trace/events/jbd2.h
> > index 7596441..ae59bc2 100644
> > --- a/include/trace/events/jbd2.h
> > +++ b/include/trace/events/jbd2.h
> > @@ -81,6 +81,13 @@ DEFINE_EVENT(jbd2_commit, jbd2_commit_logging,
> > TP_ARGS(journal, commit_transaction)
> > );
> >
> > +DEFINE_EVENT(jbd2_commit, jbd2_drop_transaction,
> > +
> > + TP_PROTO(journal_t *journal, transaction_t *commit_transaction),
> > +
> > + TP_ARGS(journal, commit_transaction)
> > +);
> > +
> > TRACE_EVENT(jbd2_end_commit,
> > TP_PROTO(journal_t *journal, transaction_t *commit_transaction),
> >
> > @@ -229,6 +236,27 @@ TRACE_EVENT(jbd2_cleanup_journal_tail,
> > __entry->block_nr, __entry->freed)
> > );
> >
> > +TRACE_EVENT(jbd2_update_superblock_end,
> > +
> > + TP_PROTO(journal_t *journal, int wait),
> > +
> > + TP_ARGS(journal, wait),
> > +
> > + TP_STRUCT__entry(
> > + __field( dev_t, dev )
> > + __field( int, wait )
> > + ),
> > +
> > + TP_fast_assign(
> > + __entry->dev = journal->j_fs_dev->bd_dev;
> > + __entry->wait = wait;
> > + ),
> > +
> > + TP_printk("dev %d,%d wait %d",
> > + MAJOR(__entry->dev), MINOR(__entry->dev),
> > + __entry->wait)
> > +);
> > +
> > #endif /* _TRACE_JBD2_H */
> >
> > /* This part must be outside protection */
> >
>
>

--

2012-02-06 20:02:16

by Seiji Aguchi

[permalink] [raw]
Subject: RE: [PATCH][RESEND]tracepoint: add drop_transaction/update_superblock_end to jbd2

Hello Ted,

I explain the reason why this patch is needed.
Tracepoints of drop_transaction and update_superblock are missing from jbd2 even though
They are in jbd.

When our customers migrate their file system from ext3 to ext4 and some issues happen in jbd2,
we may not diagnose it even though we could do it in ext3 system.

It seems like a regression for our customers.

This patch is important for us.
Please give us some feedback.

Seiji

-----Original Message-----
From: Lukas Czerner [mailto:[email protected]]
Sent: Monday, January 30, 2012 1:21 AM
To: Lukas Czerner
Cc: Seiji Aguchi; [email protected]; [email protected]; [email protected]; [email protected]; Satoru Moriya
Subject: Re: [PATCH][RESEND]tracepoint: add drop_transaction/update_superblock_end to jbd2

On Tue, 24 Jan 2012, Lukas Czerner wrote:

> On Thu, 12 Jan 2012, Seiji Aguchi wrote:
>
> > Hello Ted,
> >
> > Please review this patch adding jbd2 tracepoints(and hopefully apply to your tree).
> > This has already been accepted by Lukas and Jan.
> >
> > Seiji
>
> Hi Ted,
>
> can we get this thing in ? It has ACK and review so I do not thing
> there is a reason to hold it back. Or do you have any problems with it ?
>
> Thanks!
> -Lukas

Ping!

-Lukas

>
> >
> > ---
> > This patch adds trace_jbd2_drop_transaction and
> > trace_jbd2_update_superblock_end because there are similar
> > tracepoints in jbd and they are needed in jbd2 as well.
> >
> >
> > Signed-off-by: Seiji Aguchi <[email protected]>
> > Reviewed-by: Lukas Czerner <[email protected]>
> > Acked-by: Jan Kara <[email protected]>
> >
> > ---
> > fs/jbd2/checkpoint.c | 2 ++
> > fs/jbd2/journal.c | 2 ++
> > include/trace/events/jbd2.h | 28 ++++++++++++++++++++++++++++
> > 3 files changed, 32 insertions(+), 0 deletions(-)
> >
> > diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c index
> > 16a698b..2bfd8b0 100644
> > --- a/fs/jbd2/checkpoint.c
> > +++ b/fs/jbd2/checkpoint.c
> > @@ -797,5 +797,7 @@ void __jbd2_journal_drop_transaction(journal_t *journal, transaction_t *transact
> > J_ASSERT(journal->j_committing_transaction != transaction);
> > J_ASSERT(journal->j_running_transaction != transaction);
> >
> > + trace_jbd2_drop_transaction(journal, transaction);
> > +
> > jbd_debug(1, "Dropping transaction %d, all done\n",
> > transaction->t_tid); } diff --git a/fs/jbd2/journal.c
> > b/fs/jbd2/journal.c index f24df13..5953b3d 100644
> > --- a/fs/jbd2/journal.c
> > +++ b/fs/jbd2/journal.c
> > @@ -1185,6 +1185,8 @@ void jbd2_journal_update_superblock(journal_t *journal, int wait)
> > } else
> > write_dirty_buffer(bh, WRITE);
> >
> > + trace_jbd2_update_superblock_end(journal, wait);
> > +
> > out:
> > /* If we have just flushed the log (by marking s_start==0), then
> > * any future commit will have to be careful to update the diff
> > --git a/include/trace/events/jbd2.h b/include/trace/events/jbd2.h
> > index 7596441..ae59bc2 100644
> > --- a/include/trace/events/jbd2.h
> > +++ b/include/trace/events/jbd2.h
> > @@ -81,6 +81,13 @@ DEFINE_EVENT(jbd2_commit, jbd2_commit_logging,
> > TP_ARGS(journal, commit_transaction) );
> >
> > +DEFINE_EVENT(jbd2_commit, jbd2_drop_transaction,
> > +
> > + TP_PROTO(journal_t *journal, transaction_t *commit_transaction),
> > +
> > + TP_ARGS(journal, commit_transaction) );
> > +
> > TRACE_EVENT(jbd2_end_commit,
> > TP_PROTO(journal_t *journal, transaction_t *commit_transaction),
> >
> > @@ -229,6 +236,27 @@ TRACE_EVENT(jbd2_cleanup_journal_tail,
> > __entry->block_nr, __entry->freed) );
> >
> > +TRACE_EVENT(jbd2_update_superblock_end,
> > +
> > + TP_PROTO(journal_t *journal, int wait),
> > +
> > + TP_ARGS(journal, wait),
> > +
> > + TP_STRUCT__entry(
> > + __field( dev_t, dev )
> > + __field( int, wait )
> > + ),
> > +
> > + TP_fast_assign(
> > + __entry->dev = journal->j_fs_dev->bd_dev;
> > + __entry->wait = wait;
> > + ),
> > +
> > + TP_printk("dev %d,%d wait %d",
> > + MAJOR(__entry->dev), MINOR(__entry->dev),
> > + __entry->wait)
> > +);
> > +
> > #endif /* _TRACE_JBD2_H */
> >
> > /* This part must be outside protection */
> >
>
>

--