Messages in this thread | | | Date | Tue, 17 Aug 2004 16:17:42 +0100 | From | Christoph Hellwig <> | Subject | Re: Merge I2O patches from -mm |
| |
On Tue, Aug 17, 2004 at 03:31:18PM +0200, Markus Lidel wrote: > > Now to i2o_scsi: > > - the logic of "demand-allocating" Scsi_Hosts looks rather bad to me, > > life would be much simpler with a Scsi_Host per i2o device. > > But wouldn't it be a waste of resources to allocate a Scsi_Host > structure for every I2O device? Note that the i2o_scsi "sees" all disks > even if they are in a RAID array, so in most cases there are at least 3 > Scsi_Host adapters...
I wouldn't wasted ressources but it seems Alan found another problem.
> We also now know which disk is on which controller, this information is > lost with your approach...
You could still keep that information in your data structure. But what do you actually need it for?
> > - the slave_configure/i2o_scsi_probe_dev logical is quite horriblebut > > fortunately with the suggestion above it would just go away > > Yep, i know that it would be better to extend scsi_add_device, so it's > possible to pass a pointer to i2o_scsi_slave_alloc. This is only a > workaround, which breaks as soon as things are done in parallel :-(
Just keep some lookup data structure so you can find the device data by host/target/lun numbers.
> > - the global list of hosts and wlaking it on exit is a very bad design, > > that's something the ->remove callback should do on per-device basis > > But what if the I2O device isn't removed?
you're using the driver model, and that calls ->remove and every device when the driver is unregistered.
> > - please reorder the functions a little so you don't need forward-declarations > > most of the forward-declarations are not needed at all, should i remove > unneeded completely?
Yes, please.
Okay, some brainstorming to get the data structures and lookup right:
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.
Anyway, I think we could live without that.
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. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |