>-----Original Message----- >From: Mike Christie [mailto:[EMAIL PROTECTED] >Sent: Monday, August 04, 2008 5:45 PM >To: Dev, Vasu >Cc: [email protected] >Subject: Re: [Open-FCoE] [PATCH 1/1] libfc: remove shared tag map and fix >timer usage in fc_fcp.c (take 2) > >Dev, Vasu wrote: >>> -----Original Message----- >>> From: Mike Christie [mailto:[EMAIL PROTECTED] >>> Sent: Sunday, August 03, 2008 11:51 PM >>> To: Dev, Vasu >>>> and that is a separate issue but in general what do you think on >>>> calling into fc_exch.c with or without any lock held ? >>>> >>> For fc_fcp.c we just have those issues where we want to set the seq_ptr >>> after we call exch_seq_send, so if there was some crazy case where we >>> send the sequence, and it completes on a another thread before >>> exch_seq_send returns the fc_fcp.c response handler is going to get >>> confused. Does fc_exch.c prevent this type of race for the upper layer? >> >> No, fc_exch.c cannot ensure this in current implementation. >> >> I think I didn't explain my self well in last response, my intent was >> to get clarification to have any lock held or not before >> calling into fc_exch.c functions including (exch_seq_send()). >> >> In fact, currently local port lock is held almost all the time when >> calling into fc_exch.c, so not just changed fc_lun_reset_send() >> calling into fc_exch.c with pkt lock held and some other code >> doing same. Rob is re-working on local port locking which >> may not require lock across fc_exch.c but not sure. >> >> I get your point that in some case upper layers need locking >> across fc_exch.c and perhaps this cannot be avoided. >> > >Hey so what is the exact issue here?
No specific known issue to me in this regard, sees more comments below on this. > Does the network layer sleep We are using dev_alloc_skb() in xmit() paths so network layer sleep is not an issue AFAIK. >or is >it something in our code or is it just a locking dependency issue? The >fc_queuecommand/fc_fcp_send_cmd path can be run from the scsi softirrq >so it cannot sleep. No specific known issue to me and I was just seeking for your comments on calling into fc_exch.c with any lock held v/s not. You have already answered your opinion on this in last response http://www.open-fcoe.org/pipermail/devel/2008-August/000472.html. I was getting impression that you guys are heading for no lock held in fc_exch.c xmit path since :- 1) Earlier you added this code in queuecommand call path :- spin_unlock_irq(lp->host->host_lock); rc = fc_fcp_send_cmd(fsp); spin_lock_irq(lp->host->host_lock); However fps pkt lock is still held in this code path but later you mentioned in one of the email to drop fsp pkt lock and then call fc_exch.c. "I was tinkering with something like del_timer_sync where the fc_fcp.c resp function would hold the pkt lock, set some state bit indicating it is cleaning up the exchange, drop the pkt lock then call a fc_exch.c function to clean up the exachnge" 2) I think Rob is also looking into not to lock local port when sending frame down via fc_exch.c. However not fully sure on this. So I thought you guys are heading for no lock held in fc_exch.c xmit path. Therefore I asked question to get clarification to have lock or no lock before calling into fc_exch.c or you got some plan on this. Now if I look back on thread start and I see that I didn't write enough description of what I was thinking with recent email info and changes from you and sought for your general/design comments in wrong context since I can see fsp pkt is still locked in other places (fc_queuecommand path) apart from recently added fsp pkt lock in fc_lun_reset_send(), so not any issue with this change. My bad for causing any confusion on this. --Vasu _______________________________________________ devel mailing list [email protected] http://www.open-fcoe.org/mailman/listinfo/devel
