>-----Original Message----- >From: Mike Christie [mailto:[EMAIL PROTECTED] >Sent: Tuesday, September 09, 2008 8:50 AM >To: Dev, Vasu >Cc: [email protected] >Subject: Re: [Open-FCoE] [PATCH 2/2] libfc: fixed some more possible >fc_exch_reset() races > >Mike Christie wrote: >> 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 meant to write I think we need to move the ep init code after the >"update exchange" comment in fc_exch_alloc to before we add the ep to >mp->ex_lost.
This is not an issue with ver-2 of this patch which grabs ex_lock first then adds ep to mp->ex_list, so the ep init code can be anywhere with ex lock held but I kept this after em_lock dropped since em_lock is critical in fast path. But yes your comment is valid for this patch. Thanks Mike for comments. _______________________________________________ devel mailing list [email protected] http://www.open-fcoe.org/mailman/listinfo/devel
