On Tue, 2015-05-05 at 15:14 -0400, Alan Stern wrote: > On Tue, 5 May 2015, James Bottomley wrote: > > > > 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. > > Agreed, the size is relatively insignificant. I drop any objection on > that account. (But turning things around, what's wrong with > duplicating the .owner field in the Scsi_Host structure when we're > already duplicating this_id, can_queue, sg_tablesize, and a large bunch > of others?) It's really only the pattern wrongness. However, as Rusty says (at length) the best APIs are those that you can't get wrong and having the owner in the template gives that (it's a Level 10 interface). Duplicating the owner in the host gets us to the point where following convention you get it right but nothing prevents abuse (it's a Level 4 interface). I mean, it could be worse (his scale goes down to -10) but I'd rather not move down if we don't have to. > On the other hand, your code to populate the template copies is a mess. > I like Akinobu's approach much better Unfortunately with that approach, values in the host template can't be overridden. > , although I would change a few > details: > > Declare usb_stor_host_template as "const", and define a new, > separate usb_stor_template in usb.c for actual use. > > In usb_stor_host_template_init(), do a plain structure > assignment rather than memcpy. OK, so I'm happy with that: it solves the "can't be overridden" problem if you mean a driver should declare a local template call usb_stor_host_template_init() on the local template override whatever they want (or nothing) call usb_stor_probe1 with the new template. I also note that you can #define usb_stor_host_template_init(t, n) __usb_stor_host_template_init((t), (n), THIS_MODULE) And make it just work for normal usage. 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
