SMP pass through interface via bsg

SMP pass through interface via bsg

[FUJITA Tomonori]

I started to work on SMP pass through via bsg as a non-SCSI command
experimentation. I asked Doug to try the patch but it seems to take
some time to make Doug's hardware work with the mainline aic94xxx
driver. Meanwhile, I'd like to discuss some issues.

- user/kernel interface

How should a user-space tool (like Doug's smp_utils) send a request
and get the response via bsg?

-- each SAS object (host, device, expander, etc) has the own bsg device

A user-space tool opens a bsg device for a sas object that it wants to
send a request to.

-- one bsg device handles multiple SAS objects

Some options: one bsg device in the whole system; one bsg device per
SAS driver; one bsg device per SAS host; etc.

With this approach, we need to invent a header incluing routing
information.

My hacky patch creates one bsg device per SAS host and invents:

struct smp_passthrough_hdr {
	__u64 sas_address;
	__u8 phy_identifier;
};

a user-space tool sends this header instead of scb via bsg and put a
SMP request at dout_xfer and a buffer response at din_xfer (note that
now I do bidi transfer in a hacky way).


- SAS implementation details

As I said, the patch is too hacky. I just tried to implement a
smp_portal alternative by using bsg.

The patch adds a hook into sas transport class. sas_host_setup calls
bsg_register_queue. Then, the request_fn calls smp_execute_task to
send a smp request and get the response. It doesn't look good to link
the sas transport class with libsas. In addition, the mpt driver
handles smp request/response in a very different way.

Any suggestion to bind SMP pass through via bsg to aic94xx and mpt
cleanly?

Re: SMP pass through interface via bsg

[James Bottomley]

On Mon, 2007-04-02 at 11:49 +0900, FUJITA Tomonori wrote:
> I started to work on SMP pass through via bsg as a non-SCSI command
> experimentation. I asked Doug to try the patch but it seems to take
> some time to make Doug's hardware work with the mainline aic94xxx
> driver.

If you post the code, others who also have the hardware can try it
too ...

> How should a user-space tool (like Doug's smp_utils) send a request
> and get the response via bsg?

Basic frame in/frame out ... this is why we need the block layer
bidirectional support

> -- each SAS object (host, device, expander, etc) has the own bsg
> device

I think so; probably attached via the transport class.

> A user-space tool opens a bsg device for a sas object that it wants to
> send a request to.
> 
> -- one bsg device handles multiple SAS objects
> 
> Some options: one bsg device in the whole system; one bsg device per
> SAS driver; one bsg device per SAS host; etc.
> 
> With this approach, we need to invent a header incluing routing
> information.
> 
> My hacky patch creates one bsg device per SAS host and invents:
> 
> struct smp_passthrough_hdr {
>         __u64 sas_address;
>         __u8 phy_identifier;
> };

I'd really rather not go this route unless the one device per object
approach becomes untenable.

> a user-space tool sends this header instead of scb via bsg and put a
> SMP request at dout_xfer and a buffer response at din_xfer (note that
> now I do bidi transfer in a hacky way).
> 
> 
> - SAS implementation details
> 
> As I said, the patch is too hacky. I just tried to implement a
> smp_portal alternative by using bsg.
> 
> The patch adds a hook into sas transport class. sas_host_setup calls
> bsg_register_queue. Then, the request_fn calls smp_execute_task to
> send a smp request and get the response. It doesn't look good to link
> the sas transport class with libsas. In addition, the mpt driver
> handles smp request/response in a very different way.
> 
> Any suggestion to bind SMP pass through via bsg to aic94xx and mpt
> cleanly?

bind in the transport class, not the driver ...

James

Re: SMP pass through interface via bsg

[FUJITA Tomonori]

From: James Bottomley <James.Bottomley@SteelEye.com>
Subject: Re: SMP pass through interface via bsg
Date: Sun, 01 Apr 2007 22:57:42 -0500

> On Mon, 2007-04-02 at 11:49 +0900, FUJITA Tomonori wrote:
> > I started to work on SMP pass through via bsg as a non-SCSI command
> > experimentation. I asked Doug to try the patch but it seems to take
> > some time to make Doug's hardware work with the mainline aic94xxx
> > driver.
> 
> If you post the code, others who also have the hardware can try it
> too ...

I've attached a patch against Jens' bsg tree and uploaed a patch
against the sgv4-tools tree:

http://zaal.org/bsg/smp-test.diff

The sgv4-tools tree is at:

http://git.kernel.org/?p=linux/kernel/git/tomo/sgv4-tools.git;a=summary


If everything works:

gazania:/sys/class/bsg# ls
end_device-0:0  end_device-0:1  expander-0:0  sas_host0  sda  sdb

gazania:/home/fujita/sgv4-tools# ./smp_test /sys/class/bsg/expander-0\:0


smp_test works like Doug's smp_rep_manufacturer.

Note that `smp_test /sys/class/bsg/expander-0\:0/` doesn't work.

I've not tested these patches at all since I don't have proper
hardware. So please don't be surprised if the kernel crashes.


> > How should a user-space tool (like Doug's smp_utils) send a request
> > and get the response via bsg?
> 
> Basic frame in/frame out ... this is why we need the block layer
> bidirectional support

Yeah. Currently, I use a dirty hack to use sense buffer for bidi.


> > -- each SAS object (host, device, expander, etc) has the own bsg
> > device
> 
> I think so; probably attached via the transport class.

I see. I changed my patch to use this approach.


> > A user-space tool opens a bsg device for a sas object that it wants to
> > send a request to.
> > 
> > -- one bsg device handles multiple SAS objects
> > 
> > Some options: one bsg device in the whole system; one bsg device per
> > SAS driver; one bsg device per SAS host; etc.
> > 
> > With this approach, we need to invent a header incluing routing
> > information.
> > 
> > My hacky patch creates one bsg device per SAS host and invents:
> > 
> > struct smp_passthrough_hdr {
> >         __u64 sas_address;
> >         __u8 phy_identifier;
> > };
> 
> I'd really rather not go this route unless the one device per object
> approach becomes untenable.

OK. Probably, Dougs has some comments on this. I chose this because
Dougs prefers it and at the workshop Christoph seemed to be neutral in
two approaches.

Anyway, as I said, the attached patch uses "one bsg device per one SAS
object" approach.


> > a user-space tool sends this header instead of scb via bsg and put a
> > SMP request at dout_xfer and a buffer response at din_xfer (note that
> > now I do bidi transfer in a hacky way).
> > 
> > 
> > - SAS implementation details
> > 
> > As I said, the patch is too hacky. I just tried to implement a
> > smp_portal alternative by using bsg.
> > 
> > The patch adds a hook into sas transport class. sas_host_setup calls
> > bsg_register_queue. Then, the request_fn calls smp_execute_task to
> > send a smp request and get the response. It doesn't look good to link
> > the sas transport class with libsas. In addition, the mpt driver
> > handles smp request/response in a very different way.
> > 
> > Any suggestion to bind SMP pass through via bsg to aic94xx and mpt
> > cleanly?
> 
> bind in the transport class, not the driver ...

Yeah. The problem is that requests go to libsas for aic94xx while requests
directly go to the mpt driver.

I added a hook for SMP to struct sas_function_template.

diff --git a/block/bsg.c b/block/bsg.c
index a333c93..6c7802f 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -230,8 +230,8 @@ static int blk_fill_sgv4_hdr_rq(request_
 	if (copy_from_user(rq->cmd, (void *)(unsigned long)hdr->request,
 			   hdr->request_len))
 		return -EFAULT;
-	if (blk_verify_command(rq->cmd, has_write_perm))
-		return -EPERM;
+/* 	if (blk_verify_command(rq->cmd, has_write_perm)) */
+/* 		return -EPERM; */
 
 	/*
 	 * fill in request structure
@@ -263,7 +263,7 @@ bsg_validate_sgv4_hdr(request_queue_t *q
 		return -EIO;
 
 	/* not supported currently */
-	if (hdr->protocol || hdr->subprotocol)
+	if (hdr->protocol)
 		return -EINVAL;
 
 	/*
@@ -273,7 +273,7 @@ bsg_validate_sgv4_hdr(request_queue_t *q
 		return 0;
 
 	/* not supported currently */
-	if (hdr->dout_xfer_len && hdr->din_xfer_len)
+	if (!hdr->subprotocol && hdr->dout_xfer_len && hdr->din_xfer_len)
 		return -EINVAL;
 
 	*rw = hdr->dout_xfer_len ? WRITE : READ;
@@ -312,6 +312,12 @@ bsg_map_hdr(struct bsg_device *bd, struc
 		return ERR_PTR(ret);
 	}
 
+	if (hdr->subprotocol) {
+		rq->sense_len = hdr->dout_xfer_len;
+		rq->sense = (void*)(unsigned long)hdr->dout_xferp;
+		hdr->dout_xfer_len = 0;
+	}
+
 	if (hdr->dout_xfer_len) {
 		dxfer_len = hdr->dout_xfer_len;
 		dxferp = (void*)(unsigned long)hdr->dout_xferp;
@@ -361,8 +367,10 @@ static void bsg_rq_end_io(struct request
 static void bsg_add_command(struct bsg_device *bd, request_queue_t *q,
 			    struct bsg_command *bc, struct request *rq)
 {
-	rq->sense = bc->sense;
-	rq->sense_len = 0;
+	if (!bc->hdr.subprotocol) {
+		rq->sense = bc->sense;
+		rq->sense_len = 0;
+	}
 
 	/*
 	 * add bc command to busy queue and submit rq for io
@@ -449,7 +457,7 @@ static int blk_complete_sgv4_hdr_rq(stru
 	hdr->din_resid = rq->data_len;
 	hdr->response_len = 0;
 
-	if (rq->sense_len && hdr->response) {
+	if (!hdr->subprotocol && rq->sense_len && hdr->response) {
 		int len = min((unsigned int) hdr->max_response_len,
 			      rq->sense_len);
 
diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index 404c014..2fa4863 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -51,6 +51,8 @@ #include <linux/errno.h>
 #include <linux/jiffies.h>
 #include <linux/workqueue.h>
 #include <linux/delay.h>	/* for mdelay */
+#include <linux/blkdev.h>
+#include <linux/bio.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
@@ -1324,11 +1326,24 @@ mptsas_get_bay_identifier(struct sas_rph
 	return rc;
 }
 
+static int mptsas_smp_request(struct Scsi_Host *host, struct sas_rphy *rphy,
+			      struct request *req)
+{
+	{
+		char *buf = bio_data(req->bio);
+		printk("%s %d: %x %x %x %x\n", __FUNCTION__, __LINE__,
+		       buf[0], buf[1], buf[2], buf[3]);
+	}
+
+	return 0;
+}
+
 static struct sas_function_template mptsas_transport_functions = {
 	.get_linkerrors		= mptsas_get_linkerrors,
 	.get_enclosure_identifier = mptsas_get_enclosure_identifier,
 	.get_bay_identifier	= mptsas_get_bay_identifier,
 	.phy_reset		= mptsas_phy_reset,
+	.smp_request		= mptsas_smp_request,
 };
 
 static struct scsi_transport_template *mptsas_transport_template;
diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index dc70c18..a082fe2 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -24,6 +24,7 @@
 
 #include <linux/pci.h>
 #include <linux/scatterlist.h>
+#include <linux/blkdev.h>
 
 #include "sas_internal.h"
 
@@ -1915,3 +1916,47 @@ out:
 	return res;
 }
 #endif
+
+int sas_smp_request(struct Scsi_Host *shost, struct sas_rphy *rphy,
+		    struct request *req)
+{
+	struct domain_device *dev;
+	char *rsp;
+	int error, type = rphy->identify.device_type;
+
+	/* seems aic94xx doesn't support */
+	if (!rphy) {
+		printk("can we send a smp request to a host?\n");
+		return -EINVAL;
+	}
+
+	if (type != SAS_EDGE_EXPANDER_DEVICE &&
+	    type != SAS_FANOUT_EXPANDER_DEVICE) {
+		printk("can we send a smp request to a device?\n");
+		return -EINVAL;
+	}
+
+	dev = sas_find_dev_by_rphy(rphy);
+	if (!dev) {
+		printk("fail to find a domain_device?\n");
+		return -EINVAL;
+	}
+
+	rsp = kzalloc(req->sense_len, GFP_KERNEL);
+	if (!rsp) {
+		printk("fail to alloc smp response buffer\n");
+		return -ENOMEM;
+	}
+
+	/* TODO: multiple pages */
+	error = smp_execute_task(dev, bio_data(req->bio),
+				 req->data_len,
+				 rsp, req->sense_len);
+
+	if (copy_to_user(req->sense, rsp, req->sense_len))
+		printk("%s %d: fault\n", __FUNCTION__, __LINE__);
+
+	kfree(rsp);
+
+	return error;
+}
diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
index 965698c..a15b2f6 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -259,6 +259,7 @@ static struct sas_function_template sft
 	.phy_reset = sas_phy_reset,
 	.set_phy_speed = sas_set_phy_speed,
 	.get_linkerrors = sas_get_linkerrors,
+	.smp_request = sas_smp_request,
 };
 
 struct scsi_transport_template *
diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
index b2ef71a..bc17025 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -29,6 +29,8 @@ #include <linux/jiffies.h>
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/string.h>
+#include <linux/blkdev.h>
+#include <linux/bsg.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_device.h>
@@ -152,6 +154,77 @@ static struct {
 sas_bitfield_name_search(linkspeed, sas_linkspeed_names)
 sas_bitfield_name_set(linkspeed, sas_linkspeed_names)
 
+static void sas_smp_fn(struct request_queue *q, struct Scsi_Host *shost,
+		       struct sas_rphy *rphy)
+{
+	struct request *req;
+	int error;
+
+	while (!blk_queue_plugged(q)) {
+		req = elv_next_request(q);
+		if (!req)
+			break;
+
+		blkdev_dequeue_request(req);
+
+		spin_unlock(q->queue_lock);
+
+		printk("%s %d: %u %p %u\n", __FUNCTION__, __LINE__,
+		       req->data_len, req->sense, req->sense_len);
+
+		error = to_sas_internal(shost->transportt)->f->smp_request(shost, rphy, req);
+
+		spin_lock_irq(q->queue_lock);
+		req->end_io(req, error);
+	}
+}
+
+static void sas_host_smp_fn(struct request_queue *q)
+{
+	printk("%s %d\n", __FUNCTION__, __LINE__);
+
+	sas_smp_fn(q, (struct Scsi_Host *)q->queuedata, NULL);
+}
+
+static void sas_non_host_smp_fn(struct request_queue *q)
+{
+	struct sas_rphy *rphy = q->queuedata;
+
+	printk("%s %d\n", __FUNCTION__, __LINE__);
+
+	sas_smp_fn(q, rphy_to_shost(rphy), rphy);
+}
+
+static int sas_bsg_initialize(struct Scsi_Host *shost, struct sas_rphy *rphy,
+			      char *name)
+{
+	struct request_queue *q;
+	int error;
+
+	if (!to_sas_internal(shost->transportt)->f->smp_request)
+		return 0;
+
+	if (rphy)
+		q = blk_init_queue(sas_non_host_smp_fn, NULL);
+	else
+		q = blk_init_queue(sas_host_smp_fn, NULL);
+	if (!q)
+		return -ENOMEM;
+
+	error = bsg_register_queue(q, name);
+	if (error) {
+		blk_cleanup_queue(q);
+		return -ENOMEM;
+	}
+
+	if (rphy)
+		q->queuedata = rphy;
+	else
+		q->queuedata = shost;
+
+	return 0;
+}
+
 /*
  * SAS host attributes
  */
@@ -161,12 +234,19 @@ static int sas_host_setup(struct transpo
 {
 	struct Scsi_Host *shost = dev_to_shost(dev);
 	struct sas_host_attrs *sas_host = to_sas_host_attrs(shost);
+	char name[BUS_ID_SIZE];
 
 	INIT_LIST_HEAD(&sas_host->rphy_list);
 	mutex_init(&sas_host->lock);
 	sas_host->next_target_id = 0;
 	sas_host->next_expander_id = 0;
 	sas_host->next_port_id = 0;
+
+	snprintf(name, sizeof(name), "sas_host%d", shost->host_no);
+	if (sas_bsg_initialize(shost, NULL, name))
+		dev_printk(KERN_ERR, dev, "fail to a bsg device %d\n",
+			   shost->host_no);
+
 	return 0;
 }
 
@@ -1221,6 +1301,9 @@ struct sas_rphy *sas_end_device_alloc(st
 	sas_rphy_initialize(&rdev->rphy);
 	transport_setup_device(&rdev->rphy.dev);
 
+	if (sas_bsg_initialize(shost, &rdev->rphy, rdev->rphy.dev.bus_id))
+		printk("fail to a bsg device %s\n", rdev->rphy.dev.bus_id);
+
 	return &rdev->rphy;
 }
 EXPORT_SYMBOL(sas_end_device_alloc);
@@ -1260,6 +1343,9 @@ struct sas_rphy *sas_expander_alloc(stru
 	sas_rphy_initialize(&rdev->rphy);
 	transport_setup_device(&rdev->rphy.dev);
 
+	if (sas_bsg_initialize(shost, &rdev->rphy, rdev->rphy.dev.bus_id))
+		printk("fail to a bsg device %s\n", rdev->rphy.dev.bus_id);
+
 	return &rdev->rphy;
 }
 EXPORT_SYMBOL(sas_expander_alloc);
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index ad0182e..83af586 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -662,4 +662,6 @@ int __sas_task_abort(struct sas_task *);
 int sas_eh_device_reset_handler(struct scsi_cmnd *cmd);
 int sas_eh_bus_reset_handler(struct scsi_cmnd *cmd);
 
+extern int sas_smp_request(struct Scsi_Host *shost, struct sas_rphy *rphy,
+			   struct request *req);
 #endif /* _SASLIB_H_ */
diff --git a/include/scsi/scsi_transport_sas.h b/include/scsi/scsi_transport_sas.h
index 9aedc19..29631c7 100644
--- a/include/scsi/scsi_transport_sas.h
+++ b/include/scsi/scsi_transport_sas.h
@@ -7,7 +7,7 @@ #include <linux/mutex.h>
 
 struct scsi_transport_template;
 struct sas_rphy;
-
+struct request;
 
 enum sas_device_type {
 	SAS_PHY_UNUSED,
@@ -166,6 +166,7 @@ struct sas_function_template {
 	int (*phy_reset)(struct sas_phy *, int);
 	int (*phy_enable)(struct sas_phy *, int);
 	int (*set_phy_speed)(struct sas_phy *, struct sas_phy_linkrates *);
+	int (*smp_request)(struct Scsi_Host *, struct sas_rphy *, struct request *);
 };
 
 

Re: SMP pass through interface via bsg

[James Bottomley]

On Mon, 2007-04-02 at 21:43 +0900, FUJITA Tomonori wrote:
> From: James Bottomley <James.Bottomley@SteelEye.com>
> > If you post the code, others who also have the hardware can try it
> > too ...
> 
> I've attached a patch against Jens' bsg tree and uploaed a patch
> against the sgv4-tools tree:
> 
> http://zaal.org/bsg/smp-test.diff
> 
> The sgv4-tools tree is at:
> 
> http://git.kernel.org/?p=linux/kernel/git/tomo/sgv4-tools.git;a=summary
> 
> 
> If everything works:
> 
> gazania:/sys/class/bsg# ls
> end_device-0:0  end_device-0:1  expander-0:0  sas_host0  sda  sdb
> 
> gazania:/home/fujita/sgv4-tools# ./smp_test /sys/class/bsg/expander-0\:0
> 
> 
> smp_test works like Doug's smp_rep_manufacturer.
> 
> Note that `smp_test /sys/class/bsg/expander-0\:0/` doesn't work.

Well, I get a lot further than Doug with aic94xx, but it still doesn't
quite work.  This is the console output:

sas_non_host_smp_fn 193
sas_smp_fn 172: 8 0804d000 4
BUG: sleeping function called from invalid context at mm/slab.c:3032
in_atomic():0, irqs_disabled():1
no locks held by smp_test/3102.
irq event stamp: 1234
hardirqs last  enabled at (1233): [<c01370b5>] debug_check_no_locks_freed+0xa5/0x1c0
hardirqs last disabled at (1234): [<c02d906f>] _spin_lock_irq+0xf/0x40
softirqs last  enabled at (0): [<c011878d>] copy_process+0x2dd/0x11c0
softirqs last disabled at (0): [<00000000>] 0x0
 [<c0103b0a>] show_trace_log_lvl+0x1a/0x30
 [<c0104182>] show_trace+0x12/0x20
 [<c0104236>] dump_stack+0x16/0x20
 [<c0113829>] __might_sleep+0xb9/0xd0
 [<c0164448>] __kmalloc_track_caller+0xd8/0x110
 [<c0150799>] __kzalloc+0x19/0x50
 [<f88f3a6f>] sas_smp_request+0x3f/0x130 [libsas]
 [<f88d474f>] sas_smp_fn+0x8f/0xd0 [scsi_transport_sas]
 [<f88d53e0>] sas_non_host_smp_fn+0x60/0x70 [scsi_transport_sas]
 [<c01b48b1>] elv_insert+0x91/0x160
 [<c01b49dd>] __elv_add_request+0x5d/0xb0
 [<c01b7b82>] blk_execute_rq_nowait+0x52/0xa0
 [<c01b7c65>] blk_execute_rq+0x95/0xe0
 [<c01bd053>] bsg_ioctl+0x133/0x1b0
 [<c017308b>] do_ioctl+0x6b/0x80
 [<c01730f7>] vfs_ioctl+0x57/0x2a0
 [<c0173379>] sys_ioctl+0x39/0x60
 [<c0102b08>] syscall_call+0x7/0xb
 =======================
sas: smp_execute_task: task to dev 500605b000001450 response: 0x0 status 0x82
sas: smp_execute_task: task to dev 500605b000001450 response: 0x0 status 0x82
sas: smp_execute_task: task to dev 500605b000001450 response: 0x0 status 0x82

The first huge dump is just complaining about a kzalloc GFP_KERNEL with irqs off in sas_smp_request.

I'll debug why the execute_task is failing when I get more time today.

James

Re: SMP pass through interface via bsg

[FUJITA Tomonori]

From: James Bottomley <James.Bottomley@SteelEye.com>
Subject: Re: SMP pass through interface via bsg
Date: Mon, 02 Apr 2007 11:02:01 -0500

> On Mon, 2007-04-02 at 21:43 +0900, FUJITA Tomonori wrote:
> > From: James Bottomley <James.Bottomley@SteelEye.com>
> > > If you post the code, others who also have the hardware can try it
> > > too ...
> > 
> > I've attached a patch against Jens' bsg tree and uploaed a patch
> > against the sgv4-tools tree:
> > 
> > http://zaal.org/bsg/smp-test.diff
> > 
> > The sgv4-tools tree is at:
> > 
> > http://git.kernel.org/?p=linux/kernel/git/tomo/sgv4-tools.git;a=summary
> > 
> > 
> > If everything works:
> > 
> > gazania:/sys/class/bsg# ls
> > end_device-0:0  end_device-0:1  expander-0:0  sas_host0  sda  sdb
> > 
> > gazania:/home/fujita/sgv4-tools# ./smp_test /sys/class/bsg/expander-0\:0
> > 
> > 
> > smp_test works like Doug's smp_rep_manufacturer.
> > 
> > Note that `smp_test /sys/class/bsg/expander-0\:0/` doesn't work.
> 
> Well, I get a lot further than Doug with aic94xx, but it still doesn't
> quite work.  This is the console output:

Thanks.

>  =======================
> sas: smp_execute_task: task to dev 500605b000001450 response: 0x0 status 0x82
> sas: smp_execute_task: task to dev 500605b000001450 response: 0x0 status 0x82
> sas: smp_execute_task: task to dev 500605b000001450 response: 0x0 status 0x82
> 
> The first huge dump is just complaining about a kzalloc GFP_KERNEL
> with irqs off in sas_smp_request.

Probably, this is due to a silly bug in sas_smp_fn (please replace
spin_unlock with spin_unclok_irq).


> I'll debug why the execute_task is failing when I get more time today.

Thanks.

Re: SMP pass through interface via bsg

[FUJITA Tomonori]

From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: Re: SMP pass through interface via bsg
Date: Tue, 03 Apr 2007 01:27:47 +0900

> From: James Bottomley <James.Bottomley@SteelEye.com>
> Subject: Re: SMP pass through interface via bsg
> Date: Mon, 02 Apr 2007 11:02:01 -0500
> 
> > On Mon, 2007-04-02 at 21:43 +0900, FUJITA Tomonori wrote:
> > > From: James Bottomley <James.Bottomley@SteelEye.com>
> > > > If you post the code, others who also have the hardware can try it
> > > > too ...
> > > 
> > > I've attached a patch against Jens' bsg tree and uploaed a patch
> > > against the sgv4-tools tree:
> > > 
> > > http://zaal.org/bsg/smp-test.diff
> > > 
> > > The sgv4-tools tree is at:
> > > 
> > > http://git.kernel.org/?p=linux/kernel/git/tomo/sgv4-tools.git;a=summary
> > > 
> > > 
> > > If everything works:
> > > 
> > > gazania:/sys/class/bsg# ls
> > > end_device-0:0  end_device-0:1  expander-0:0  sas_host0  sda  sdb
> > > 
> > > gazania:/home/fujita/sgv4-tools# ./smp_test /sys/class/bsg/expander-0\:0
> > > 
> > > 
> > > smp_test works like Doug's smp_rep_manufacturer.
> > > 
> > > Note that `smp_test /sys/class/bsg/expander-0\:0/` doesn't work.
> > 
> > Well, I get a lot further than Doug with aic94xx, but it still doesn't
> > quite work.  This is the console output:
> 
> Thanks.
> 
> >  =======================
> > sas: smp_execute_task: task to dev 500605b000001450 response: 0x0 status 0x82
> > sas: smp_execute_task: task to dev 500605b000001450 response: 0x0 status 0x82
> > sas: smp_execute_task: task to dev 500605b000001450 response: 0x0 status 0x82
> > 
> > The first huge dump is just complaining about a kzalloc GFP_KERNEL
> > with irqs off in sas_smp_request.
> 
> Probably, this is due to a silly bug in sas_smp_fn (please replace
> spin_unlock with spin_unclok_irq).
> 
> 
> > I'll debug why the execute_task is failing when I get more time today.
> 
> Thanks.

OK. I found another bug in smp_test tool (sends bogus response buffer
len to kernel). I've uploaded a new patch:

http://zaal.org/bsg/smp-test2.diff

Re: SMP pass through interface via bsg

[James Bottomley]

On Tue, 2007-04-03 at 01:43 +0900, FUJITA Tomonori wrote:
> OK. I found another bug in smp_test tool (sends bogus response buffer
> len to kernel). I've uploaded a new patch:
> 
> http://zaal.org/bsg/smp-test2.diff

That sort of works; you have a final bug in that the manufacturer info
response frame is 64 bytes, not 128 bytes, but with that corrected
everything goes through and I get the result back:

hobholes:~# /home/jejb/git/sgv4-tools/smp_test /sys/class/bsg/expander-2\:0
  SAS-1.1 format: 0
  vendor identification: LSILOGIC
  product identification: SASx12 A.0      
  product revision level: 

So we can class this one as a success ... 

Thanks!

James

Re: SMP pass through interface via bsg

[FUJITA Tomonori]

From: James Bottomley <James.Bottomley@SteelEye.com>
Subject: Re: SMP pass through interface via bsg
Date: Mon, 02 Apr 2007 12:01:41 -0500

> On Tue, 2007-04-03 at 01:43 +0900, FUJITA Tomonori wrote:
> > OK. I found another bug in smp_test tool (sends bogus response buffer
> > len to kernel). I've uploaded a new patch:
> > 
> > http://zaal.org/bsg/smp-test2.diff
> 
> That sort of works; you have a final bug in that the manufacturer info
> response frame is 64 bytes, not 128 bytes, but with that corrected
> everything goes through and I get the result back:
> 
> hobholes:~# /home/jejb/git/sgv4-tools/smp_test /sys/class/bsg/expander-2\:0
>   SAS-1.1 format: 0
>   vendor identification: LSILOGIC
>   product identification: SASx12 A.0      
>   product revision level: 
> 
> So we can class this one as a success ... 
>
> Thanks!

Great! Thanks. I'll try to finish the mpt driver's hook
sometime. Finally, We have a bsg user (though it also needs proper
bidi support).

Jens, what remains to be done before bsg is merged into mainline?

Re: SMP pass through interface via bsg

[Jens Axboe]

On Tue, Apr 03 2007, FUJITA Tomonori wrote:
> From: James Bottomley <James.Bottomley@SteelEye.com>
> Subject: Re: SMP pass through interface via bsg
> Date: Mon, 02 Apr 2007 12:01:41 -0500
> 
> > On Tue, 2007-04-03 at 01:43 +0900, FUJITA Tomonori wrote:
> > > OK. I found another bug in smp_test tool (sends bogus response buffer
> > > len to kernel). I've uploaded a new patch:
> > > 
> > > http://zaal.org/bsg/smp-test2.diff
> > 
> > That sort of works; you have a final bug in that the manufacturer info
> > response frame is 64 bytes, not 128 bytes, but with that corrected
> > everything goes through and I get the result back:
> > 
> > hobholes:~# /home/jejb/git/sgv4-tools/smp_test /sys/class/bsg/expander-2\:0
> >   SAS-1.1 format: 0
> >   vendor identification: LSILOGIC
> >   product identification: SASx12 A.0      
> >   product revision level: 
> > 
> > So we can class this one as a success ... 
> >
> > Thanks!
> 
> Great! Thanks. I'll try to finish the mpt driver's hook
> sometime. Finally, We have a bsg user (though it also needs proper
> bidi support).
> 
> Jens, what remains to be done before bsg is merged into mainline?

Well the bi-dir stuff and sg v4 design were the two bits that needed to
get done before pushing bsg made sense, so we are getting there...
Probably a 2.6.23 target, leaving the bidi bits a revision cycle to get
sorted out.

-- 
Jens Axboe

Re: SMP pass through interface via bsg

[Mike Christie]

Jens Axboe wrote:
> On Tue, Apr 03 2007, FUJITA Tomonori wrote:
>> From: James Bottomley <James.Bottomley@SteelEye.com>
>> Subject: Re: SMP pass through interface via bsg
>> Date: Mon, 02 Apr 2007 12:01:41 -0500
>>
>>> On Tue, 2007-04-03 at 01:43 +0900, FUJITA Tomonori wrote:
>>>> OK. I found another bug in smp_test tool (sends bogus response buffer
>>>> len to kernel). I've uploaded a new patch:
>>>>
>>>> http://zaal.org/bsg/smp-test2.diff
>>> That sort of works; you have a final bug in that the manufacturer info
>>> response frame is 64 bytes, not 128 bytes, but with that corrected
>>> everything goes through and I get the result back:
>>>
>>> hobholes:~# /home/jejb/git/sgv4-tools/smp_test /sys/class/bsg/expander-2\:0
>>>   SAS-1.1 format: 0
>>>   vendor identification: LSILOGIC
>>>   product identification: SASx12 A.0      
>>>   product revision level: 
>>>
>>> So we can class this one as a success ... 
>>>
>>> Thanks!
>> Great! Thanks. I'll try to finish the mpt driver's hook
>> sometime. Finally, We have a bsg user (though it also needs proper
>> bidi support).
>>
>> Jens, what remains to be done before bsg is merged into mainline?
> 
> Well the bi-dir stuff and sg v4 design were the two bits that needed to
> get done before pushing bsg made sense, so we are getting there...
> Probably a 2.6.23 target, leaving the bidi bits a revision cycle to get
> sorted out.
> 

Could we get the bidi parts in without having to do the other sg clean
ups (my patches to merge the blk_rq_map* with the scsi ULD (sg, etc,
etc) equivalents or do the clean ups have to be done first?

Re: SMP pass through interface via bsg

[Jens Axboe]

On Mon, Apr 02 2007, Mike Christie wrote:
> Jens Axboe wrote:
> > On Tue, Apr 03 2007, FUJITA Tomonori wrote:
> >> From: James Bottomley <James.Bottomley@SteelEye.com>
> >> Subject: Re: SMP pass through interface via bsg
> >> Date: Mon, 02 Apr 2007 12:01:41 -0500
> >>
> >>> On Tue, 2007-04-03 at 01:43 +0900, FUJITA Tomonori wrote:
> >>>> OK. I found another bug in smp_test tool (sends bogus response buffer
> >>>> len to kernel). I've uploaded a new patch:
> >>>>
> >>>> http://zaal.org/bsg/smp-test2.diff
> >>> That sort of works; you have a final bug in that the manufacturer info
> >>> response frame is 64 bytes, not 128 bytes, but with that corrected
> >>> everything goes through and I get the result back:
> >>>
> >>> hobholes:~# /home/jejb/git/sgv4-tools/smp_test /sys/class/bsg/expander-2\:0
> >>>   SAS-1.1 format: 0
> >>>   vendor identification: LSILOGIC
> >>>   product identification: SASx12 A.0      
> >>>   product revision level: 
> >>>
> >>> So we can class this one as a success ... 
> >>>
> >>> Thanks!
> >> Great! Thanks. I'll try to finish the mpt driver's hook
> >> sometime. Finally, We have a bsg user (though it also needs proper
> >> bidi support).
> >>
> >> Jens, what remains to be done before bsg is merged into mainline?
> > 
> > Well the bi-dir stuff and sg v4 design were the two bits that needed to
> > get done before pushing bsg made sense, so we are getting there...
> > Probably a 2.6.23 target, leaving the bidi bits a revision cycle to get
> > sorted out.
> > 
> 
> Could we get the bidi parts in without having to do the other sg clean
> ups (my patches to merge the blk_rq_map* with the scsi ULD (sg, etc,
> etc) equivalents or do the clean ups have to be done first?

IMO the sg cleanups are an orthogonal effort.

-- 
Jens Axboe

Re: SMP pass through interface via bsg

[Boaz Harrosh]

Jens Axboe wrote:
> On Mon, Apr 02 2007, Mike Christie wrote:
>> Jens Axboe wrote:
>>> On Tue, Apr 03 2007, FUJITA Tomonori wrote:
>>>> From: James Bottomley <James.Bottomley@SteelEye.com>
>>>> Subject: Re: SMP pass through interface via bsg
>>>> Date: Mon, 02 Apr 2007 12:01:41 -0500
>>>>
>>>>> On Tue, 2007-04-03 at 01:43 +0900, FUJITA Tomonori wrote:
>>>>>> OK. I found another bug in smp_test tool (sends bogus response buffer
>>>>>> len to kernel). I've uploaded a new patch:
>>>>>>
>>>>>> http://zaal.org/bsg/smp-test2.diff
>>>>> That sort of works; you have a final bug in that the manufacturer info
>>>>> response frame is 64 bytes, not 128 bytes, but with that corrected
>>>>> everything goes through and I get the result back:
>>>>>
>>>>> hobholes:~# /home/jejb/git/sgv4-tools/smp_test /sys/class/bsg/expander-2\:0
>>>>>   SAS-1.1 format: 0
>>>>>   vendor identification: LSILOGIC
>>>>>   product identification: SASx12 A.0      
>>>>>   product revision level: 
>>>>>
>>>>> So we can class this one as a success ... 
>>>>>
>>>>> Thanks!
>>>> Great! Thanks. I'll try to finish the mpt driver's hook
>>>> sometime. Finally, We have a bsg user (though it also needs proper
>>>> bidi support).
>>>>
>>>> Jens, what remains to be done before bsg is merged into mainline?
>>> Well the bi-dir stuff and sg v4 design were the two bits that needed to
>>> get done before pushing bsg made sense, so we are getting there...
>>> Probably a 2.6.23 target, leaving the bidi bits a revision cycle to get
>>> sorted out.
>>>
>> Could we get the bidi parts in without having to do the other sg clean
>> ups (my patches to merge the blk_rq_map* with the scsi ULD (sg, etc,
>> etc) equivalents or do the clean ups have to be done first?
> 
> IMO the sg cleanups are an orthogonal effort.
> 

Following this e-mail is a patchset for bidi-support at the block layer.


Mike's sg patches will not apply trivially over the bidi patchset,
mainly at ll_rw_blk.c and scsi_lib.c. There were other people doing
cleanups in the direction of removing scsi_execute_async(). If these patches
add to ll_rw_blk.c/blkdev.h and eventually touch scsi_lib.c they might have
issues also. My suggestion is that Mike (and others) send me their latest patches
against Jens's block tree. (or point me to a git tree) so I can then help.

Boaz Harrosh

Re: SMP pass through interface via bsg

[FUJITA Tomonori]

From: Jens Axboe <jens.axboe@oracle.com>
Subject: Re: SMP pass through interface via bsg
Date: Mon, 2 Apr 2007 20:12:38 +0200

> On Tue, Apr 03 2007, FUJITA Tomonori wrote:
> > From: James Bottomley <James.Bottomley@SteelEye.com>
> > Subject: Re: SMP pass through interface via bsg
> > Date: Mon, 02 Apr 2007 12:01:41 -0500
> > 
> > > On Tue, 2007-04-03 at 01:43 +0900, FUJITA Tomonori wrote:
> > > > OK. I found another bug in smp_test tool (sends bogus response buffer
> > > > len to kernel). I've uploaded a new patch:
> > > > 
> > > > http://zaal.org/bsg/smp-test2.diff
> > > 
> > > That sort of works; you have a final bug in that the manufacturer info
> > > response frame is 64 bytes, not 128 bytes, but with that corrected
> > > everything goes through and I get the result back:
> > > 
> > > hobholes:~# /home/jejb/git/sgv4-tools/smp_test /sys/class/bsg/expander-2\:0
> > >   SAS-1.1 format: 0
> > >   vendor identification: LSILOGIC
> > >   product identification: SASx12 A.0      
> > >   product revision level: 
> > > 
> > > So we can class this one as a success ... 
> > >
> > > Thanks!
> > 
> > Great! Thanks. I'll try to finish the mpt driver's hook
> > sometime. Finally, We have a bsg user (though it also needs proper
> > bidi support).
> > 
> > Jens, what remains to be done before bsg is merged into mainline?
> 
> Well the bi-dir stuff and sg v4 design were the two bits that needed to
> get done before pushing bsg made sense, so we are getting there...
> Probably a 2.6.23 target, leaving the bidi bits a revision cycle to get
> sorted out.

sg v4 design issues refer to:

- iovec support

- an alternative to the read/write interface

- scsi command tag (possibly larger than 64 bits)

What else?

Re: SMP pass through interface via bsg

[James Smart]

James Bottomley wrote:
>> -- each SAS object (host, device, expander, etc) has the own bsg
>> device
> 
> I think so; probably attached via the transport class.

FYI - I understand the idea of a bsg device per object, but really, for
something that is used rarely, it's a bunch of overhead. Objects, data
structures, etc - more udev/kobject mgmt....  I believe I prefer the
approach of a shared distribution point - e.g. one bsg device at the
transport globally, or perhaps one at the host (actually the outbound
port aka host/channel) supporting the transport - followed by headers
in the messages that direct flow after that. This kinda follows the
model we have today for I/O - w/ queuecommand for the host, and
addressing in the SCSI command.

Additionally, I've always had some concern that we had to create an
object for everything in the SAN (every phy!), and have that view replicated
per host (for multi-initiator/multi-path SANs). I always believed there
was some sets of things that you would want to talk to that just doesn't
justify a new object (for example - do we start talking to process associators
in FC ?).  Another reason to move toward a transport-specific addressing
header.

My other concern with using bsg and the i/o path for transport management
functions is they compete with i/o for things like the can_queue values.
Should they ? Should they have higher priority ?

> I'd really rather not go this route unless the one device per object
> approach becomes untenable.

Understood, but building things until they topple is not a great idea
as there will be back-ward compatibility issues w/ user-space/sysfs and
the tools built around it. If you start with the shared distribution
point, you can always support both (eventually) if its a good idea.
Harder to do that in the reverse if it's toppling.

>> The patch adds a hook into sas transport class. sas_host_setup calls
>> bsg_register_queue. Then, the request_fn calls smp_execute_task to
>> send a smp request and get the response. It doesn't look good to link
>> the sas transport class with libsas. In addition, the mpt driver
>> handles smp request/response in a very different way.
>>
>> Any suggestion to bind SMP pass through via bsg to aic94xx and mpt
>> cleanly?
> 
> bind in the transport class, not the driver ...

Agree - the trick for libsas is to get an interface into the driver that
both drivers can support.

-- james s

Re: SMP pass through interface via bsg

[Douglas Gilbert]

James Smart wrote:
> 
> 
> James Bottomley wrote:
>>> -- each SAS object (host, device, expander, etc) has the own bsg
>>> device
>>
>> I think so; probably attached via the transport class.
> 
> FYI - I understand the idea of a bsg device per object, but really, for
> something that is used rarely, it's a bunch of overhead. Objects, data
> structures, etc - more udev/kobject mgmt....  I believe I prefer the
> approach of a shared distribution point - e.g. one bsg device at the
> transport globally, or perhaps one at the host (actually the outbound
> port aka host/channel) supporting the transport - followed by headers
> in the messages that direct flow after that. This kinda follows the
> model we have today for I/O - w/ queuecommand for the host, and
> addressing in the SCSI command.

James,
I fully agree.

> Additionally, I've always had some concern that we had to create an
> object for everything in the SAN (every phy!), and have that view
> replicated
> per host (for multi-initiator/multi-path SANs). I always believed there
> was some sets of things that you would want to talk to that just doesn't
> justify a new object (for example - do we start talking to process
> associators
> in FC ?).  Another reason to move toward a transport-specific addressing
> header.

Yes, seldom used things like well known logical units
and virtual SMP targets (there is one on every MPT Fusion
SAS LBA) that don't make the cut in the "devices for everything"
model become invisible to Linux users. It is exactly these
type of things that specialized user space programs use
a pass-through interface for.

So if the kernel can't find a use for it, then you, the
owner of the hardware, won't be able to use it either.
Hard to describe that approach as open software.

> My other concern with using bsg and the i/o path for transport management
> functions is they compete with i/o for things like the can_queue values.
> Should they ? Should they have higher priority ?

sg v4 adds priority control mechanisms but there still
remains possibilities for conflict, some of which may
cause problems.

I can see that a state based driver like st may want
to stop a pass-through getting to a logical unit most
of the time (and mechanisms could be added). However
even st may want to use a pass through to the transport
to reset the target ("hard reset") if it can't get the
LU RESET task management function to work.

>> I'd really rather not go this route unless the one device per object
>> approach becomes untenable.
> 
> Understood, but building things until they topple is not a great idea
> as there will be back-ward compatibility issues w/ user-space/sysfs and
> the tools built around it. If you start with the shared distribution
> point, you can always support both (eventually) if its a good idea.
> Harder to do that in the reverse if it's toppling.

We are talking about the SAS Management Protocol (SMP)
in this thread and in SAS-1 and SAS-1.1 discovery is done
by every SAS initiator, for every ripple in the topology.
In large topologies this approach can cause a "SMP storm"
that can temporarily drop SAS bandwidth to SCSI-1 figures.
Today discovery is done in the LLD or firmware (Adaptec and
LSI respectively) so they can magically make devices appear.
The approach in SAS-2 is to devolve SAS discovery to
expanders and use more efficient SMP functions. Current
generation SAS HBAs (and some LLDs) will need to alter
or stop their current SAS discovery techniques. The
user space may need to get involved, for zoning and
associated security.

Only allowing the SMP pass-through to talk to devices
that the kernel thinks are SAS expanders has some
shortcomings:
  - how can user space SAS topology discovery be done?
  - what about SMP targets that are not on expanders
  - disabling the phy that connects an expander
    to the SAS domain is problematic when the
    file descriptor you are using notionally represents
    that expander.


Note: discovery of a SAS topology is a different process from
finding logical units within SCSI targets. In the context of
SAS, the latter process can stay in the kernel and can be
done for each SSP target found, preferably after the SAS
topology has been fully discovered.

>>> The patch adds a hook into sas transport class. sas_host_setup calls
>>> bsg_register_queue. Then, the request_fn calls smp_execute_task to
>>> send a smp request and get the response. It doesn't look good to link
>>> the sas transport class with libsas. In addition, the mpt driver
>>> handles smp request/response in a very different way.
>>>
>>> Any suggestion to bind SMP pass through via bsg to aic94xx and mpt
>>> cleanly?
>>
>> bind in the transport class, not the driver ...
> 
> Agree - the trick for libsas is to get an interface into the driver that
> both drivers can support.

For LSI MPT Fusion it should be almost trivial to
map the <host,phy_id,sas_address> (Tomo's "hacky" approach")
through to LSI's ioc_num and SMP pass-through structures.

The aic94xx must have a similar structure. How else could
it implement a SMP pass-through for its own use :-)

Doug Gilbert

Re: SMP pass through interface via bsg

[James Bottomley]

On Mon, 2007-04-02 at 09:45 -0400, James Smart wrote:
> 
> James Bottomley wrote:
> >> -- each SAS object (host, device, expander, etc) has the own bsg
> >> device
> > 
> > I think so; probably attached via the transport class.
> 
> FYI - I understand the idea of a bsg device per object, but really, for
> something that is used rarely, it's a bunch of overhead.

It's some overhead, yes ... although rarely do the control elements
(switches, expanders, etc.) outnumber the actual end devices
significantly, so it's only a fractional increase in most configurations

>  Objects, data
> structures, etc - more udev/kobject mgmt.... 

This I'm prepared to trade for usability ... also, in our current sysfs
layout, we already have most of these objects.  Fundamentally, it's
easier just to use something that already exists in your hierarchy to do
the mapping, so /sys/class/bsg/expander-2:0 in tomo's example.

However, I realise this could be hidden by the tools as well in the
single tap, so it's a weak argument.

>  I believe I prefer the
> approach of a shared distribution point - e.g. one bsg device at the
> transport globally, or perhaps one at the host (actually the outbound
> port aka host/channel) supporting the transport - followed by headers
> in the messages that direct flow after that. This kinda follows the
> model we have today for I/O - w/ queuecommand for the host, and
> addressing in the SCSI command.

> Additionally, I've always had some concern that we had to create an
> object for everything in the SAN (every phy!), and have that view replicated
> per host (for multi-initiator/multi-path SANs). I always believed there
> was some sets of things that you would want to talk to that just doesn't
> justify a new object (for example - do we start talking to process associators
> in FC ?).  Another reason to move toward a transport-specific addressing
> header.

Well, that's a generic problem with sysfs and the device model.  We can
shortcircuit it by removing irrelevant objects ... so we don't actually
see phys in the FC tree, do we?  but for relevant objects, yes we do
show and allocate them.

> My other concern with using bsg and the i/o path for transport management
> functions is they compete with i/o for things like the can_queue values.
> Should they ? Should they have higher priority ?
> 
> > I'd really rather not go this route unless the one device per object
> > approach becomes untenable.
> 
> Understood, but building things until they topple is not a great idea
> as there will be back-ward compatibility issues w/ user-space/sysfs and
> the tools built around it. If you start with the shared distribution
> point, you can always support both (eventually) if its a good idea.
> Harder to do that in the reverse if it's toppling.

True ... but the one tap per relevant object approach won't explode over
what we currently have (i.e. if it's going to fall over, it will anyway
regardless of this issue ...)

> >> The patch adds a hook into sas transport class. sas_host_setup calls
> >> bsg_register_queue. Then, the request_fn calls smp_execute_task to
> >> send a smp request and get the response. It doesn't look good to link
> >> the sas transport class with libsas. In addition, the mpt driver
> >> handles smp request/response in a very different way.
> >>
> >> Any suggestion to bind SMP pass through via bsg to aic94xx and mpt
> >> cleanly?
> > 
> > bind in the transport class, not the driver ...
> 
> Agree - the trick for libsas is to get an interface into the driver that
> both drivers can support.
> 
> -- james s

James