public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Merge I2O patches from -mm
@ 2004-08-15 10:15 Warren Togami
  2004-08-17  2:15 ` Andrew Morton
  2004-08-17 11:53 ` Christoph Hellwig
  0 siblings, 2 replies; 24+ messages in thread
From: Warren Togami @ 2004-08-15 10:15 UTC (permalink / raw)
  To: linux-kernel

This is a request to please merge the I2O patches currently in Andrew 
Morton's -mm tree into the mainline kernel.  They resolve all known 
reported issues with I2O RAID devices.  If they can be included soon, it 
would be possible to implement and test direct installation before FC3 
Test2 freeze.

Also because Markus would never ask himself, I nominate Markus Lidel as 
the "maintainer" of the 2.6 generic I2O layer.  He has put a tremendous 
amount of work into improving an otherwise neglected part of the kernel. 
  Thanks to his efforts it is today usable and stable on multiple archs 
and all known supported cards.

Thank you,
Warren Togami
wtogami@redhat.com

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: Merge I2O patches from -mm
@ 2004-08-18 23:08 Markus Lidel
  2004-08-18 23:24 ` Christoph Hellwig
  0 siblings, 1 reply; 24+ messages in thread
From: Markus Lidel @ 2004-08-18 23:08 UTC (permalink / raw)
  To: hch; +Cc: alan, wtogami, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]

Hi...

Christoph Hellwig wrote:
 > What really seems to miss in your model is a callback to i2o_scsi
 > from the main i2o code when a new i2o_controller is found, if you
 > implemented
 > that we'd allocate/deallocate the Scsi_Host in that callback and
 > ->probe/->remove could be sure it'd always have it.
 > i2o_scsi_get_host would get inlined into i2o_scsi_probe.
 > i2o_scsi_remove basically needs a full rewrite, you need to find a way
 > to store a scsi_device ini i2o_dev and it becomes completely trivial.
 > Not sure about how to sanitize the scsi_devie probing logic
 > in i2o_scsi_probe yet.

Okay, patch i2o_scsi-cleanup.patch adds a notification facility to the 
i2o_driver, which notify if a controller is added or removed. The 
i2o_controller structure has now the ability to store per-driver data 
and the SCSI-OSM now takes advantage of this. So all ugly parts should 
be removed now :-)

If you have further things which should be changed, please let me know...



Best regards,


Markus Lidel
------------------------------------------
Markus Lidel (Senior IT Consultant)

Shadow Connect GmbH
Carl-Reisch-Weg 12
D-86381 Krumbach
Germany

Phone:  +49 82 82/99 51-0
Fax:    +49 82 82/99 51-11

E-Mail: Markus.Lidel@shadowconnect.com
URL:    http://www.shadowconnect.com

[-- Attachment #2: i2o_scsi-cleanup.patch --]
[-- Type: text/x-patch, Size: 15086 bytes --]

Index: include/linux/i2o.h
===================================================================
--- a/include/linux/i2o.h	(revision 111)
+++ b/include/linux/i2o.h	(revision 112)
@@ -34,6 +34,10 @@
 /* message queue empty */
 #define I2O_QUEUE_EMPTY		0xffffffff
 
+enum i2o_driver_notify {
+	I2O_DRIVER_NOTIFY_CONTROLLER_ADD = 0,
+	I2O_DRIVER_NOTIFY_CONTROLLER_REMOVE = 1,
+};
 
 /*
  *	Message structures
@@ -113,6 +117,9 @@
 
 	struct device_driver driver;
 
+	/* notification of changes */
+	void (*notify)(enum i2o_driver_notify, void *);
+
 	struct semaphore lock;
 };
 
@@ -201,6 +208,8 @@
 #endif
 	spinlock_t lock;			/* lock for controller
 						   configuration */
+
+	void *driver_data[I2O_MAX_DRIVERS];	/* storage for drivers */
 };
 
 /*
@@ -275,6 +284,7 @@
 extern u32 i2o_cntxt_list_add(struct i2o_controller *, void *);
 extern void *i2o_cntxt_list_get(struct i2o_controller *, u32);
 extern u32 i2o_cntxt_list_remove(struct i2o_controller *, void *);
+extern u32 i2o_cntxt_list_get_ptr(struct i2o_controller *, void *);
 
 static inline u32 i2o_ptr_low(void *ptr)
 {
@@ -301,6 +311,11 @@
 	return (u32)ptr;
 };
 
+static inline u32 i2o_cntxt_list_get_ptr(struct i2o_controller *c, void *ptr)
+{
+	return (u32)ptr;
+};
+
 static inline u32 i2o_ptr_low(void *ptr)
 {
 	return (u32)ptr;
@@ -316,6 +331,19 @@
 extern int i2o_driver_register(struct i2o_driver *);
 extern void i2o_driver_unregister(struct i2o_driver *);
 
+/**
+ *	i2o_driver_notify - Send notification to a single I2O drivers
+ *
+ *	Send notifications to a single registered driver.
+ */
+static inline void i2o_driver_notify(struct i2o_driver *drv,
+			enum i2o_driver_notify notify, void *data) {
+	if(drv->notify)
+			drv->notify(notify, data);
+}
+
+extern void i2o_driver_notify_all(enum i2o_driver_notify, void *);
+
 /* I2O device functions */
 extern int i2o_device_claim(struct i2o_device *);
 extern int i2o_device_claim_release(struct i2o_device *);
@@ -890,6 +918,7 @@
 #define I2O_TIMEOUT_RESET		30
 #define I2O_TIMEOUT_STATUS_GET		5
 #define I2O_TIMEOUT_LCT_GET		20
+#define I2O_TIMEOUT_SCSI_SCB_ABORT	240
 
 /* retries */
 #define I2O_HRT_GET_TRIES		3
Index: drivers/message/i2o/iop.c
===================================================================
--- a/drivers/message/i2o/iop.c	(revision 111)
+++ b/drivers/message/i2o/iop.c	(revision 112)
@@ -108,8 +108,8 @@
 #if BITS_PER_LONG == 64
 /**
  *      i2o_cntxt_list_add - Append a pointer to context list and return a id
+ *	@c: controller to which the context list belong
  *	@ptr: pointer to add to the context list
- *	@c: controller to which the context list belong
  *
  *	Because the context field in I2O is only 32-bit large, on 64-bit the
  *	pointer is to large to fit in the context field. The i2o_cntxt_list
@@ -117,7 +117,7 @@
  *
  *	Returns context id > 0 on success or 0 on failure.
  */
-u32 i2o_cntxt_list_add(struct i2o_controller * c, void *ptr)
+u32 i2o_cntxt_list_add(struct i2o_controller *c, void *ptr)
 {
 	struct i2o_context_list_element *entry;
 	unsigned long flags;
@@ -154,15 +154,15 @@
 
 /**
  *      i2o_cntxt_list_remove - Remove a pointer from the context list
+ *	@c: controller to which the context list belong
  *	@ptr: pointer which should be removed from the context list
- *	@c: controller to which the context list belong
  *
  *	Removes a previously added pointer from the context list and returns
  *	the matching context id.
  *
  *	Returns context id on succes or 0 on failure.
  */
-u32 i2o_cntxt_list_remove(struct i2o_controller * c, void *ptr)
+u32 i2o_cntxt_list_remove(struct i2o_controller *c, void *ptr)
 {
 	struct i2o_context_list_element *entry;
 	u32 context = 0;
@@ -189,9 +189,11 @@
 
 /**
  *      i2o_cntxt_list_get - Get a pointer from the context list and remove it
+ *	@c: controller to which the context list belong
  *	@context: context id to which the pointer belong
- *	@c: controller to which the context list belong
- *	returns pointer to the matching context id
+ *
+ *	Returns pointer to the matching context id on success or NULL on
+ *	failure.
  */
 void *i2o_cntxt_list_get(struct i2o_controller *c, u32 context)
 {
@@ -216,6 +218,37 @@
 
 	return ptr;
 };
+
+/**
+ *      i2o_cntxt_list_get_ptr - Get a context id from the context list
+ *	@c: controller to which the context list belong
+ *	@ptr: pointer to which the context id should be fetched
+ *
+ *	Returns context id which matches to the pointer on succes or 0 on
+ *	failure.
+ */
+u32 i2o_cntxt_list_get_ptr(struct i2o_controller * c, void *ptr)
+{
+	struct i2o_context_list_element *entry;
+	u32 context = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&c->context_list_lock, flags);
+	list_for_each_entry(entry, &c->context_list, list)
+	    if (entry->ptr == ptr) {
+		context = entry->context;
+		break;
+	}
+	spin_unlock_irqrestore(&c->context_list_lock, flags);
+
+	if (!context)
+		printk(KERN_WARNING "i2o: Could not find nonexistent ptr "
+		       "%p\n", ptr);
+
+	pr_debug("get context id from context list %p -> %d\n", ptr, context);
+
+	return context;
+};
 #endif
 
 /**
@@ -782,6 +815,8 @@
 
 	pr_debug("Deleting controller %s\n", c->name);
 
+	i2o_driver_notify_all(I2O_DRIVER_NOTIFY_CONTROLLER_REMOVE, c);
+
 	list_del(&c->list);
 
 	list_for_each_entry_safe(dev, tmp, &c->devices, list)
@@ -1098,6 +1133,8 @@
 
 	list_add(&c->list, &i2o_controllers);
 
+	i2o_driver_notify_all(I2O_DRIVER_NOTIFY_CONTROLLER_ADD, c);
+
 	printk(KERN_INFO "%s: Controller added\n", c->name);
 
 	return 0;
@@ -1209,6 +1246,7 @@
 EXPORT_SYMBOL(i2o_cntxt_list_add);
 EXPORT_SYMBOL(i2o_cntxt_list_get);
 EXPORT_SYMBOL(i2o_cntxt_list_remove);
+EXPORT_SYMBOL(i2o_cntxt_list_get_ptr);
 #endif
 EXPORT_SYMBOL(i2o_msg_get_wait);
 EXPORT_SYMBOL(i2o_msg_nop);
Index: drivers/message/i2o/i2o_scsi.c
===================================================================
--- a/drivers/message/i2o/i2o_scsi.c	(revision 111)
+++ b/drivers/message/i2o/i2o_scsi.c	(revision 112)
@@ -67,13 +67,12 @@
 
 #define VERSION_STRING        "Version 0.1.2"
 
+static struct i2o_driver i2o_scsi_driver;
+
 static int i2o_scsi_max_id = 16;
 static int i2o_scsi_max_lun = 8;
 
-static LIST_HEAD(i2o_scsi_hosts);
-
 struct i2o_scsi_host {
-	struct list_head list;	/* node in in i2o_scsi_hosts */
 	struct Scsi_Host *scsi_host;	/* pointer to the SCSI host */
 	struct i2o_controller *iop;	/* pointer to the I2O controller */
 	struct i2o_device *channel[0];	/* channel->i2o_dev mapping table */
@@ -81,19 +80,6 @@
 
 static struct scsi_host_template i2o_scsi_host_template;
 
-/*
- * This is only needed, because we can only set the hostdata after the device is
- * added to the scsi core. So we need this little workaround.
- */
-static DECLARE_MUTEX(i2o_scsi_probe_lock);
-static struct i2o_device *i2o_scsi_probe_dev = NULL;
-
-static int i2o_scsi_slave_alloc(struct scsi_device *sdp)
-{
-	sdp->hostdata = i2o_scsi_probe_dev;
-	return 0;
-};
-
 #define I2O_SCSI_CAN_QUEUE	4
 
 /* SCSI OSM class handling definition */
@@ -169,37 +155,11 @@
  *	is returned, otherwise the I2O controller is added to the SCSI
  *	core.
  *
- *	Returns pointer to the I2O SCSI host on success or negative error code
- *	on failure.
+ *	Returns pointer to the I2O SCSI host on success or NULL on failure.
  */
 static struct i2o_scsi_host *i2o_scsi_get_host(struct i2o_controller *c)
 {
-	struct i2o_scsi_host *i2o_shost;
-	int rc;
-
-	/* skip if already registered as I2O SCSI host */
-	list_for_each_entry(i2o_shost, &i2o_scsi_hosts, list)
-	    if (i2o_shost->iop == c)
-		return i2o_shost;
-
-	i2o_shost = i2o_scsi_host_alloc(c);
-	if (IS_ERR(i2o_shost)) {
-		printk(KERN_ERR "scsi-osm: Could not initialize SCSI host\n");
-		return i2o_shost;
-	}
-
-	rc = scsi_add_host(i2o_shost->scsi_host, &c->device);
-	if (rc) {
-		printk(KERN_ERR "scsi-osm: Could not add SCSI host\n");
-		scsi_host_put(i2o_shost->scsi_host);
-		return ERR_PTR(rc);
-	}
-
-	list_add(&i2o_shost->list, &i2o_scsi_hosts);
-	pr_debug("new I2O SCSI host added\n");
-
-	return i2o_shost;
-
+	return c->driver_data[i2o_scsi_driver.context];
 };
 
 /**
@@ -252,8 +212,8 @@
 	int i;
 
 	i2o_shost = i2o_scsi_get_host(c);
-	if (IS_ERR(i2o_shost))
-		return PTR_ERR(i2o_shost);
+	if (!i2o_shost)
+		return -EFAULT;
 
 	scsi_host = i2o_shost->scsi_host;
 
@@ -292,11 +252,8 @@
 		return -EFAULT;
 	}
 
-	down_interruptible(&i2o_scsi_probe_lock);
-	i2o_scsi_probe_dev = i2o_dev;
-	scsi_dev = scsi_add_device(i2o_shost->scsi_host, channel, id, lun);
-	i2o_scsi_probe_dev = NULL;
-	up(&i2o_scsi_probe_lock);
+	scsi_dev =
+	    __scsi_add_device(i2o_shost->scsi_host, channel, id, lun, i2o_dev);
 
 	if (!scsi_dev) {
 		printk(KERN_WARNING "scsi-osm: can not add SCSI device "
@@ -536,13 +493,67 @@
 				 cmd->request_bufflen, cmd->sc_data_direction);
 
 	return 1;
-}
+};
 
+/**
+ *	i2o_scsi_notify - Retrieve notifications of controller added or removed
+ *	@notify: the notification event which occurs
+ *	@data: pointer to additional data
+ *
+ *	If a I2O controller is added, we catch the notification to add a
+ *	corresponding Scsi_Host. On removal also remove the Scsi_Host.
+ */
+void i2o_scsi_notify(enum i2o_driver_notify notify, void *data)
+{
+	struct i2o_controller *c = data;
+	struct i2o_scsi_host *i2o_shost;
+	int rc;
+
+	switch (notify) {
+	case I2O_DRIVER_NOTIFY_CONTROLLER_ADD:
+		i2o_shost = i2o_scsi_host_alloc(c);
+		if (IS_ERR(i2o_shost)) {
+			printk(KERN_ERR "scsi-osm: Could not initialize"
+			       " SCSI host\n");
+			return;
+		}
+
+		rc = scsi_add_host(i2o_shost->scsi_host, &c->device);
+		if (rc) {
+			printk(KERN_ERR "scsi-osm: Could not add SCSI "
+			       "host\n");
+			scsi_host_put(i2o_shost->scsi_host);
+			return;
+		}
+
+		c->driver_data[i2o_scsi_driver.context] = i2o_shost;
+
+		pr_debug("new I2O SCSI host added\n");
+		break;
+
+	case I2O_DRIVER_NOTIFY_CONTROLLER_REMOVE:
+		i2o_shost = i2o_scsi_get_host(c);
+		if (!i2o_shost)
+			return;
+
+		c->driver_data[i2o_scsi_driver.context] = NULL;
+
+		scsi_remove_host(i2o_shost->scsi_host);
+		scsi_host_put(i2o_shost->scsi_host);
+		pr_debug("I2O SCSI host removed\n");
+		break;
+
+	default:
+		break;
+	}
+};
+
 /* SCSI OSM driver struct */
 static struct i2o_driver i2o_scsi_driver = {
 	.name = "scsi-osm",
 	.reply = i2o_scsi_reply,
 	.classes = i2o_scsi_class_id,
+	.notify = i2o_scsi_notify,
 	.driver = {
 		   .probe = i2o_scsi_probe,
 		   .remove = i2o_scsi_remove,
@@ -736,54 +747,47 @@
 	return 0;
 };
 
-#if 0
-FIXME
 /**
- *	i2o_scsi_abort	-	abort a running command
+ *	i2o_scsi_abort - abort a running command
  *	@SCpnt: command to abort
  *
  *	Ask the I2O controller to abort a command. This is an asynchrnous
- *	process and our callback handler will see the command complete
- *	with an aborted message if it succeeds.
+ *	process and our callback handler will see the command complete with an
+ *	aborted message if it succeeds.
  *
- *	Locks: no locks are held or needed
+ *	Returns 0 if the command is successfully aborted or negative error code
+ *	on failure.
  */
 int i2o_scsi_abort(struct scsi_cmnd *SCpnt)
 {
+	struct i2o_device *i2o_dev;
 	struct i2o_controller *c;
-	struct Scsi_Host *host;
-	struct i2o_scsi_host *hostdata;
-	u32 msg[5];
+	struct i2o_message *msg;
+	u32 m;
 	int tid;
 	int status = FAILED;
 
 	printk(KERN_WARNING "i2o_scsi: Aborting command block.\n");
 
-	host = SCpnt->device->host;
-	hostdata = (struct i2o_scsi_host *)host->hostdata;
-	tid = hostdata->task[SCpnt->device->id][SCpnt->device->lun];
-	if (tid == -1) {
-		printk(KERN_ERR "i2o_scsi: Impossible command to abort!\n");
-		return status;
-	}
-	c = hostdata->controller;
+	i2o_dev = SCpnt->device->hostdata;
+	c = i2o_dev->iop;
+	tid = i2o_dev->lct_data.tid;
 
-	spin_unlock_irq(host->host_lock);
+	m = i2o_msg_get_wait(c, &msg, I2O_TIMEOUT_MESSAGE_GET);
+	if (m == I2O_QUEUE_EMPTY)
+		return SCSI_MLQUEUE_HOST_BUSY;
 
-	msg[0] = FIVE_WORD_MSG_SIZE;
-	msg[1] = I2O_CMD_SCSI_ABORT << 24 | HOST_TID << 12 | tid;
-	msg[2] = scsi_context;
-	msg[3] = 0;
-	msg[4] = i2o_context_list_remove(SCpnt, c);
-	if (i2o_post_wait(c, msg, sizeof(msg), 240))
+	writel(FIVE_WORD_MSG_SIZE | SGL_OFFSET_0, &msg->u.head[0]);
+	writel(I2O_CMD_SCSI_ABORT << 24 | HOST_TID << 12 | tid,
+	       &msg->u.head[1]);
+	writel(i2o_cntxt_list_get_ptr(c, SCpnt), &msg->body[0]);
+
+	if (i2o_msg_post_wait(c, m, I2O_TIMEOUT_SCSI_SCB_ABORT))
 		status = SUCCESS;
 
-	spin_lock_irq(host->host_lock);
 	return status;
 }
 
-#endif
-
 /**
  *	i2o_scsi_bios_param	-	Invent disk geometry
  *	@sdev: scsi device
@@ -817,15 +821,12 @@
 	.name = "I2O SCSI Peripheral OSM",
 	.info = i2o_scsi_info,
 	.queuecommand = i2o_scsi_queuecommand,
-/*
-	.eh_abort_handler	= i2o_scsi_abort,
-*/
+	.eh_abort_handler = i2o_scsi_abort,
 	.bios_param = i2o_scsi_bios_param,
 	.can_queue = I2O_SCSI_CAN_QUEUE,
 	.sg_tablesize = 8,
 	.cmd_per_lun = 6,
 	.use_clustering = ENABLE_CLUSTERING,
-	.slave_alloc = i2o_scsi_slave_alloc,
 };
 
 /*
@@ -867,14 +868,6 @@
  */
 static void __exit i2o_scsi_exit(void)
 {
-	struct i2o_scsi_host *i2o_shost, *tmp;
-
-	/* Remove I2O SCSI hosts */
-	list_for_each_entry_safe(i2o_shost, tmp, &i2o_scsi_hosts, list) {
-		scsi_remove_host(i2o_shost->scsi_host);
-		scsi_host_put(i2o_shost->scsi_host);
-	}
-
 	/* Unregister I2O SCSI OSM from I2O core */
 	i2o_driver_unregister(&i2o_scsi_driver);
 };
Index: drivers/message/i2o/driver.c
===================================================================
--- a/drivers/message/i2o/driver.c	(revision 111)
+++ b/drivers/message/i2o/driver.c	(revision 112)
@@ -72,6 +72,7 @@
  */
 int i2o_driver_register(struct i2o_driver *drv)
 {
+	struct i2o_controller *c;
 	int i;
 	int rc = 0;
 	unsigned long flags;
@@ -108,6 +109,9 @@
 	spin_unlock_irqrestore(&i2o_drivers_lock, flags);
 
 	pr_debug("driver %s gets context id %d\n", drv->name, drv->context);
+	
+	list_for_each_entry(c, &i2o_controllers, list)
+		i2o_driver_notify(drv, I2O_DRIVER_NOTIFY_CONTROLLER_ADD, c);
 
 	rc = driver_register(&drv->driver);
 	if (rc)
@@ -125,12 +129,16 @@
  */
 void i2o_driver_unregister(struct i2o_driver *drv)
 {
+	struct i2o_controller *c;
 	unsigned long flags;
 
 	pr_debug("unregister driver %s\n", drv->name);
 
 	driver_unregister(&drv->driver);
 
+	list_for_each_entry(c, &i2o_controllers, list)
+		i2o_driver_notify(drv, I2O_DRIVER_NOTIFY_CONTROLLER_REMOVE, c);
+
 	spin_lock_irqsave(&i2o_drivers_lock, flags);
 	i2o_drivers[drv->context] = NULL;
 	spin_unlock_irqrestore(&i2o_drivers_lock, flags);
@@ -220,6 +228,23 @@
 }
 
 /**
+ *	i2o_driver_notify_all - Send notification to all I2O drivers
+ *
+ *	Send notifications to all registered drivers.
+ */
+void i2o_driver_notify_all(enum i2o_driver_notify evt, void *data) {
+	int i;
+	struct i2o_driver *drv;
+
+	for(i = 0; i < I2O_MAX_DRIVERS; i ++) {
+		drv = i2o_drivers[i];
+
+		if(drv)
+			i2o_driver_notify(drv, evt, data);
+	}
+}
+
+/**
  *	i2o_driver_init - initialize I2O drivers (OSMs)
  *
  *	Registers the I2O bus and allocate memory for the array of OSMs.
@@ -267,3 +292,4 @@
 
 EXPORT_SYMBOL(i2o_driver_register);
 EXPORT_SYMBOL(i2o_driver_unregister);
+EXPORT_SYMBOL(i2o_driver_notify_all);

^ permalink raw reply	[flat|nested] 24+ messages in thread
end of thread, other threads:[~2004-08-28 10:17 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-08-15 10:15 Merge I2O patches from -mm Warren Togami
2004-08-17  2:15 ` Andrew Morton
2004-08-17  8:36   ` Markus Lidel
2004-08-17 11:53 ` Christoph Hellwig
2004-08-17 13:31   ` Markus Lidel
2004-08-17 14:00     ` Alan Cox
2004-08-17 15:06       ` Christoph Hellwig
2004-08-17 14:50         ` Alan Cox
2004-08-17 15:17     ` Christoph Hellwig
2004-08-17 17:05       ` Markus Lidel
2004-08-17 16:56     ` Christoph Hellwig
2004-08-17 18:37       ` Markus Lidel
  -- strict thread matches above, loose matches on Subject: below --
2004-08-18 23:08 Markus Lidel
2004-08-18 23:24 ` Christoph Hellwig
2004-08-18 23:33   ` Markus Lidel
2004-08-19  9:48     ` Christoph Hellwig
2004-08-19 10:16       ` Markus Lidel
2004-08-19 10:06         ` Christoph Hellwig
2004-08-19 11:54           ` Markus Lidel
2004-08-23 17:55             ` Christoph Hellwig
2004-08-24  8:16               ` Markus Lidel
2004-08-24 12:45                 ` Christoph Hellwig
2004-08-24 16:00                   ` Markus Lidel
2004-08-28 10:13                     ` Warren Togami

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox