[PATCH] Add a tracepoint for block request remapping

[PATCH] Add a tracepoint for block request remapping

[Jun'ichi Nomura]

# Re-sending just the core bit.

Since 2.6.31 now has request-based device-mapper, it's useful to have
a tracepoint for request-remapping as well as bio-remapping.
This patch adds a tracepoint for request-remapping, trace_block_rq_remap().

Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Cc: Jens Axboe <jens.axboe@oracle.com>
Cc: Alasdair G Kergon <agk@redhat.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>
---
 block/blk-core.c             |    1 +
 include/trace/events/block.h |   33 +++++++++++++++++++++++++++++++++
 kernel/trace/blktrace.c      |   34 ++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+)

Index: linux-2.6.31.work/block/blk-core.c
===================================================================
--- linux-2.6.31.work.orig/block/blk-core.c
+++ linux-2.6.31.work/block/blk-core.c
@@ -34,6 +34,7 @@
 #include "blk.h"

 EXPORT_TRACEPOINT_SYMBOL_GPL(block_remap);
+EXPORT_TRACEPOINT_SYMBOL_GPL(block_rq_remap);
 EXPORT_TRACEPOINT_SYMBOL_GPL(block_bio_complete);

 static int __make_request(struct request_queue *q, struct bio *bio);
Index: linux-2.6.31.work/include/trace/events/block.h
===================================================================
--- linux-2.6.31.work.orig/include/trace/events/block.h
+++ linux-2.6.31.work/include/trace/events/block.h
@@ -486,6 +486,39 @@ TRACE_EVENT(block_remap,
 		  (unsigned long long)__entry->old_sector)
 );

+TRACE_EVENT(block_rq_remap,
+
+	TP_PROTO(struct request_queue *q, struct request *rq, dev_t dev,
+		 sector_t from),
+
+	TP_ARGS(q, rq, dev, from),
+
+	TP_STRUCT__entry(
+		__field( dev_t,		dev		)
+		__field( sector_t,	sector		)
+		__field( unsigned int,	nr_sector	)
+		__field( dev_t,		old_dev		)
+		__field( sector_t,	old_sector	)
+		__array( char,		rwbs,	6	)
+	),
+
+	TP_fast_assign(
+		__entry->dev		= disk_devt(rq->rq_disk);
+		__entry->sector		= blk_rq_pos(rq);
+		__entry->nr_sector	= blk_rq_sectors(rq);
+		__entry->old_dev	= dev;
+		__entry->old_sector	= from;
+		blk_fill_rwbs_rq(__entry->rwbs, rq);
+	),
+
+	TP_printk("%d,%d %s %llu + %u <- (%d,%d) %llu",
+		  MAJOR(__entry->dev), MINOR(__entry->dev), __entry->rwbs,
+		  (unsigned long long)__entry->sector,
+		  __entry->nr_sector,
+		  MAJOR(__entry->old_dev), MINOR(__entry->old_dev),
+		  (unsigned long long)__entry->old_sector)
+);
+
 #endif /* _TRACE_BLOCK_H */

 /* This part must be outside protection */
