Re: [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host
2015-01-21 0:20 GMT+09:00 Alan Stern st...@rowland.harvard.edu: On Tue, 20 Jan 2015, Akinobu Mita wrote: 2015-01-19 23:22 GMT+09:00 Tejun Heo t...@kernel.org: 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? -- You received this message because you are subscribed to the Google Groups open-iscsi group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To post to this group, send email to open-iscsi@googlegroups.com. Visit this group at http://groups.google.com/group/open-iscsi. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host
2015-01-19 23:22 GMT+09:00 Tejun Heo t...@kernel.org: 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. -- You received this message because you are subscribed to the Google Groups open-iscsi group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To post to this group, send email to open-iscsi@googlegroups.com. Visit this group at http://groups.google.com/group/open-iscsi. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host
Hello, Akinobu. On Tue, Jan 20, 2015 at 11:57:37PM +0900, Akinobu Mita wrote: 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. Hmmm, libata already exports most of the necessary symbols. libahci or platform drivers might have to export more but that shouldn't be much. For libata, pushing sht's to the leaf drivers would make far more sense as sht's already get inherited and modified along the hierarchy. Thanks. -- tejun -- You received this message because you are subscribed to the Google Groups open-iscsi group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To post to this group, send email to open-iscsi@googlegroups.com. Visit this group at http://groups.google.com/group/open-iscsi. For more options, visit https://groups.google.com/d/optout.
