linux-next: block tree build failure

PATCH In review Stephen Rothwell 4 participants 14 messages Nov 26, 2008

Subsystems (6)

Memory management VFS Block layer Linux-next tree +2 more subsystems

lore

Share & tools

lore

Stephen Rothwell

author

2008-11-26 10:29 UTC #0 lore raw

Hi Jens,

Today's linux-next build (powerpc ppc64_defconfig) failed like this:

In file included from block/blk-core.c:31:
include/trace/block.h:9:15: error: macro "DEFINE_TRACE" passed 3 arguments, but takes just 1 In file included from block/blk-core.c:31:
include/trace/block.h:7: warning: data definition has no type or storage class include/trace/block.h:7: warning: type defaults to 'int' in declaration of 'DEFINE_TRACE'

and many more. This is caused by commit 714d010b5c249c2e9282515e0a2309a61ababbf4 ("blktrace: port to tracepoints") interacting with commit 7e066fb870fcd1025ec3ba7bbde5d541094f4ce1 ("tracepoints: add DECLARE_TRACE () and DEFINE_TRACE()") which entered linux-next for the first time today via the ftrace tree. This latter commit has changed the tracepoints API.

I will drop the block tree for today. I am not sure of the way forward.

Signature · 3 lines

Cheers,
Stephen Rothwell                    [email protected]
http://www.canb.auug.org.au/~sfr/

Ingo Molnar

2008-11-26 10:43 UTC #1 lore raw

Quoted 18 lines · Stephen Rothwell

Hi Jens,
Today's linux-next build (powerpc ppc64_defconfig) failed like this:
In file included from block/blk-core.c:31:
include/trace/block.h:9:15: error: macro "DEFINE_TRACE" passed 3 arguments, but takes just 1 In file included from block/blk-core.c:31:
include/trace/block.h:7: warning: data definition has no type or storage class include/trace/block.h:7: warning: type defaults to 'int' in declaration of 'DEFINE_TRACE'
and many more. This is caused by commit 714d010b5c249c2e9282515e0a2309a61ababbf4 ("blktrace: port to tracepoints") interacting with commit 7e066fb870fcd1025ec3ba7bbde5d541094f4ce1 ("tracepoints: add DECLARE_TRACE () and DEFINE_TRACE()") which entered linux-next for the first time today via the ftrace tree. This latter commit has changed the tracepoints API.

Quoted 1 line

I will drop the block tree for today. I am not sure of the way forward.

hm, i think that might be interaction between ftrace tree updates and blktrace?

Jens, would it be fine with you if we created a new tip/tracing/blktrace branch and picked up your changes and kept it all uptodate and merged with the ftrace tree? It should merge fine with other bits of the block tree, right? (the tracepoints are in rather stable spots of the code i think)

the other direction (you merging ftrace bits) doesnt look too hot, you'd have to pick up:

   95 files changed, 5308 insertions(+), 1603 deletions(-)

Ingo

Jens Axboe

2008-11-26 10:49 UTC #2 lore raw

Quoted 32 lines · Ingo Molnar

Quoted 18 lines · Stephen Rothwell
Hi Jens,
Today's linux-next build (powerpc ppc64_defconfig) failed like this:
In file included from block/blk-core.c:31:
include/trace/block.h:9:15: error: macro "DEFINE_TRACE" passed 3 arguments, but takes just 1 In file included from block/blk-core.c:31:
include/trace/block.h:7: warning: data definition has no type or storage class include/trace/block.h:7: warning: type defaults to 'int' in declaration of 'DEFINE_TRACE'
and many more. This is caused by commit 714d010b5c249c2e9282515e0a2309a61ababbf4 ("blktrace: port to tracepoints") interacting with commit 7e066fb870fcd1025ec3ba7bbde5d541094f4ce1 ("tracepoints: add DECLARE_TRACE () and DEFINE_TRACE()") which entered linux-next for the first time today via the ftrace tree. This latter commit has changed the tracepoints API.
Quoted 1 line
I will drop the block tree for today. I am not sure of the way forward.
hm, i think that might be interaction between ftrace tree updates and blktrace?
Jens, would it be fine with you if we created a new tip/tracing/blktrace branch and picked up your changes and kept it all uptodate and merged with the ftrace tree? It should merge fine with other bits of the block tree, right? (the tracepoints are in rather stable spots of the code i think)

Quoted 4 lines

the other direction (you merging ftrace bits) doesnt look too hot, you'd have to pick up:
   95 files changed, 5308 insertions(+), 1603 deletions(-)

Lets do that, it's just a single patch here from Arnaldo. I have attached it to this email.

Signature · 1 line

Jens Axboe

Quoted 1 line

From 714d010b5c249c2e9282515e0a2309a61ababbf4 Mon Sep 17 00:00:00 2001

From: Arnaldo Carvalho de Melo <[email protected]>
Date: Thu, 30 Oct 2008 08:34:33 +0100
Subject: [PATCH] blktrace: port to tracepoints

This was a forward port of work done by Mathieu Desnoyers, I changed it to encode the 'what' parameter on the tracepoint name, so that one can register interest in specific events and not on classes of events to then check the 'what' parameter.

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

