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.


Reply via email to