On Tue, 2015-05-05 at 10:25 -0400, Alan Stern wrote: > 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. So your essential objection is the host template duplication? I know it's a couple of hundred bytes, but surely its dwarfed by all the other stuff you have to duplicate ... the module size of each of these is around 0.25MB, so a couple of hundred bytes would seem a bit insignificant. James -- 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
