Re: [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host

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

 



2015-01-21 0:20 GMT+09:00 Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>:
> On Tue, 20 Jan 2015, Akinobu Mita wrote:
>
>> 2015-01-19 23:22 GMT+09:00 Tejun Heo <tj@xxxxxxxxxx>:
>> > On Mon, Jan 19, 2015 at 12:05:58AM +0900, Akinobu Mita wrote:
>> >> While accessing a scsi_device, the use count of the underlying LLDD
>> >> module is incremented.  The module reference is retrieved through
>> >> .module field of struct scsi_host_template.
>> >>
>> >> This mapping between scsi_device and underlying LLDD module works well
>> >> except some drivers which consist with the core driver and the actual
>> >> LLDDs and scsi_host_template is defined in the core driver.  In these
>> >> cases, the actual LLDDs can be unloaded even if the scsi_device is
>> >> being accessed.
>> >>
>> >> This patch series fixes the module reference mismatch problem for
>> >> ufs, usb-storage, esp_scsi, ahci_platform, and pata_platform drivers
>> >> by moving owner module reference field from struct scsi_host_template
>> >> to struct Scsi_Host and allowing the LLDDs to set their correct module
>> >> reference.
>> >
>> > Hmmm, can't we just move sht definitions to actual LLDDs?  libata can
>> > do that easily.  sht, as its name implies, is the template for
>> > creating the scsi_hosts of a given type.  We're now just moving module
>> > ownership from sht definition site to whatever callsite the actual
>> > instance is being created which can also be wrapped in a separate
>> > layer requiring explicit propagation.  Why not just propagate sht's
>> > directly?  What's the difference?
>>
>> The reason I didn't move sht from the core driver to the LLDDs for
>> fixing ufs and ums-* in the first place is to avoid exporting many
>> symbols for callbacks in sht.  But I realized that we can do it
>> without that many exported symbols by creating a single function that
>> returns a kmemdup()ed sht with a few change including ->module.
>>
>> So there are three options we can take for fixing this problem.
>> I would like to know the opinions which one should be taken.
>>
>> (1) Add owner module field to Scsi_Host for allowing LLDDs to adjust
>> it after scsi host allocation.  This approach is used by v1,2,3 of
>> this patch series and the scsi midlayer change is minimum.
>>
>> (2) Move owner module field from scsi_host_template to Scsi_Host.
>> The owner module reference is retrieved from the callsite of
>> scsi_host_alloc() by passing THIS_MODULE to the extra argument.
>> The scsi midlayer change is small, but we need the same macro trick
>> for each scsi_host_alloc() wrapper and these changes are relatively
>> large (required by libata, libiscsi, libfc, cxgbi, 53c700, legacy).
>> This approach is used by v4 of this patch series.
>>
>> (3) Allocate scsi host template for each module.  No scsi midlayer
>> change is required.  Instead of sharing a single scsi host template
>> defined in the core, create a single function that returns a
>> kmemdup()ed sht with a few change including ->module so that the sub
>> modules can use it as their sht.
>
> (3) means duplicating a reasonably large data structure in order to
> alter just one field.  It also means changing all the subdrivers to
> make them call the new function.
>
> (1) is the simplest.  Since the use of subdrivers in general tends to
> be a special case (most SCSI drivers don't do it), I prefer to keep the
> code optimized for it.  In other words, I prefer option (1).  If people
> think (2) is better, it can always be layered on top of (1).

OK, I agree that (1) is the simplest.

Christoph,

Option (2) required a lot of changes as this v4 shows that
scsi_host_alloc() is used by several libraries on scsi mid level in
various forms.  It is a bit different from pci_register_driver() or
similar registration interfaces.  Although they also use the same
macro trick to get a module reference, but they are called directly
or through thin and straightforward wrapper interfaces by lower
level drivers.

So, option (1) seems to be a better choice than (2), (3) for fixing
ufs, ums-*, and esp_scsi drivers.  For ahci_platform and pata_platform,
they can be fixed easily by moving sht definision to LLDDs as Tejun
suggested.  Do you think it is ok to prepare v5 of series in this way?
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux