Re: Merge I2O patches from -mm

From: Markus Lidel
Date: Wed Aug 18 2004 - 18:18:33 EST

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@xxxxxxxxxxxxxxxxx
URL: http://www.shadowconnect.com
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);