Re: [PATCH] blk queue io tracing support

[Nathan Scott <nathans@sgi.com>]

Hi Jens,

On Wed, Aug 24, 2005 at 11:28:39AM +0200, Jens Axboe wrote:
> Ok, updated version.

One thing I found a bit awkward was the way its putting all inodes
in the root of the relayfs namespace, with the cpuid tacked on the
end of the bdevname - I was a bit confused at first when a trace of
sdd on my 4P box spontaneously created files for "partitions" sdd0,
sdd1, sdd2, and sdd3 ;).

I suppose if many more users of relayfs spring into existance, this
is going to get quite ugly.  Below is a patch that aligns the names
to the conventions used in sysfs; so, for example, when running two
traces simultaneously on /dev/sdd and /dev/sdb, instead of this:

# find /relay
/relay
/relay/sdd3
/relay/sdd2
/relay/sdd1
/relay/sdd0
/relay/sdb3
/relay/sdb2
/relay/sdb1
/relay/sdb0

it now uses this...

# find /relay
/relay
/relay/block
/relay/block/sdd
/relay/block/sdd/trace3
/relay/block/sdd/trace2
/relay/block/sdd/trace1
/relay/block/sdd/trace0
/relay/block/sdb
/relay/block/sdb/trace3
/relay/block/sdb/trace2
/relay/block/sdb/trace1
/relay/block/sdb/trace0

and does the correct dynamic setup and teardown of the hierarchy
as the userspace tool starts and stops tracing.  I had to modify
the relayfs rmdir code a bit to make this work properly, I'll
send a separate patch for that shortly.

> http://www.kernel.org/pub/linux/kernel/people/axboe/tools/blktrace.c
> 
> has been updated as well, the protocol version was increased to
> accomodate the trace structure changes.

I have the associated userspace change for this, as well as several
other fixes and tweaks for your tool - if you could slap a copyright
and license notice onto that source (pretty please? :) I'll send 'em
right along.

cheers.

-- 
Nathan

Index: 2.6.x-xfs/drivers/block/blktrace.c
===================================================================
--- 2.6.x-xfs.orig/drivers/block/blktrace.c
+++ 2.6.x-xfs/drivers/block/blktrace.c
@@ -40,6 +40,50 @@ void __blk_add_trace(struct blk_trace *b
 	local_irq_restore(flags);
 }
 
+static struct dentry * blk_tree_root;
+static DEFINE_SPINLOCK(blk_tree_lock);
+
+static inline void blk_remove_root(void)
+{
+	if (relayfs_remove_dir(blk_tree_root) != -ENOTEMPTY)
+		blk_tree_root = NULL;
+}
+
+static void blk_remove_tree(struct dentry *dir)
+{
+	spin_lock(&blk_tree_lock);
+	relayfs_remove_dir(dir);
+	blk_remove_root();
+	spin_unlock(&blk_tree_lock);
+}
+
+static struct dentry *blk_create_tree(const char *blk_name)
+{
+	struct dentry *dir;
+
+	spin_lock(&blk_tree_lock);
+	if (!blk_tree_root) {
+		blk_tree_root = relayfs_create_dir("block", NULL);
+		if (!blk_tree_root) {
+			spin_unlock(&blk_tree_lock);
+			return NULL;
+		}
+	}
+	dir = relayfs_create_dir(blk_name, blk_tree_root);
+	if (!dir)
+		blk_remove_root();
+	spin_unlock(&blk_tree_lock);
+
+	return dir;
+}
+
+void blk_cleanup_trace(struct blk_trace *bt)
+{
+	relay_close(bt->rchan);
+	blk_remove_tree(bt->dir);
+	kfree(bt);
+}
+
 int blk_stop_trace(struct block_device *bdev)
 {
 	request_queue_t *q = bdev_get_queue(bdev);
@@ -61,10 +105,8 @@ int blk_stop_trace(struct block_device *
 
 	up(&bdev->bd_sem);
 
-	if (bt) {
-		relay_close(bt->rchan);
-		kfree(bt);
-	}
+	if (bt)
+		blk_cleanup_trace(bt);
 
 	return ret;
 }
@@ -74,6 +116,7 @@ int blk_start_trace(struct block_device 
 	request_queue_t *q = bdev_get_queue(bdev);
 	struct blk_user_trace_setup buts;
 	struct blk_trace *bt = NULL;
+	struct dentry *dir = NULL;
 	char b[BDEVNAME_SIZE];
 	int ret;
 
@@ -101,11 +144,16 @@ int blk_start_trace(struct block_device 
 	if (!bt)
 		goto err;
 
+	ret = -ENOENT;
+	dir = blk_create_tree(bdevname(bdev, b));
+	if (!dir)
+		goto err;
+
+	bt->dir = dir;
 	atomic_set(&bt->sequence, 0);
 
-	bt->rchan = relay_open(bdevname(bdev, b), NULL, buts.buf_size,
-				buts.buf_nr, NULL);
 	ret = -EIO;
+	bt->rchan = relay_open("trace", dir, buts.buf_size, buts.buf_nr, NULL);
 	if (!bt->rchan)
 		goto err;
 
@@ -122,6 +170,8 @@ int blk_start_trace(struct block_device 
 
 err:
 	up(&bdev->bd_sem);
+	if (dir)
+		blk_remove_tree(dir);
 	if (bt)
 		kfree(bt);
 	return ret;
Index: 2.6.x-xfs/include/linux/blktrace.h
===================================================================
--- 2.6.x-xfs.orig/include/linux/blktrace.h
+++ 2.6.x-xfs/include/linux/blktrace.h
@@ -71,6 +71,7 @@ struct blk_io_trace {
 };
 
 struct blk_trace {
+	struct dentry *dir;
 	struct rchan *rchan;
 	atomic_t sequence;
 	u16 act_mask;
@@ -89,6 +90,7 @@ struct blk_user_trace_setup {
 #if defined(CONFIG_BLK_DEV_IO_TRACE)
 extern int blk_start_trace(struct block_device *, char __user *);
 extern int blk_stop_trace(struct block_device *);
+extern void blk_cleanup_trace(struct blk_trace *);
 extern void __blk_add_trace(struct blk_trace *, sector_t, int, int, u32, int, int, char *);
 
 static inline void blk_add_trace_rq(struct request_queue *q, struct request *rq,
@@ -137,6 +139,7 @@ static inline void blk_add_trace_generic
 #else /* !CONFIG_BLK_DEV_IO_TRACE */
 #define blk_start_trace(bdev, arg)		(-EINVAL)
 #define blk_stop_trace(bdev)			(-EINVAL)
+#define blk_cleanup_trace(bt)			do { } while (0)
 #define blk_add_trace_rq(q, rq, what)		do { } while (0)
 #define blk_add_trace_bio(q, rq, what)		do { } while (0)
 #define blk_add_trace_generic(q, rq, rw, what)	do { } while (0)
Index: 2.6.x-xfs/drivers/block/ll_rw_blk.c
===================================================================
--- 2.6.x-xfs.orig/drivers/block/ll_rw_blk.c
+++ 2.6.x-xfs/drivers/block/ll_rw_blk.c
@@ -1625,8 +1625,7 @@ void blk_cleanup_queue(request_queue_t *
 		__blk_queue_free_tags(q);
 
 	if (q->blk_trace) {
-		relay_close(q->blk_trace->rchan);
-		kfree(q->blk_trace);
+		blk_cleanup_trace(q->blk_trace);
 		q->blk_trace = NULL;
 	}