Re: [PATCH v6 0/4] scsi: ufs & ums-* & esp_scsi: fix module reference counting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux