Or Gerlitz wrote: > Both the TX (iscsi_queuecommand, iscsi_xmit_task, iscsi_conn_send_pdu) > and the RX (iscsi_complete_pdu and iser/tcp as well) code flows compete > on the session lock. Since the RX flow runs in softirq/bh/tasklet context, > if it cuts the TX flow on the same cpu, a deadlock may happen. To prevent > that, make iscsi_queuecommand use the _bh form of spin lock/unlock in > the same manner done by iscsi_xmit_task and iscsi_conn_send_pdu
The scsi_host_template->queuecommand function is called by scsi-ml with irqs disabled spin_lock_irqsave(host->host_lock, flags). I thought bhs are run from interrupt context, so scsi-mls disabling irqs did what we needed (see the bottom of Rusty Russells locking guide http://www.kernel.org/pub/linux/kernel/people/rusty/kernel-locking/c188.html#HARDIRQ-SOFTIRQ or I think there is more detail in the Linux Device Driver book). As for optimizations, disabling irqs for libiscsi's case is overkill since all drivers using iscsi_queuecommand/iscsi_complete_pdu use bhs for the iscsi processing. You could do something like iscsi_queuecommand { spin_unlock_irq(host lock) spin_lock_bh(session lock) ..... spin_unlock_bh(session lock) spin_lock_irq(host lock) } I think it would be a little harder than this because of scsi-ml's use of spin_lock_irqsave/spin_lock_irqrestore. Will that do the right thing still if we have enabled and disabled irqs or is it a nice way to use the api. For the latter I mean, I think most drivers do not need the host lock taken in the queuecommand function and so we could just change scsi-ml) and benefit everyone? -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To post to this group, send email to open-is...@googlegroups.com. To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/open-iscsi?hl=en.
