[Open-FCoE] [PATCH 2/2] libfc: fixed some more possible fc_exch_reset() races

Mike Christie michaelc at cs.wisc.edu
Wed Sep 10 17:57:13 UTC 2008


Dev, Vasu wrote:
>> -----Original Message-----
>> From: Mike Christie [mailto:michaelc at cs.wisc.edu]
>> Sent: Tuesday, September 09, 2008 8:50 AM
>> To: Dev, Vasu
>> Cc: devel at open-fcoe.org
>> 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.



More information about the devel mailing list