Index: linux-2.6.31.work/kernel/trace/blktrace.c
===================================================================
--- linux-2.6.31.work.orig/kernel/trace/blktrace.c
+++ linux-2.6.31.work/kernel/trace/blktrace.c
@@ -852,6 +852,37 @@ static void blk_add_trace_remap(struct r
 }

 /**
+ * blk_add_trace_rq_remap - Add a trace for a request-remap operation
+ * @q:		queue the io is for
+ * @rq:		the source request
+ * @dev:	target device
+ * @from:	source sector
+ *
+ * Description:
+ *     Device mapper remaps request to other devices.
+ *     Add a trace for that action.
+ *
+ **/
+static void blk_add_trace_rq_remap(struct request_queue *q,
+				   struct request *rq, dev_t dev,
+				   sector_t from)
+{
+	struct blk_trace *bt = q->blk_trace;
+	struct blk_io_trace_remap r;
+
+	if (likely(!bt))
+		return;
+
+	r.device_from = cpu_to_be32(dev);
+	r.device_to   = cpu_to_be32(disk_devt(rq->rq_disk));
+	r.sector_from = cpu_to_be64(from);
+
+	__blk_add_trace(bt, blk_rq_pos(rq), blk_rq_bytes(rq),
+			rq_data_dir(rq), BLK_TA_REMAP, !!rq->errors,
+			sizeof(r), &r);
+}
+
+/**
  * blk_add_driver_data - Add binary message with driver-specific data
  * @q:		queue the io is for
  * @rq:		io request
@@ -918,10 +949,13 @@ static void blk_register_tracepoints(voi
 	WARN_ON(ret);
 	ret = register_trace_block_remap(blk_add_trace_remap);
 	WARN_ON(ret);
+	ret = register_trace_block_rq_remap(blk_add_trace_rq_remap);
+	WARN_ON(ret);
 }

 static void blk_unregister_tracepoints(void)
 {
+	unregister_trace_block_rq_remap(blk_add_trace_rq_remap);
 	unregister_trace_block_remap(blk_add_trace_remap);
 	unregister_trace_block_split(blk_add_trace_split);
 	unregister_trace_block_unplug_io(blk_add_trace_unplug_io);

Re: [PATCH] Add a tracepoint for block request remapping

[Jens Axboe]

On Tue, Sep 22 2009, Jun'ichi Nomura wrote:
> # Re-sending just the core bit.
> 
> Since 2.6.31 now has request-based device-mapper, it's useful to have
> a tracepoint for request-remapping as well as bio-remapping.
> This patch adds a tracepoint for request-remapping, trace_block_rq_remap().

Applied to the upstream for-linus branch (for 2.6.32).

-- 
Jens Axboe

[PATCH] Add a tracepoint for block request remapping

[Jun'ichi Nomura]

Since 2.6.31 now has request-based device-mapper, it's useful to have
a tracepoint for request-remapping as well as bio-remapping.

This patch adds a tracepoint for request-remapping, trace_block_rq_remap().
Existing trace_block_remap() is left unchanged but it might be better to
rename it to trace_block_bio_remap().

Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Cc: Jens Axboe <jens.axboe@oracle.com>
Cc: Alasdair G Kergon <agk@redhat.com>
---
 block/blk-core.c             |   11 +++++-----
 drivers/md/dm.c              |    6 +++--
 include/trace/events/block.h |   35 +++++++++++++++++++++++++++++++++-
 kernel/trace/blktrace.c      |   44 ++++++++++++++++++++++++++++++++++++++-----
 4 files changed, 83 insertions(+), 13 deletions(-)

Index: linux-2.6.31/block/blk-core.c
===================================================================
--- linux-2.6.31.orig/block/blk-core.c
+++ linux-2.6.31/block/blk-core.c
@@ -34,6 +34,7 @@
 #include "blk.h"
 
 EXPORT_TRACEPOINT_SYMBOL_GPL(block_remap);
+EXPORT_TRACEPOINT_SYMBOL_GPL(block_rq_remap);
 EXPORT_TRACEPOINT_SYMBOL_GPL(block_bio_complete);
 
 static int __make_request(struct request_queue *q, struct bio *bio);
Index: linux-2.6.31/drivers/md/dm.c
===================================================================
--- linux-2.6.31.orig/drivers/md/dm.c
+++ linux-2.6.31/drivers/md/dm.c
@@ -1503,6 +1503,8 @@ static void map_request(struct dm_target
 		break;
 	case DM_MAPIO_REMAPPED:
 		/* The target has remapped the I/O so dispatch it */
+		trace_block_rq_remap(clone->q, clone, disk_devt(dm_disk(md)),
+				     blk_rq_pos(tio->orig));
 		dm_dispatch_request(clone);
 		break;
 	case DM_MAPIO_REQUEUE:
