2009-09-18 01:43:31

by Junichi Nomura

[permalink] [raw]
Subject: [PATCH] Add a tracepoint for block request remapping

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 <[email protected]>
Signed-off-by: Jun'ichi Nomura <[email protected]>
Cc: Jens Axboe <[email protected]>
Cc: Alasdair G Kergon <[email protected]>
---
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);


2009-09-18 02:02:53

by KOSAKI Motohiro

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

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 <[email protected]>
> Signed-off-by: Jun'ichi Nomura <[email protected]>
> Cc: Jens Axboe <[email protected]>
> Cc: Alasdair G Kergon <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/


2009-09-18 05:58:44

by Li Zefan

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

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);
> +}

2009-09-18 10:45:38

by Jens Axboe

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

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

2009-09-18 14:59:13

by Junichi Nomura

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

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

2009-09-18 17:07:55

by Junichi Nomura

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

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