diff mbox series

[4/4] lightnvm: pblk: add tracing for chunk resets

Message ID 1535545877-770-5-git-send-email-hans.ml.holmberg@owltronix.com (mailing list archive)
State New, archived
Headers show
Series Introduce trace events for pblk | expand

Commit Message

Hans Holmberg Aug. 29, 2018, 12:31 p.m. UTC
From: Hans Holmberg <hans.holmberg@cnexlabs.com>

Trace state of chunk resets.

Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
---
 drivers/lightnvm/pblk-core.c  | 12 ++++++++++++
 drivers/lightnvm/pblk-trace.h | 31 +++++++++++++++++++++++++++++++
 drivers/lightnvm/pblk.h       |  6 ++++++
 3 files changed, 49 insertions(+)

Comments

Javier Gonzalez Aug. 29, 2018, 8:09 p.m. UTC | #1
> On 29 Aug 2018, at 14.31, Hans Holmberg <hans.ml.holmberg@owltronix.com> wrote:
> 
> From: Hans Holmberg <hans.holmberg@cnexlabs.com>
> 
> Trace state of chunk resets.
> 
> Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> ---
> drivers/lightnvm/pblk-core.c  | 12 ++++++++++++
> drivers/lightnvm/pblk-trace.h | 31 +++++++++++++++++++++++++++++++
> drivers/lightnvm/pblk.h       |  6 ++++++
> 3 files changed, 49 insertions(+)
> 
> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
> index 566a5d3165ee..b874001d1175 100644
> --- a/drivers/lightnvm/pblk-core.c
> +++ b/drivers/lightnvm/pblk-core.c
> @@ -90,9 +90,15 @@ static void __pblk_end_io_erase(struct pblk *pblk, struct nvm_rq *rqd)
> 	atomic_dec(&line->left_seblks);
> 
> 	if (rqd->error) {
> +		trace_pblk_chunk_reset(pblk_disk_name(pblk),
> +				&rqd->ppa_addr, PBLK_CHUNK_RESET_FAILED);
> +
> 		chunk->state = NVM_CHK_ST_OFFLINE;
> 		pblk_mark_bb(pblk, line, rqd->ppa_addr);
> 	} else {
> +		trace_pblk_chunk_reset(pblk_disk_name(pblk),
> +				&rqd->ppa_addr, PBLK_CHUNK_RESET_DONE);
> +
> 		chunk->state = NVM_CHK_ST_FREE;
> 	}
> 
> @@ -958,6 +964,9 @@ int pblk_line_erase(struct pblk *pblk, struct pblk_line *line)
> 		WARN_ON(test_and_set_bit(bit, line->erase_bitmap));
> 		spin_unlock(&line->lock);
> 
> +		trace_pblk_chunk_reset(pblk_disk_name(pblk),
> +				&ppa, PBLK_CHUNK_RESET_START);
> +

Can you move this inside of pblk_blk_erase_sync() instead? I was looking
at the code now to implement a different thing and it is confusing to
see tracing inside erase_async and not in erase_sync.

Thanks,
Javier
Hans Holmberg Sept. 4, 2018, 10:39 a.m. UTC | #2
On Wed, Aug 29, 2018 at 10:09 PM Javier Gonzalez <javier@cnexlabs.com> wrote:
>
> > On 29 Aug 2018, at 14.31, Hans Holmberg <hans.ml.holmberg@owltronix.com> wrote:
> >
> > From: Hans Holmberg <hans.holmberg@cnexlabs.com>
> >
> > Trace state of chunk resets.
> >
> > Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> > ---
> > drivers/lightnvm/pblk-core.c  | 12 ++++++++++++
> > drivers/lightnvm/pblk-trace.h | 31 +++++++++++++++++++++++++++++++
> > drivers/lightnvm/pblk.h       |  6 ++++++
> > 3 files changed, 49 insertions(+)
> >
> > diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
> > index 566a5d3165ee..b874001d1175 100644
> > --- a/drivers/lightnvm/pblk-core.c
> > +++ b/drivers/lightnvm/pblk-core.c
> > @@ -90,9 +90,15 @@ static void __pblk_end_io_erase(struct pblk *pblk, struct nvm_rq *rqd)
> >       atomic_dec(&line->left_seblks);
> >
> >       if (rqd->error) {
> > +             trace_pblk_chunk_reset(pblk_disk_name(pblk),
> > +                             &rqd->ppa_addr, PBLK_CHUNK_RESET_FAILED);
> > +
> >               chunk->state = NVM_CHK_ST_OFFLINE;
> >               pblk_mark_bb(pblk, line, rqd->ppa_addr);
> >       } else {
> > +             trace_pblk_chunk_reset(pblk_disk_name(pblk),
> > +                             &rqd->ppa_addr, PBLK_CHUNK_RESET_DONE);
> > +
> >               chunk->state = NVM_CHK_ST_FREE;
> >       }
> >
> > @@ -958,6 +964,9 @@ int pblk_line_erase(struct pblk *pblk, struct pblk_line *line)
> >               WARN_ON(test_and_set_bit(bit, line->erase_bitmap));
> >               spin_unlock(&line->lock);
> >
> > +             trace_pblk_chunk_reset(pblk_disk_name(pblk),
> > +                             &ppa, PBLK_CHUNK_RESET_START);
> > +
>
> Can you move this inside of pblk_blk_erase_sync() instead? I was looking
> at the code now to implement a different thing and it is confusing to
> see tracing inside erase_async and not in erase_sync.
>

