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

Dev, Vasu vasu.dev at intel.com
Wed Sep 10 17:56:56 UTC 2008


>-----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.



More information about the devel mailing list