On Mon, 4 May 2015, James Bottomley wrote: > On Mon, 2015-05-04 at 16:09 -0400, Alan Stern wrote: > > On Mon, 4 May 2015, James Bottomley wrote: > > > > > However, it does also strike me that these three drivers have problems > > > because they're using the wrong initialisation pattern: the template is > > > supposed to be in the bus connector for compound drivers not in the > > > core. > > > > Why is it supposed to be done that way? Isn't that less efficient? It > > means you have to have a separate copy of the template for each bus > > connector driver, instead of letting them all share a common template > > in the core. > > Well, no it doesn't. The way 53c700 implements it is that there is a > common template in the core. The drivers just initialise their variant > fields (for 53c700 it's name, proc_name and this_id) and the core fills > in the rest. Admittedly wd33c93 doesn't quite get this right, that's > why I cited 53c700. You seem to be agreeing with me, even though you think you are disagreeing. "... there is a common template in the core." -- that's one scsi_host_template structure. "The drivers just initialize their variant fields ... and the core fills in the rest." -- that's an additional scsi_host_template structure for each driver. The total comes to N+1 scsi_host_templates, where N is the number of drivers. Now consider the way usb-storage does things. There is a common template in the core. That's one scsi_host_template structure. The drivers use the core's template. They have _no_ variant fields other than .owner. That's why I thought Akinobu's idea of putting the owner field in the Scsi_Host structure was a good idea. The total comes to 1 scsi_host_template. Is that not better than N+1? Consider the patch Akinobu just posted. In addition to a whole bunch of new code, it adds this: > --- a/drivers/usb/storage/alauda.c > +++ b/drivers/usb/storage/alauda.c ... > @@ -1232,6 +1233,8 @@ static int alauda_transport(struct scsi_cmnd *srb, struct us_data *us) > return USB_STOR_TRANSPORT_FAILED; > } > > +static struct scsi_host_template alauda_host_template; alauda.c was the only driver modified in that patch, and it gained an new scsi_host_template. For the case where the variants are identical in all respects other than .owner, doesn't it make sense to allow them to share a single scsi_host_template? The original design of the SCSI stack envisioned multiple drivers, each in control of multiple SCSI hosts. The idea was that scsi_host_template would be associated with the driver and Scsi_Host with the individual host. Now the kernel has evolved, and we have multiple drivers, some of which contain multiple subdrivers, each in control of multiple SCSI hosts. In this situation we should be flexible enough to allow the scsi_host_template to be associated with either the driver or the subdriver (decision to be made by the driver). When the only variant field is .owner, it makes sense to associate the scsi_host_template with the driver, not force it to be associated with the subdriver. The cost is duplication of the .owner field in every Scsi_Host. The savings is a reduction in the number of scsi_host_templates. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
