On Jan 24 Nicholas A. Bellinger wrote: > On Tue, 2011-01-25 at 01:08 +0100, Stefan Richter wrote: > > void se_clear_dev_ports(struct se_device *dev) [...] > > Can this mess of releasing and reacquiring locks --- which looks all rather > > dangerous --- be cleaned up if you move the logical units (?) to be deleted > > over to a secondary list? > > Fair point on this one. Having to sleep in core_dev_del_lun() waiting > for all outstanding struct se_cmd -> struct se_lun I/O descriptor > mappings to be shutdown makes this look pretty ugly currently in > se_clear_dev_ports(). However adding another list+lock to this mix > really does not make it any less complex. I meant an on-stack throwaway list which you can change inside se_clear_dev_ports() without any lock. But I didn't look whether this is actually possible and beneficial. > Looking at se_clear_dev_ports() again in the two usage contexts > target_core_device.c:se_free_virtual_device() and > target_core_hba.c:core_delete_hba(), [...] > config_group structures, I think se_clear_dev_ports() should be able to > be dropped all together now. That of course sounds great. > I will take a deeper look and see if this > is really in fact safe for v4.0/for-38 code. As a thought from an outsider: If drivers/target/ weren't entirely new in 2.6.38, then only a minimal cautious locking fix would be post -rc1 material. Linus releases so frequently that anything beyond essential fixes can easily wait for regular merge windows. "Essential" = very low risk : reward ratio && with reasonable reward. Those who directly work with the code tend to underestimate risk and to overestimate reward. ;-) -- Stefan Richter -=====-==-== ---= ==--= http://arcgraph.de/sr/ -- 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