Fixed in a V2. Thanks.
diff mbox series

Patch

diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index 566a5d3165ee..b874001d1175 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -90,9 +90,15 @@  static void __pblk_end_io_erase(struct pblk *pblk, struct nvm_rq *rqd)
 	atomic_dec(&line->left_seblks);
 
 	if (rqd->error) {
+		trace_pblk_chunk_reset(pblk_disk_name(pblk),
+				&rqd->ppa_addr, PBLK_CHUNK_RESET_FAILED);
+
 		chunk->state = NVM_CHK_ST_OFFLINE;
 		pblk_mark_bb(pblk, line, rqd->ppa_addr);
 	} else {
+		trace_pblk_chunk_reset(pblk_disk_name(pblk),
+				&rqd->ppa_addr, PBLK_CHUNK_RESET_DONE);
+
 		chunk->state = NVM_CHK_ST_FREE;
 	}
 
@@ -958,6 +964,9 @@  int pblk_line_erase(struct pblk *pblk, struct pblk_line *line)
 		WARN_ON(test_and_set_bit(bit, line->erase_bitmap));
 		spin_unlock(&line->lock);
 
+		trace_pblk_chunk_reset(pblk_disk_name(pblk),
+				&ppa, PBLK_CHUNK_RESET_START);
+
 		ret = pblk_blk_erase_sync(pblk, ppa);
 		if (ret) {
 			pblk_err(pblk, "failed to erase line %d\n", line->id);
@@ -1736,6 +1745,9 @@  int pblk_blk_erase_async(struct pblk *pblk, struct ppa_addr ppa)
 	rqd->end_io = pblk_end_io_erase;
 	rqd->private = pblk;
 
+	trace_pblk_chunk_reset(pblk_disk_name(pblk),
+				&ppa, PBLK_CHUNK_RESET_START);
+
 	/* The write thread schedules erases so that it minimizes disturbances
 	 * with writes. Thus, there is no need to take the LUN semaphore.
 	 */
diff --git a/drivers/lightnvm/pblk-trace.h b/drivers/lightnvm/pblk-trace.h
index c171d0450c07..679e5c458ca6 100644
--- a/drivers/lightnvm/pblk-trace.h
+++ b/drivers/lightnvm/pblk-trace.h
@@ -31,6 +31,37 @@  struct ppa_addr;
 	{ PBLK_STATE_RECOVERING,	"RECOVERING",	},	\
 	{ PBLK_STATE_STOPPED,		"STOPPED"	})
 
+#define show_chunk_erase_state(state) __print_symbolic(state,	\
+	{ PBLK_CHUNK_RESET_START,	"START",	},	\
+	{ PBLK_CHUNK_RESET_DONE,	"OK",		},	\
+	{ PBLK_CHUNK_RESET_FAILED,	"FAILED"	})
+
+
+TRACE_EVENT(pblk_chunk_reset,
+
+	TP_PROTO(const char *name, struct ppa_addr *ppa, int state),
+
+	TP_ARGS(name, ppa, state),
+
+	TP_STRUCT__entry(
+		__string(name, name)
+		__field(u64, ppa)
+		__field(int, state);
+	),
+
+	TP_fast_assign(
+		__assign_str(name, name);
+		__entry->ppa = ppa->ppa;
+		__entry->state = state;
+	),
+
+	TP_printk("dev=%s grp=%llu pu=%llu chk=%llu state=%s", __get_str(name),
+			(u64)(((struct ppa_addr *)(&__entry->ppa))->m.grp),
+			(u64)(((struct ppa_addr *)(&__entry->ppa))->m.pu),
+			(u64)(((struct ppa_addr *)(&__entry->ppa))->m.chk),
+			show_chunk_erase_state((int)__entry->state))
+
+);
 
 TRACE_EVENT(pblk_chunk_state,
 
diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index ed17db2cae2c..6195e3f5d2e6 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -79,6 +79,12 @@  enum {
 	PBLK_BLK_ST_CLOSED =	0x2,
 };
 
+enum {
+	PBLK_CHUNK_RESET_START,
+	PBLK_CHUNK_RESET_DONE,
+	PBLK_CHUNK_RESET_FAILED,
+};
+
 struct pblk_sec_meta {
 	u64 reserved;
 	__le64 lba;