Signed-off-by Jens Axboe <[email protected]>

---
 block/Kconfig                |    1 +
 block/blk-core.c             |   33 ++---
 block/blktrace.c             |  332 +++++++++++++++++++++++++++++++++++++++++-
 block/elevator.c             |    7 +-
 drivers/md/dm.c              |    6 +-
 fs/bio.c                     |    3 +-
 include/linux/blktrace_api.h |  172 +---------------------
 include/trace/block.h        |   60 ++++++++
 mm/bounce.c                  |    3 +-
 9 files changed, 418 insertions(+), 199 deletions(-)
 create mode 100644 include/trace/block.h

block/Kconfig +1 -0

index 1ab7c15..290b219 100644
@@ -47,6 +47,7 @@ config BLK_DEV_IO_TRACE
 	depends on SYSFS
 	select RELAY
 	select DEBUG_FS
+	select TRACEPOINTS
 	help
 	  Say Y here if you want to be able to trace the block layer actions
 	  on a given queue. Tracing allows you to see any traffic happening

block/blk-core.c +14 ~~-19~~

index 10e8a64..04267d6 100644
@@ -28,6 +28,7 @@
 #include <linux/task_io_accounting_ops.h>
 #include <linux/blktrace_api.h>
 #include <linux/fault-inject.h>
+#include <trace/block.h>
 
 #include "blk.h"

@@ -205,7 +206,7 @@ void blk_plug_device(struct request_queue *q)
 
 	if (!queue_flag_test_and_set(QUEUE_FLAG_PLUGGED, q)) {
 		mod_timer(&q->unplug_timer, jiffies + q->unplug_delay);
-		blk_add_trace_generic(q, NULL, 0, BLK_TA_PLUG);
+		trace_block_plug(q);
 	}
 }
 EXPORT_SYMBOL(blk_plug_device);

@@ -292,9 +293,7 @@ void blk_unplug_work(struct work_struct *work)
 	struct request_queue *q =
 		container_of(work, struct request_queue, unplug_work);
 
-	blk_add_trace_pdu_int(q, BLK_TA_UNPLUG_IO, NULL,
-				q->rq.count[READ] + q->rq.count[WRITE]);
-
+	trace_block_unplug_io(q);
 	q->unplug_fn(q);
 }

@@ -302,9 +301,7 @@ void blk_unplug_timeout(unsigned long data)
 {
 	struct request_queue *q = (struct request_queue *)data;
 
-	blk_add_trace_pdu_int(q, BLK_TA_UNPLUG_TIMER, NULL,
-				q->rq.count[READ] + q->rq.count[WRITE]);
-
+	trace_block_unplug_timer(q);
 	kblockd_schedule_work(q, &q->unplug_work);
 }

@@ -314,9 +311,7 @@ void blk_unplug(struct request_queue *q)
 	 * devices don't necessarily have an ->unplug_fn defined
 	 */
 	if (q->unplug_fn) {
-		blk_add_trace_pdu_int(q, BLK_TA_UNPLUG_IO, NULL,
-					q->rq.count[READ] + q->rq.count[WRITE]);
-
+		trace_block_unplug_io(q);
 		q->unplug_fn(q);
 	}
 }

@@ -822,7 +817,7 @@ rq_starved:
 	if (ioc_batching(q, ioc))
 		ioc->nr_batch_requests--;
 
-	blk_add_trace_generic(q, bio, rw, BLK_TA_GETRQ);
+	trace_block_getrq(q, bio, rw);
 out:
 	return rq;
 }

