Vasu Dev wrote:
> /*
> @@ -553,6 +553,7 @@ struct fc_exch *fc_exch_alloc(struct fc_exch_mgr *mp, u16
> xid)
> fc_seq_alloc(ep, ep->seq_id++);
> mp->total_exches++;
> spin_unlock_bh(&mp->em_lock);
> + fc_exch_hold(ep); /* hold for exch in mp */
>
> /*
> * update exchange
> @@ -567,7 +568,12 @@ struct fc_exch *fc_exch_alloc(struct fc_exch_mgr *mp,
> u16 xid)
> spin_lock_init(&ep->ex_lock);
> setup_timer(&ep->ex_timer, fc_exch_timeout, (unsigned long)ep);
>
> - fc_exch_hold(ep); /* hold for caller */
> + /*
> + * Hold exch lock for caller to prevent
> + * fc_exch_reset() from releasing exch
> + * while caller is still working on exch.
> + */
> + spin_lock_bh(&ep->ex_lock);
I think we need to move the fc_exch_hold and ep initialization (code
after the "update exchange" comment in fc_exch_alloc to before where add
the ep to the mp->ex_list.
I am thinking about if fc_exch_mgr_reset runs right after fc_exch_alloc
does this:
mp->exches[xid - min_xid] = ep;
list_add_tail(&ep->ex_list, &mp->ex_list);
fc_seq_alloc(ep, ep->seq_id++);
mp->total_exches++;
spin_unlock_bh(&mp->em_lock);
fc_exch_mgr_reset will see the ep in the ex_list and call fc_exch_reset,
but the ep is not fully setup at this time, so when fc_exch_reset tries
to grab the ex_lock we should get an error about using a spin lock that
is not inited.
Do we also need to add a check for FC_EX_RST_CLEANUP in fc_exch_alloc
after we have grabbed the ex_lock in case after fc_exch_alloc dropped
the em_lock, fc_exch_mgr_reset had grabbed the em_lock, seen the ep in
the ex_list and called fc_exch_reset on it?
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
