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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
