Dev, Vasu wrote: >> -----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. >
Yeah, I saw ver2. It looks good. Sorry for the screw up. _______________________________________________ devel mailing list [email protected] http://www.open-fcoe.org/mailman/listinfo/devel
