2015-05-06 6:42 GMT+09:00 James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>: > 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. I have just posted a formal patch. usb_stor_host_template_init() is now called in the newly added module_usb_stor_driver() helper macro which is almost same as module_usb_driver() so that it doesn't need to specify THIS_MODULE for each call. -- 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