Index: linux-2.6.31/include/trace/events/block.h
===================================================================
--- linux-2.6.31.orig/include/trace/events/block.h
+++ linux-2.6.31/include/trace/events/block.h
@@ -486,6 +486,39 @@ TRACE_EVENT(block_remap,
 		  (unsigned long long)__entry->old_sector)
 );
 
+TRACE_EVENT(block_rq_remap,
+
+	TP_PROTO(struct request_queue *q, struct request *rq, dev_t dev,
+		 sector_t from),
+
+	TP_ARGS(q, rq, dev, from),
+
+	TP_STRUCT__entry(
+		__field( dev_t,		dev		)
+		__field( sector_t,	sector		)
+		__field( unsigned int,	nr_sector	)
+		__field( dev_t,		old_dev		)
+		__field( sector_t,	old_sector	)
+		__array( char,		rwbs,	6	)
+	),
+
+	TP_fast_assign(
+		__entry->dev		= disk_devt(rq->rq_disk);
+		__entry->sector		= blk_rq_pos(rq);
+		__entry->nr_sector	= blk_rq_sectors(rq);
+		__entry->old_dev	= dev;
+		__entry->old_sector	= from;
+		blk_fill_rwbs_rq(__entry->rwbs, rq);
+	),
+
+	TP_printk("%d,%d %s %llu + %u <- (%d,%d) %llu",
+		  MAJOR(__entry->dev), MINOR(__entry->dev), __entry->rwbs,
+		  (unsigned long long)__entry->sector,
+		  __entry->nr_sector,
+		  MAJOR(__entry->old_dev), MINOR(__entry->old_dev),
+		  (unsigned long long)__entry->old_sector)
+);
+
 #endif /* _TRACE_BLOCK_H */
 
 /* This part must be outside protection */
