Re: [PATCH 1/3] ipr: Reduce queuecommand lock time

[Brian King <brking@linux.vnet.ibm.com>]

On 07/16/2012 09:03 PM, Matthew Wilcox wrote:
> On Mon, Jul 16, 2012 at 03:48:08PM -0500, Brian King wrote:
>> +static int ipr_queuecommand(struct Scsi_Host *shost,
>> +			    struct scsi_cmnd *scsi_cmd)
>>  {
>>  	struct ipr_ioa_cfg *ioa_cfg;
>>  	struct ipr_resource_entry *res;
>>  	struct ipr_ioarcb *ioarcb;
>>  	struct ipr_cmnd *ipr_cmd;
>> +	unsigned long lock_flags = 0;
> 
> You don't need to initialise lock_flags.
> 
> Looking at the rest of the code, you drop the lock in the middle,
> then re-acquire it.  That'll help with hold time, but I'm not convinced
> it'll help with performance.  Have you done performance testing with
> these changes?  I seem to remember we used an eight-socket box to show
> host_lock problems in the past.

We've done performance testing of these patches and they provided
roughly a 25% increase in the number of IOPs we are able to push
through an adapter on Power. This was running on an 8 socket box with
4 way SMT, so 32 separate hardware threads.

One of the main things these patches do is to get the dma map/unmap
call from underneath the host lock. On Power, these calls have
more overhead than on some other platforms, since they end up
resulting in a hypervisor call, which can significantly increase
host lock hold times. 

I'll resend with the change to not initialize the lock flags.

Thanks,

Brian


-- 
Brian King
Power Linux I/O
IBM Linux Technology Center