PERFORCE change 82377 for review

Scott Long scottl at samsco.org
Sun Aug 21 22:10:22 GMT 2005


Nate Lawson wrote:

> Scott Long wrote:
> 
>> http://perforce.freebsd.org/chv.cgi?CH=82377
>>
>> Change 82377 by scottl at scottl-junior on 2005/08/21 17:13:18
>>
>>     Introduce the topology lock.  It covers the reference counting of
>>     path components so that peripherals and sims can use path objects
>>     without having to lock them.
>>
>> @@ -4808,6 +4839,7 @@
>>                        sim->c_handle);
>>                  sim->flags &= ~CAM_SIM_REL_TIMEOUT_PENDING;
>>              }
>> +            mtx_lock(&cam_topo_lock);
>>              bus = xpt_find_bus(sim->path_id);
>>              splx(s);
>>  
>> @@ -4815,9 +4847,12 @@
>>                  /*
>>                   * Now that we are unfrozen run the send queue.
>>                   */
>> +                mtx_unlock(&cam_topo_lock);
>>                  xpt_run_dev_sendq(bus);
>> +                mtx_lock(&cam_topo_lock);
>>              }
>>              xpt_release_bus(bus);
>> +            mtx_unlock(&cam_topo_lock);
>>          } else
>>              splx(s);
>>      } else
> 
> 
> I've heard of some performance problems from unlocking and relocking in 
> the xpt_start() path for each ccb.  I do that in scsi_target.  Since 
> this is only once per bus, this may be fine here.

I don't like what I did in here either.  I need to think about it more.

> 
> There is also a dangling splx() above and a few others left that were 
> obsoleted by your lock.
> 

No.  The sendq is still only protected by Giant, so the spls are still
valid markers.

Scott


More information about the p4-projects mailing list