On 10/12/2011 2:32 PM, Vasu Dev wrote:
On Wed, 2011-10-12 at 12:52 -0700, Bhanu Prakash Gollapudi wrote:
On 10/12/2011 10:16 AM, Vasu Dev wrote:
On Fri, 2011-09-30 at 15:36 -0700, Bhanu Prakash Gollapudi wrote:
+static int fc_exch_abort_locked(struct fc_exch *ep,
+                             unsigned int timer_msec)

Vasu, this patch causes a deadlock when the fcoe interface is
destroyed.
The reason is, holding the exch_lock, we try to hold it again in
fc_seq_send that is called in this context, causing the deadlock. (We
saw this issue with latest net-next tree).



Were you seeing the actual deadlock or only the log as possible deadlock
from LOCK_DEP ?  I don't see possibility of double locking in this code
path due to added this conditional in fc_seq_send() before trying to
grab lock here:-

          if (fh->fh_type == FC_TYPE_BLS)
                  return error;


The FC_TYPE_BLS is always set in this code path before calling
fc_seq_send(),  therefore possibly just LOCK_DEP noise in this case, am
I right ?

Vasu,

It is not LOCK_DEP noise.

I think your intention of exiting out if it is ABTS was right, but the
condition does not work, as the skb would have been freed after

I see this is the root cause here and and that still needs to be fixed
and as this issue remains with recent patch also doing exch lock drop in
the caller and grabbing again. I'd avoid drop-grab-drop-grap exch lock
sequence while exch lock already held here,  it complicates.

I agree.


frame_send is done, and fh_type will have some junk value. If you want
to implement it this way, you have to cache the fh_type before calling
frame_send.


Yes caching is correct fix here and that fix should not cause any
LOCKDEP issues also for this code path, I'll check that but if you could
also then that would great with patch pasted below.

I already verified this. Good to go.


Thanks
Vasu


diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 7c055fd..81235f3 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -469,6 +469,7 @@ static int fc_seq_send(struct fc_lport *lport,
struct fc_seq *sp,
         struct fc_frame_header *fh = fc_frame_header_get(fp);
         int error;
         u32 f_ctl;
+       u8 fh_type = fh->fh_type;

         ep = fc_seq_exch(sp);
         WARN_ON((ep->esb_stat&  ESB_ST_SEQ_INIT) != ESB_ST_SEQ_INIT);
@@ -493,7 +494,7 @@ static int fc_seq_send(struct fc_lport *lport,
struct fc_seq *sp,
          */
         error = lport->tt.frame_send(lport, fp);

-       if (fh->fh_type == FC_TYPE_BLS)
+       if (fh_type == FC_TYPE_BLS)
                 return error;


_______________________________________________
devel mailing list
[email protected]
https://lists.open-fcoe.org/mailman/listinfo/devel

Reply via email to