Index: linux-2.6.31/kernel/trace/blktrace.c
===================================================================
--- linux-2.6.31.orig/kernel/trace/blktrace.c
+++ linux-2.6.31/kernel/trace/blktrace.c
@@ -852,6 +852,37 @@ static void blk_add_trace_remap(struct r
 }
 
 /**
+ * blk_add_trace_rq_remap - Add a trace for a request-remap operation
+ * @q:		queue the io is for
+ * @rq:		the source request
+ * @dev:	target device
+ * @from:	source sector
+ *
+ * Description:
+ *     Device mapper remaps request to other devices.
+ *     Add a trace for that action.
+ *
+ **/
+static void blk_add_trace_rq_remap(struct request_queue *q,
+				   struct request *rq, dev_t dev,
+				   sector_t from)
+{
+	struct blk_trace *bt = q->blk_trace;
+	struct blk_io_trace_remap r;
+
+	if (likely(!bt))
+		return;
+
+	r.device_from = cpu_to_be32(dev);
+	r.device_to   = disk_devt(rq->rq_disk);
+	r.sector_from = cpu_to_be64(from);
+
+	__blk_add_trace(bt, blk_rq_pos(rq), blk_rq_bytes(rq),
+			rq_data_dir(rq), BLK_TA_REMAP, !!rq->errors,
+			sizeof(r), &r);
+}
+
+/**
  * blk_add_driver_data - Add binary message with driver-specific data
  * @q:		queue the io is for
  * @rq:		io request
@@ -918,10 +949,13 @@ static void blk_register_tracepoints(voi
 	WARN_ON(ret);
 	ret = register_trace_block_remap(blk_add_trace_remap);
 	WARN_ON(ret);
+	ret = register_trace_block_rq_remap(blk_add_trace_rq_remap);
+	WARN_ON(ret);
 }
 
 static void blk_unregister_tracepoints(void)
 {
+	unregister_trace_block_rq_remap(blk_add_trace_rq_remap);
 	unregister_trace_block_remap(blk_add_trace_remap);
 	unregister_trace_block_split(blk_add_trace_split);
 	unregister_trace_block_unplug_io(blk_add_trace_unplug_io);

Re: [PATCH] Add a tracepoint for block request remapping

[KOSAKI Motohiro]

CC to Li

Li, What do you think?


> Since 2.6.31 now has request-based device-mapper, it's useful to have
> a tracepoint for request-remapping as well as bio-remapping.
> 
> This patch adds a tracepoint for request-remapping, trace_block_rq_remap().
> Existing trace_block_remap() is left unchanged but it might be better to
> rename it to trace_block_bio_remap().
> 
> Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
> Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
> Cc: Jens Axboe <jens.axboe@oracle.com>
> Cc: Alasdair G Kergon <agk@redhat.com>
> ---
>  block/blk-core.c             |   11 +++++-----
>  drivers/md/dm.c              |    6 +++--
>  include/trace/events/block.h |   35 +++++++++++++++++++++++++++++++++-
>  kernel/trace/blktrace.c      |   44 ++++++++++++++++++++++++++++++++++++++-----
>  4 files changed, 83 insertions(+), 13 deletions(-)
> 
> Index: linux-2.6.31/block/blk-core.c
> ===================================================================
> --- linux-2.6.31.orig/block/blk-core.c
> +++ linux-2.6.31/block/blk-core.c
> @@ -34,6 +34,7 @@
>  #include "blk.h"
>  
>  EXPORT_TRACEPOINT_SYMBOL_GPL(block_remap);
> +EXPORT_TRACEPOINT_SYMBOL_GPL(block_rq_remap);
>  EXPORT_TRACEPOINT_SYMBOL_GPL(block_bio_complete);
>  
>  static int __make_request(struct request_queue *q, struct bio *bio);
> Index: linux-2.6.31/drivers/md/dm.c
> ===================================================================
> --- linux-2.6.31.orig/drivers/md/dm.c
> +++ linux-2.6.31/drivers/md/dm.c
> @@ -1503,6 +1503,8 @@ static void map_request(struct dm_target
>  		break;
>  	case DM_MAPIO_REMAPPED:
>  		/* The target has remapped the I/O so dispatch it */
> +		trace_block_rq_remap(clone->q, clone, disk_devt(dm_disk(md)),
> +				     blk_rq_pos(tio->orig));
>  		dm_dispatch_request(clone);
>  		break;
>  	case DM_MAPIO_REQUEUE:
> Index: linux-2.6.31/include/trace/events/block.h
> ===================================================================
> --- linux-2.6.31.orig/include/trace/events/block.h
> +++ linux-2.6.31/include/trace/events/block.h
> @@ -486,6 +486,39 @@ TRACE_EVENT(block_remap,
>  		  (unsigned long long)__entry->old_sector)
>  );
>  
> +TRACE_EVENT(block_rq_remap,
> +
> +	TP_PROTO(struct request_queue *q, struct request *rq, dev_t dev,
> +		 sector_t from),
> +
> +	TP_ARGS(q, rq, dev, from),
> +
> +	TP_STRUCT__entry(
> +		__field( dev_t,		dev		)
> +		__field( sector_t,	sector		)
> +		__field( unsigned int,	nr_sector	)
> +		__field( dev_t,		old_dev		)
> +		__field( sector_t,	old_sector	)
> +		__array( char,		rwbs,	6	)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->dev		= disk_devt(rq->rq_disk);
> +		__entry->sector		= blk_rq_pos(rq);
> +		__entry->nr_sector	= blk_rq_sectors(rq);
> +		__entry->old_dev	= dev;
> +		__entry->old_sector	= from;
> +		blk_fill_rwbs_rq(__entry->rwbs, rq);
> +	),
> +
> +	TP_printk("%d,%d %s %llu + %u <- (%d,%d) %llu",
> +		  MAJOR(__entry->dev), MINOR(__entry->dev), __entry->rwbs,
> +		  (unsigned long long)__entry->sector,
> +		  __entry->nr_sector,
> +		  MAJOR(__entry->old_dev), MINOR(__entry->old_dev),
> +		  (unsigned long long)__entry->old_sector)
> +);
> +
>  #endif /* _TRACE_BLOCK_H */
>  
>  /* This part must be outside protection */
> Index: linux-2.6.31/kernel/trace/blktrace.c
> ===================================================================
> --- linux-2.6.31.orig/kernel/trace/blktrace.c
> +++ linux-2.6.31/kernel/trace/blktrace.c
> @@ -852,6 +852,37 @@ static void blk_add_trace_remap(struct r
>  }
>  
>  /**
> + * blk_add_trace_rq_remap - Add a trace for a request-remap operation
> + * @q:		queue the io is for
> + * @rq:		the source request
> + * @dev:	target device
> + * @from:	source sector
> + *
> + * Description:
> + *     Device mapper remaps request to other devices.
> + *     Add a trace for that action.
> + *
> + **/
> +static void blk_add_trace_rq_remap(struct request_queue *q,
> +				   struct request *rq, dev_t dev,
> +				   sector_t from)
> +{
> +	struct blk_trace *bt = q->blk_trace;
> +	struct blk_io_trace_remap r;
> +
> +	if (likely(!bt))
> +		return;
> +
> +	r.device_from = cpu_to_be32(dev);
> +	r.device_to   = disk_devt(rq->rq_disk);
> +	r.sector_from = cpu_to_be64(from);
> +
> +	__blk_add_trace(bt, blk_rq_pos(rq), blk_rq_bytes(rq),
> +			rq_data_dir(rq), BLK_TA_REMAP, !!rq->errors,
> +			sizeof(r), &r);
> +}
> +
> +/**
>   * blk_add_driver_data - Add binary message with driver-specific data
>   * @q:		queue the io is for
>   * @rq:		io request
> @@ -918,10 +949,13 @@ static void blk_register_tracepoints(voi
>  	WARN_ON(ret);
>  	ret = register_trace_block_remap(blk_add_trace_remap);
>  	WARN_ON(ret);
> +	ret = register_trace_block_rq_remap(blk_add_trace_rq_remap);
> +	WARN_ON(ret);
>  }
>  
>  static void blk_unregister_tracepoints(void)
>  {
> +	unregister_trace_block_rq_remap(blk_add_trace_rq_remap);
>  	unregister_trace_block_remap(blk_add_trace_remap);
>  	unregister_trace_block_split(blk_add_trace_split);
>  	unregister_trace_block_unplug_io(blk_add_trace_unplug_io);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] Add a tracepoint for block request remapping

[Li Zefan]

Jun'ichi Nomura wrote:
> Since 2.6.31 now has request-based device-mapper, it's useful to have
> a tracepoint for request-remapping as well as bio-remapping.
> 
> This patch adds a tracepoint for request-remapping, trace_block_rq_remap().

> Existing trace_block_remap() is left unchanged but it might be better to
> rename it to trace_block_bio_remap().

Why not. ;)

> +static void blk_add_trace_rq_remap(struct request_queue *q,
> +				   struct request *rq, dev_t dev,
> +				   sector_t from)
> +{
> +	struct blk_trace *bt = q->blk_trace;
> +	struct blk_io_trace_remap r;
> +
> +	if (likely(!bt))
> +		return;
> +
> +	r.device_from = cpu_to_be32(dev);
> +	r.device_to   = disk_devt(rq->rq_disk);

cpu_to_be32(dis_devt(...))

> +	r.sector_from = cpu_to_be64(from);
> +
> +	__blk_add_trace(bt, blk_rq_pos(rq), blk_rq_bytes(rq),
> +			rq_data_dir(rq), BLK_TA_REMAP, !!rq->errors,
> +			sizeof(r), &r);
> +}

Re: [PATCH] Add a tracepoint for block request remapping

[Jun'ichi Nomura]

Li Zefan wrote:
> Jun'ichi Nomura wrote:
>> Since 2.6.31 now has request-based device-mapper, it's useful to have
>> a tracepoint for request-remapping as well as bio-remapping.
>>
>> This patch adds a tracepoint for request-remapping, trace_block_rq_remap().
> 
>> Existing trace_block_remap() is left unchanged but it might be better to
>> rename it to trace_block_bio_remap().
> 
> Why not. ;)

OK, I'll add a renaming patch.

>> +static void blk_add_trace_rq_remap(struct request_queue *q,
>> +				   struct request *rq, dev_t dev,
>> +				   sector_t from)
>> +{
>> +	struct blk_trace *bt = q->blk_trace;
>> +	struct blk_io_trace_remap r;
>> +
>> +	if (likely(!bt))
>> +		return;
>> +
>> +	r.device_from = cpu_to_be32(dev);
>> +	r.device_to   = disk_devt(rq->rq_disk);
> 
> cpu_to_be32(dis_devt(...))

Thank you! I'll fix that.

>> +	r.sector_from = cpu_to_be64(from);
>> +
>> +	__blk_add_trace(bt, blk_rq_pos(rq), blk_rq_bytes(rq),
>> +			rq_data_dir(rq), BLK_TA_REMAP, !!rq->errors,
>> +			sizeof(r), &r);
>> +}

-- 
Jun'ichi Nomura, NEC Corporation

Re: [PATCH] Add a tracepoint for block request remapping

[Jens Axboe]

On Fri, Sep 18 2009, Jun'ichi Nomura wrote:
> Since 2.6.31 now has request-based device-mapper, it's useful to have
> a tracepoint for request-remapping as well as bio-remapping.
> 
> This patch adds a tracepoint for request-remapping, trace_block_rq_remap().
> Existing trace_block_remap() is left unchanged but it might be better to
> rename it to trace_block_bio_remap().

This looks good, we should definitely have a remap trace at that level
too. Apart from that, nothing further to add than acking the observation
that you need to make the ->device_to part of the trace endian clean.

If you resend with that and just send the core bit (the dm part should
go through the dm branch), then I'll include it for 2.6.32.

-- 
Jens Axboe

Re: [PATCH] Add a tracepoint for block request remapping

[Jun'ichi Nomura]

Hi Jens, Alasdair,

Jens Axboe wrote:
> On Fri, Sep 18 2009, Jun'ichi Nomura wrote:
>> Since 2.6.31 now has request-based device-mapper, it's useful to have
>> a tracepoint for request-remapping as well as bio-remapping.
>>
>> This patch adds a tracepoint for request-remapping, trace_block_rq_remap().
>> Existing trace_block_remap() is left unchanged but it might be better to
>> rename it to trace_block_bio_remap().
> 
> This looks good, we should definitely have a remap trace at that level
> too. Apart from that, nothing further to add than acking the observation
> that you need to make the ->device_to part of the trace endian clean.
> 
> If you resend with that and just send the core bit (the dm part should
> go through the dm branch), then I'll include it for 2.6.32.

I resent a revised patchset but I'm afraid they might be confusing
as the other patchset for max_sectors/queue_limits were also sent
in parallel and I didn't make them look like a thread.
For your reference, these are the revised patchset for tracing 
request-remap:

[PATCH 1/3] block: Add a tracepoint for block request remapping
https://www.redhat.com/archives/dm-devel/2009-September/msg00201.html

[PATCH 2/3] block: Rename trace_block_remap to trace_block_bio_remap
https://www.redhat.com/archives/dm-devel/2009-September/msg00202.html

[PATCH 3/3] dm: Add a remapping trace to request-based dm
https://www.redhat.com/archives/dm-devel/2009-September/msg00204.html

The 3rd patch for dm depends on the 1st.

-- 
Jun'ichi Nomura, NEC Corporation