@@ -848,7 +843,7 @@ static struct request *get_request_wait(struct request_queue *q, int rw_flags,
 		prepare_to_wait_exclusive(&rl->wait[rw], &wait,
 				TASK_UNINTERRUPTIBLE);
 
-		blk_add_trace_generic(q, bio, rw, BLK_TA_SLEEPRQ);
+		trace_block_sleeprq(q, bio, rw);
 
 		__generic_unplug_device(q);
 		spin_unlock_irq(q->queue_lock);

@@ -928,7 +923,7 @@ void blk_requeue_request(struct request_queue *q, struct request *rq)
 {
 	blk_delete_timer(rq);
 	blk_clear_rq_complete(rq);
-	blk_add_trace_rq(q, rq, BLK_TA_REQUEUE);
+	trace_block_rq_requeue(q, rq);
 
 	if (blk_rq_tagged(rq))
 		blk_queue_end_tag(q, rq);

@@ -1167,7 +1162,7 @@ static int __make_request(struct request_queue *q, struct bio *bio)
 		if (!ll_back_merge_fn(q, req, bio))
 			break;
 
-		blk_add_trace_bio(q, bio, BLK_TA_BACKMERGE);
+		trace_block_bio_backmerge(q, bio);
 
 		req->biotail->bi_next = bio;
 		req->biotail = bio;

@@ -1186,7 +1181,7 @@ static int __make_request(struct request_queue *q, struct bio *bio)
 		if (!ll_front_merge_fn(q, req, bio))
 			break;
 
-		blk_add_trace_bio(q, bio, BLK_TA_FRONTMERGE);
+		trace_block_bio_frontmerge(q, bio);
 
 		bio->bi_next = req->bio;
 		req->bio = bio;

@@ -1269,7 +1264,7 @@ static inline void blk_partition_remap(struct bio *bio)
 		bio->bi_sector += p->start_sect;
 		bio->bi_bdev = bdev->bd_contains;
 
-		blk_add_trace_remap(bdev_get_queue(bio->bi_bdev), bio,
+		trace_block_remap(bdev_get_queue(bio->bi_bdev), bio,
 				    bdev->bd_dev, bio->bi_sector,
 				    bio->bi_sector - p->start_sect);
 	}

@@ -1441,10 +1436,10 @@ end_io:
 			goto end_io;
 
 		if (old_sector != -1)
-			blk_add_trace_remap(q, bio, old_dev, bio->bi_sector,
+			trace_block_remap(q, bio, old_dev, bio->bi_sector,
 					    old_sector);
 
-		blk_add_trace_bio(q, bio, BLK_TA_QUEUE);
+		trace_block_bio_queue(q, bio);
 
 		old_sector = bio->bi_sector;
 		old_dev = bio->bi_bdev->bd_dev;

@@ -1656,7 +1651,7 @@ static int __end_that_request_first(struct request *req, int error,
 	int total_bytes, bio_nbytes, next_idx = 0;
 	struct bio *bio;
 
-	blk_add_trace_rq(req->q, req, BLK_TA_COMPLETE);
+	trace_block_rq_complete(req->q, req);
 
 	/*
 	 * for a REQ_TYPE_BLOCK_PC request, we want to carry any eventual

block/blktrace.c +329 -3

375 lines not on this page · raw message

block/elevator.c +4 -3

index 9ac82dd..530fcfe 100644
@@ -33,6 +33,7 @@
 #include <linux/compiler.h>
 #include <linux/delay.h>
 #include <linux/blktrace_api.h>
+#include <trace/block.h>
 #include <linux/hash.h>
 #include <linux/uaccess.h>

@@ -586,7 +587,7 @@ void elv_insert(struct request_queue *q, struct request *rq, int where)
 	unsigned ordseq;
 	int unplug_it = 1;
 
-	blk_add_trace_rq(q, rq, BLK_TA_INSERT);
+	trace_block_rq_insert(q, rq);
 
 	rq->q = q;

@@ -772,7 +773,7 @@ struct request *elv_next_request(struct request_queue *q)
 			 * not be passed by new incoming requests
 			 */
 			rq->cmd_flags |= REQ_STARTED;
-			blk_add_trace_rq(q, rq, BLK_TA_ISSUE);
+			trace_block_rq_issue(q, rq);
 		}
 
 		if (!q->boundary_rq || q->boundary_rq == rq) {

@@ -921,7 +922,7 @@ void elv_abort_queue(struct request_queue *q)
 	while (!list_empty(&q->queue_head)) {
 		rq = list_entry_rq(q->queue_head.next);
 		rq->cmd_flags |= REQ_QUIET;
-		blk_add_trace_rq(q, rq, BLK_TA_ABORT);
+		trace_block_rq_abort(q, rq);
 		__blk_end_request(rq, -EIO, blk_rq_bytes(rq));
 	}
 }

drivers/md/dm.c +3 -3

index c99e472..d23fda1 100644
@@ -21,6 +21,7 @@
 #include <linux/idr.h>
 #include <linux/hdreg.h>
 #include <linux/blktrace_api.h>
+#include <trace/block.h>
 
 #define DM_MSG_PREFIX "core"

@@ -504,8 +505,7 @@ static void dec_pending(struct dm_io *io, int error)
 		end_io_acct(io);
 
 		if (io->error != DM_ENDIO_REQUEUE) {
-			blk_add_trace_bio(io->md->queue, io->bio,
-					  BLK_TA_COMPLETE);
+			trace_block_bio_complete(io->md->queue, io->bio);
 
 			bio_endio(io->bio, io->error);
 		}

@@ -598,7 +598,7 @@ static void __map_bio(struct dm_target *ti, struct bio *clone,
 	if (r == DM_MAPIO_REMAPPED) {
 		/* the bio has been remapped so dispatch it */
 
-		blk_add_trace_remap(bdev_get_queue(clone->bi_bdev), clone,
+		trace_block_remap(bdev_get_queue(clone->bi_bdev), clone,
 				    tio->io->bio->bi_bdev->bd_dev,
 				    clone->bi_sector, sector);

fs/bio.c +2 -1

index 77a55bc..060859c 100644
@@ -26,6 +26,7 @@
 #include <linux/mempool.h>
 #include <linux/workqueue.h>
 #include <linux/blktrace_api.h>
+#include <trace/block.h>
 #include <scsi/sg.h>		/* for struct sg_iovec */
 
 static struct kmem_cache *bio_slab __read_mostly;

@@ -1263,7 +1264,7 @@ struct bio_pair *bio_split(struct bio *bi, int first_sectors)
 	if (!bp)
 		return bp;
 
-	blk_add_trace_pdu_int(bdev_get_queue(bi->bi_bdev), BLK_TA_SPLIT, bi,
+	trace_block_split(bdev_get_queue(bi->bi_bdev), bi,
 				bi->bi_sector + first_sectors);
 
 	BUG_ON(bi->bi_vcnt != 1);

include/linux/blktrace_api.h +3 ~~-169~~

191 lines not on this page · raw message

include/trace/block.h new file +60 -0

60 lines not on this page · raw message

mm/bounce.c +2 -1

index 06722c4..bd1caaa 100644
@@ -14,6 +14,7 @@
 #include <linux/hash.h>
 #include <linux/highmem.h>
 #include <linux/blktrace_api.h>
+#include <trace/block.h>
 #include <asm/tlbflush.h>
 
 #define POOL_SIZE	64

@@ -222,7 +223,7 @@ static void __blk_queue_bounce(struct request_queue *q, struct bio **bio_orig,
 	if (!bio)
 		return;
 
-	blk_add_trace_bio(q, *bio_orig, BLK_TA_BOUNCE);
+	trace_block_bio_bounce(q, *bio_orig);
 
 	/*
 	 * at least one page was bounced, fill in possible non-highmem

Signature · 1 line

1.6.0.4.771.gef3b3

Ingo Molnar

2008-11-26 11:16 UTC #3 lore raw

Quoted 7 lines · Jens Axboe

Quoted 4 lines
the other direction (you merging ftrace bits) doesnt look too hot, you'd have to pick up:
   95 files changed, 5308 insertions(+), 1603 deletions(-)
Lets do that, it's just a single patch here from Arnaldo. I have attached it to this email.

thanks, here's the conversion to tracepoints below. Stephen, you might want to pick up the blk tree straight away plus the fix below.

Ingo

--------------->

Quoted 1 line

From 65ae5a3c622e707dbf7630397e94b7e2e9cd0ea5 Mon Sep 17 00:00:00 2001

From: Ingo Molnar <[email protected]>
Date: Wed, 26 Nov 2008 11:59:56 +0100
Subject: [PATCH] blktrace: port to tracepoints, update

Port to the new tracepoints API: split DEFINE_TRACE() and DECLARE_TRACE() sites.

Signed-off-by Ingo Molnar <[email protected]>

---
 block/blktrace.c      |   18 ++++++++++
 include/trace/block.h |   84 +++++++++++++++++++++++++++++--------------------
 2 files changed, 68 insertions(+), 34 deletions(-)

block/blktrace.c +18 -0

index b0a2cae..1dba3fb 100644
@@ -35,6 +35,24 @@ static atomic_t blk_probes_ref = ATOMIC_INIT(0);
 static int blk_register_tracepoints(void);
 static void blk_unregister_tracepoints(void);
 
+DEFINE_TRACE(block_bio_backmerge);
+DEFINE_TRACE(block_bio_bounce);
+DEFINE_TRACE(block_bio_complete);
+DEFINE_TRACE(block_bio_frontmerge);
+DEFINE_TRACE(block_bio_queue);
+DEFINE_TRACE(block_getrq);
+DEFINE_TRACE(block_plug);
+DEFINE_TRACE(block_remap);
+DEFINE_TRACE(block_rq_abort);
+DEFINE_TRACE(block_rq_complete);
+DEFINE_TRACE(block_rq_insert);
+DEFINE_TRACE(block_rq_issue);
+DEFINE_TRACE(block_rq_requeue);
+DEFINE_TRACE(block_sleeprq);
+DEFINE_TRACE(block_split);
+DEFINE_TRACE(block_unplug_io);
+DEFINE_TRACE(block_unplug_timer);
+
 /*
  * Send out a notify message.
  */

include/trace/block.h +50 ~~-34~~

107 lines not on this page · raw message

Jens Axboe

2008-11-26 11:22 UTC #4 lore raw

Quoted 6 lines · Ingo Molnar

Quoted 3 lines · Jens Axboe
Quoted 3 lines
the other direction (you merging ftrace bits) doesnt look too hot, you'd have to pick up:

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
   95 files changed, 5308 insertions(+), 1603 deletions(-)
Lets do that, it's just a single patch here from Arnaldo. I have attached it to this email.
thanks, here's the conversion to tracepoints below. Stephen, you might want to pick up the blk tree straight away plus the fix below.

I already pruned the patch from for-next and updated it, so you should not have any conflicts with or without this being merged.

Signature · 1 line

Jens Axboe

Ingo Molnar

2008-11-26 11:29 UTC #5 lore raw

Quoted 12 lines · Jens Axboe

Quoted 11 lines · Ingo Molnar
Quoted 7 lines · Jens Axboe
Quoted 4 lines
the other direction (you merging ftrace bits) doesnt look too hot, you'd have to pick up:
   95 files changed, 5308 insertions(+), 1603 deletions(-)
Lets do that, it's just a single patch here from Arnaldo. I have attached it to this email.

Quoted 6 lines

Quoted 3 lines
thanks, here's the conversion to tracepoints below. Stephen, you might want to pick up the blk tree straight away plus the fix below.
I already pruned the patch from for-next and updated it, so you should not have any conflicts with or without this being merged.

ok, thanks. The next ftrace-next update will have this.

Ingo

Mathieu Desnoyers

2008-11-26 11:23 UTC #6 lore raw

In reply to Ingo Molnar

Quoted 6 lines · * Ingo Molnar ([email protected]) wrote:

Quoted 3 lines · Jens Axboe
Quoted 3 lines
the other direction (you merging ftrace bits) doesnt look too hot, you'd have to pick up:

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
   95 files changed, 5308 insertions(+), 1603 deletions(-)
Lets do that, it's just a single patch here from Arnaldo. I have attached it to this email.
thanks, here's the conversion to tracepoints below. Stephen, you might want to pick up the blk tree straight away plus the fix below.

I guess the only difference between your implementation and mine is that you define all the tracepoints in block/blktrace.c, while I define them in the various files where they are used. I don't feel very strongly opinioned about any of the two alternatives.

Mathieu

Quoted 158 lines

Ingo

--------------->
From 65ae5a3c622e707dbf7630397e94b7e2e9cd0ea5 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <[email protected]>
Date: Wed, 26 Nov 2008 11:59:56 +0100
Subject: [PATCH] blktrace: port to tracepoints, update

Port to the new tracepoints API: split DEFINE_TRACE() and DECLARE_TRACE() sites.

Signed-off-by Ingo Molnar <[email protected]>

---
 block/blktrace.c      |   18 ++++++++++
 include/trace/block.h |   84 +++++++++++++++++++++++++++++--------------------
 2 files changed, 68 insertions(+), 34 deletions(-)

block/blktrace.c +18 -0

index b0a2cae..1dba3fb 100644
@@ -35,6 +35,24 @@ static atomic_t blk_probes_ref = ATOMIC_INIT(0);
 static int blk_register_tracepoints(void);
 static void blk_unregister_tracepoints(void);
 
+DEFINE_TRACE(block_bio_backmerge);
+DEFINE_TRACE(block_bio_bounce);
+DEFINE_TRACE(block_bio_complete);
+DEFINE_TRACE(block_bio_frontmerge);
+DEFINE_TRACE(block_bio_queue);
+DEFINE_TRACE(block_getrq);
+DEFINE_TRACE(block_plug);
+DEFINE_TRACE(block_remap);
+DEFINE_TRACE(block_rq_abort);
+DEFINE_TRACE(block_rq_complete);
+DEFINE_TRACE(block_rq_insert);
+DEFINE_TRACE(block_rq_issue);
+DEFINE_TRACE(block_rq_requeue);
+DEFINE_TRACE(block_sleeprq);
+DEFINE_TRACE(block_split);
+DEFINE_TRACE(block_unplug_io);
+DEFINE_TRACE(block_unplug_timer);
+
 /*
  * Send out a notify message.
  */

include/trace/block.h +50 ~~-34~~

index 3cc2675..25c6a1f 100644
@@ -4,57 +4,73 @@
 #include <linux/blkdev.h>
 #include <linux/tracepoint.h>
 
-DEFINE_TRACE(block_rq_abort,
+DECLARE_TRACE(block_rq_abort,
 	TPPROTO(struct request_queue *q, struct request *rq),
-	TPARGS(q, rq));
-DEFINE_TRACE(block_rq_insert,
+		TPARGS(q, rq));
+
+DECLARE_TRACE(block_rq_insert,
 	TPPROTO(struct request_queue *q, struct request *rq),
-	TPARGS(q, rq));
-DEFINE_TRACE(block_rq_issue,
+		TPARGS(q, rq));
+
+DECLARE_TRACE(block_rq_issue,
 	TPPROTO(struct request_queue *q, struct request *rq),
-	TPARGS(q, rq));
-DEFINE_TRACE(block_rq_requeue,
+		TPARGS(q, rq));
+
+DECLARE_TRACE(block_rq_requeue,
 	TPPROTO(struct request_queue *q, struct request *rq),
-	TPARGS(q, rq));
-DEFINE_TRACE(block_rq_complete,
+		TPARGS(q, rq));
+
+DECLARE_TRACE(block_rq_complete,
 	TPPROTO(struct request_queue *q, struct request *rq),
-	TPARGS(q, rq));
-DEFINE_TRACE(block_bio_bounce,
+		TPARGS(q, rq));
+
+DECLARE_TRACE(block_bio_bounce,
 	TPPROTO(struct request_queue *q, struct bio *bio),
-	TPARGS(q, bio));
-DEFINE_TRACE(block_bio_complete,
+		TPARGS(q, bio));
+
+DECLARE_TRACE(block_bio_complete,
 	TPPROTO(struct request_queue *q, struct bio *bio),
-	TPARGS(q, bio));
-DEFINE_TRACE(block_bio_backmerge,
+		TPARGS(q, bio));
+
+DECLARE_TRACE(block_bio_backmerge,
 	TPPROTO(struct request_queue *q, struct bio *bio),
-	TPARGS(q, bio));
-DEFINE_TRACE(block_bio_frontmerge,
+		TPARGS(q, bio));
+
+DECLARE_TRACE(block_bio_frontmerge,
 	TPPROTO(struct request_queue *q, struct bio *bio),
-	TPARGS(q, bio));
-DEFINE_TRACE(block_bio_queue,
+		TPARGS(q, bio));
+
+DECLARE_TRACE(block_bio_queue,
 	TPPROTO(struct request_queue *q, struct bio *bio),
-	TPARGS(q, bio));
-DEFINE_TRACE(block_getrq,
+		TPARGS(q, bio));
+
+DECLARE_TRACE(block_getrq,
 	TPPROTO(struct request_queue *q, struct bio *bio, int rw),
-	TPARGS(q, bio, rw));
-DEFINE_TRACE(block_sleeprq,
+		TPARGS(q, bio, rw));
+
+DECLARE_TRACE(block_sleeprq,
 	TPPROTO(struct request_queue *q, struct bio *bio, int rw),
-	TPARGS(q, bio, rw));
-DEFINE_TRACE(block_plug,
+		TPARGS(q, bio, rw));
+
+DECLARE_TRACE(block_plug,
 	TPPROTO(struct request_queue *q),
-	TPARGS(q));
-DEFINE_TRACE(block_unplug_timer,
+		TPARGS(q));
+
+DECLARE_TRACE(block_unplug_timer,
 	TPPROTO(struct request_queue *q),
-	TPARGS(q));
-DEFINE_TRACE(block_unplug_io,
+		TPARGS(q));
+
+DECLARE_TRACE(block_unplug_io,
 	TPPROTO(struct request_queue *q),
-	TPARGS(q));
-DEFINE_TRACE(block_split,
+		TPARGS(q));
+
+DECLARE_TRACE(block_split,
 	TPPROTO(struct request_queue *q, struct bio *bio, unsigned int pdu),
-	TPARGS(q, bio, pdu));
-DEFINE_TRACE(block_remap,
+		TPARGS(q, bio, pdu));
+
+DECLARE_TRACE(block_remap,
 	TPPROTO(struct request_queue *q, struct bio *bio, dev_t dev,
 		sector_t from, sector_t to),
-	TPARGS(q, bio, dev, from, to));
+		TPARGS(q, bio, dev, from, to));
 
 #endif

Signature · 2 lines

Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Ingo Molnar

2008-11-26 11:26 UTC #7 lore raw

Quoted 12 lines · Mathieu Desnoyers

Quoted 11 lines · * Ingo Molnar ([email protected]) wrote:
Quoted 7 lines · Jens Axboe
Quoted 4 lines
the other direction (you merging ftrace bits) doesnt look too hot, you'd have to pick up:
   95 files changed, 5308 insertions(+), 1603 deletions(-)
Lets do that, it's just a single patch here from Arnaldo. I have attached it to this email.

Quoted 7 lines

Quoted 2 lines
thanks, here's the conversion to tracepoints below. Stephen, you might want to pick up the blk tree straight away plus the fix below.
I guess the only difference between your implementation and mine is that you define all the tracepoints in block/blktrace.c, while I define them in the various files where they are used. I don't feel very strongly opinioned about any of the two alternatives.

usage site build attribute must be the same as definition site attribute. So blktrace.c is not a good place - we will fail the build in the !CONFIG_BLK_DEV_IO_TRACE case. I'll apply your spread-out definition variant.

Ingo

Mathieu Desnoyers

2008-11-26 12:21 UTC #8 lore raw

PATCH 1/1 Blktrace : conversion to tracepoint define

In reply to Jens Axboe

Here is the conversion to the new tracepoint API, which applies to -tip after the patch you just emailed.

Mathieu

Blktrace : conversion to tracepoint define

Adapt declaration/definitions of blktrace tracepoints to the new tracepoint API.

Signed-off-by Mathieu Desnoyers <[email protected]>

CC Arnaldo Carvalho de Melo <[email protected]>

CC 'Ingo Molnar' <[email protected]>

CC Jens Axboe <[email protected]>

CC Arnaldo Carvalho de Melo <[email protected]>

CC Stephen Rothwell <[email protected]>

---
 block/blk-core.c      |   13 +++++++++++++
 block/elevator.c      |    3 +++
 drivers/md/dm.c       |    2 ++
 fs/bio.c              |    2 ++
 include/trace/block.h |   34 +++++++++++++++++-----------------
 mm/bounce.c           |    2 ++
 6 files changed, 39 insertions(+), 17 deletions(-)

mm/bounce.c +2 -0

Index: linux.trees.git/mm/bounce.c
===================================================================
@@ -22,6 +22,8 @@
 
 static mempool_t *page_pool, *isa_page_pool;
 
+DEFINE_TRACE(block_bio_bounce);
+
 #ifdef CONFIG_HIGHMEM
 static __init int init_emergency_pool(void)
 {

fs/bio.c +2 -0

Index: linux.trees.git/fs/bio.c
===================================================================
@@ -29,6 +29,8 @@
 #include <trace/block.h>
 #include <scsi/sg.h>		/* for struct sg_iovec */
 
+DEFINE_TRACE(block_split);
+
 static struct kmem_cache *bio_slab __read_mostly;
 
 static mempool_t *bio_split_pool __read_mostly;

drivers/md/dm.c +2 -0

Index: linux.trees.git/drivers/md/dm.c
===================================================================
@@ -52,6 +52,8 @@ struct dm_target_io {
 	union map_info info;
 };
 
+DEFINE_TRACE(block_bio_complete);
+
 union map_info *dm_get_mapinfo(struct bio *bio)
 {
 	if (bio && bio->bi_private)

block/blk-core.c +13 -0

Index: linux.trees.git/block/blk-core.c
===================================================================
@@ -32,6 +32,19 @@
 
 #include "blk.h"
 
+DEFINE_TRACE(block_plug);
+DEFINE_TRACE(block_unplug_io);
+DEFINE_TRACE(block_unplug_timer);
+DEFINE_TRACE(block_getrq);
+DEFINE_TRACE(block_sleeprq);
+DEFINE_TRACE(block_rq_requeue);
+DEFINE_TRACE(block_bio_backmerge);
+DEFINE_TRACE(block_bio_frontmerge);
+DEFINE_TRACE(block_bio_queue);
+DEFINE_TRACE(block_rq_complete);
+DEFINE_TRACE(block_remap);	/* Also used in drivers/md/dm.c */
+EXPORT_TRACEPOINT_SYMBOL_GPL(block_remap);
+
 static int __make_request(struct request_queue *q, struct bio *bio);
 
 /*

block/elevator.c +3 -0

Index: linux.trees.git/block/elevator.c
===================================================================
@@ -53,6 +53,9 @@ static const int elv_hash_shift = 6;
 #define rq_hash_key(rq)		((rq)->sector + (rq)->nr_sectors)
 #define ELV_ON_HASH(rq)		(!hlist_unhashed(&(rq)->hash))
 
+DEFINE_TRACE(block_rq_insert);
+DEFINE_TRACE(block_rq_issue);
+
 /*
  * Query io scheduler to see if the current process issuing bio may be
  * merged with rq.

include/trace/block.h +17 ~~-17~~

Index: linux.trees.git/include/trace/block.h
===================================================================
@@ -4,55 +4,55 @@
 #include <linux/blkdev.h>
 #include <linux/tracepoint.h>
 
-DEFINE_TRACE(block_rq_abort,
+DECLARE_TRACE(block_rq_abort,
 	TPPROTO(struct request_queue *q, struct request *rq),
 	TPARGS(q, rq));
-DEFINE_TRACE(block_rq_insert,
+DECLARE_TRACE(block_rq_insert,
 	TPPROTO(struct request_queue *q, struct request *rq),
 	TPARGS(q, rq));
-DEFINE_TRACE(block_rq_issue,
+DECLARE_TRACE(block_rq_issue,
 	TPPROTO(struct request_queue *q, struct request *rq),
 	TPARGS(q, rq));
-DEFINE_TRACE(block_rq_requeue,
+DECLARE_TRACE(block_rq_requeue,
 	TPPROTO(struct request_queue *q, struct request *rq),
 	TPARGS(q, rq));
-DEFINE_TRACE(block_rq_complete,
+DECLARE_TRACE(block_rq_complete,
 	TPPROTO(struct request_queue *q, struct request *rq),
 	TPARGS(q, rq));
-DEFINE_TRACE(block_bio_bounce,
+DECLARE_TRACE(block_bio_bounce,
 	TPPROTO(struct request_queue *q, struct bio *bio),
 	TPARGS(q, bio));
-DEFINE_TRACE(block_bio_complete,
+DECLARE_TRACE(block_bio_complete,
 	TPPROTO(struct request_queue *q, struct bio *bio),
 	TPARGS(q, bio));
-DEFINE_TRACE(block_bio_backmerge,
+DECLARE_TRACE(block_bio_backmerge,
 	TPPROTO(struct request_queue *q, struct bio *bio),
 	TPARGS(q, bio));
-DEFINE_TRACE(block_bio_frontmerge,
+DECLARE_TRACE(block_bio_frontmerge,
 	TPPROTO(struct request_queue *q, struct bio *bio),
 	TPARGS(q, bio));
-DEFINE_TRACE(block_bio_queue,
+DECLARE_TRACE(block_bio_queue,
 	TPPROTO(struct request_queue *q, struct bio *bio),
 	TPARGS(q, bio));
-DEFINE_TRACE(block_getrq,
+DECLARE_TRACE(block_getrq,
 	TPPROTO(struct request_queue *q, struct bio *bio, int rw),
 	TPARGS(q, bio, rw));
-DEFINE_TRACE(block_sleeprq,
+DECLARE_TRACE(block_sleeprq,
 	TPPROTO(struct request_queue *q, struct bio *bio, int rw),
 	TPARGS(q, bio, rw));
-DEFINE_TRACE(block_plug,
+DECLARE_TRACE(block_plug,
 	TPPROTO(struct request_queue *q),
 	TPARGS(q));
-DEFINE_TRACE(block_unplug_timer,
+DECLARE_TRACE(block_unplug_timer,
 	TPPROTO(struct request_queue *q),
 	TPARGS(q));
-DEFINE_TRACE(block_unplug_io,
+DECLARE_TRACE(block_unplug_io,
 	TPPROTO(struct request_queue *q),
 	TPARGS(q));
-DEFINE_TRACE(block_split,
+DECLARE_TRACE(block_split,
 	TPPROTO(struct request_queue *q, struct bio *bio, unsigned int pdu),
 	TPARGS(q, bio, pdu));
-DEFINE_TRACE(block_remap,
+DECLARE_TRACE(block_remap,
 	TPPROTO(struct request_queue *q, struct bio *bio, dev_t dev,
 		sector_t from, sector_t to),
 	TPARGS(q, bio, dev, from, to));

Signature · 2 lines

Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Ingo Molnar

2008-11-26 11:22 UTC #9 lore raw

Re: [PATCH] Blktrace : conversion to tracepoint define

Quoted 45 lines · Mathieu Desnoyers

Here is the conversion to the new tracepoint API, which applies to -tip after the patch you just emailed.
Mathieu
Blktrace : conversion to tracepoint define
Adapt declaration/definitions of blktrace tracepoints to the new tracepoint API.
Signed-off-by Mathieu Desnoyers <[email protected]>
CC Arnaldo Carvalho de Melo <[email protected]>
CC 'Ingo Molnar' <[email protected]>
CC Jens Axboe <[email protected]>
CC Arnaldo Carvalho de Melo <[email protected]>
CC Stephen Rothwell <[email protected]>
---
 block/blk-core.c      |   13 +++++++++++++
 block/elevator.c      |    3 +++
 drivers/md/dm.c       |    2 ++
 fs/bio.c              |    2 ++
 include/trace/block.h |   34 +++++++++++++++++-----------------
 mm/bounce.c           |    2 ++
 6 files changed, 39 insertions(+), 17 deletions(-)
Index: linux.trees.git/mm/bounce.c
===================================================================
mm/bounce.c +2 -0
@@ -22,6 +22,8 @@
 
 static mempool_t *page_pool, *isa_page_pool;
 
+DEFINE_TRACE(block_bio_bounce);
+
 
 #ifdef CONFIG_HIGHMEM
 static __init int init_emergency_pool(void)
{
Index: linux.trees.git/fs/bio.c
===================================================================

fs/bio.c line 34

Quoted 6 lines

fs/bio.c +2 -0

@@ -29,6 +29,8 @@
 #include <trace/block.h>
 #include <scsi/sg.h>		/* for struct sg_iovec */
 
+DEFINE_TRACE(block_split);
+
 static struct kmem_cache *bio_slab __read_mostly;

heh, just did a similar patch :)

i moved the tracepoint definitions into a central place in block/blktrace.c, not spread out in the block files - because some of the tracepoints such as trace_block_remap() get used from multiple places so there's no real "home" file for them.

Moving the tracepoint definitions into blktrace.c consolidates it nicely without cluttering the various block/* files and ties their presence to CONFIG_BLK_DEV_IO_TRACE as well.

Ingo

Ingo Molnar

2008-11-26 11:24 UTC #10 lore raw

Re: [PATCH] Blktrace : conversion to tracepoint define

Quoted 3 lines · Ingo Molnar

Moving the tracepoint definitions into blktrace.c consolidates it nicely without cluttering the various block/* files and ties their presence to CONFIG_BLK_DEV_IO_TRACE as well.

hm, the latter is not a good idea: the tracepoints will be undefined when the io tracer is disabled - not good. I'll spread them out like you did.

Ingo

Mathieu Desnoyers

2008-11-26 11:27 UTC #11 lore raw

Re: [PATCH] Blktrace : conversion to tracepoint define

Quoted 7 lines · * Ingo Molnar ([email protected]) wrote:

Quoted 3 lines · Ingo Molnar
Moving the tracepoint definitions into blktrace.c consolidates it nicely without cluttering the various block/* files and ties their presence to CONFIG_BLK_DEV_IO_TRACE as well.

Quoted 5 lines

hm, the latter is not a good idea: the tracepoints will be undefined when the io tracer is disabled - not good. I'll spread them out like you did.
Ingo

Ah, yes, I thought there should be good reason for me doing things the way I did them. ;)

Mathieu

Signature · 2 lines

Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Ingo Molnar

2008-11-26 11:37 UTC #12 lore raw

Re: [PATCH] Blktrace : conversion to tracepoint define

In reply to Mathieu Desnoyers

Quoted 1 line · Mathieu Desnoyers

+DECLARE_TRACE(block_rq_abort,

btw., you forgot to define this one.

it should all be fine in tip/tracing/blktrace.

Ingo

Mathieu Desnoyers

2008-11-26 11:56 UTC #13 lore raw

Re: [PATCH] Blktrace : conversion to tracepoint define

Quoted 7 lines · * Ingo Molnar ([email protected]) wrote:

Quoted 1 line · Mathieu Desnoyers
+DECLARE_TRACE(block_rq_abort,
btw., you forgot to define this one.

I seems like it comes from a delta between 2.6.27.7 and 2.6.28-rc. Thanks for spotting it.

Mathieu

Quoted 3 lines

it should all be fine in tip/tracing/blktrace.
Ingo

Signature · 2 lines

Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68