2013-08-14 20:52:45

by Mike Miller

[permalink] [raw]
Subject: [PATCH 1/1] cciss: set max scatter gather entries to 32 on P600

Patch 1/1

From: Mike Miller <[email protected]>

At one time we used to set the maximum number of scatter gather elements on
all Smart Array controllers to 32. At some point in time the firmware began
to write the "appropriate" value for each controller into the config table.
The cciss driver would then read that and set h->maxsgentries.

h->maxsgentries = readl(&(h->cfgtable->MaxSGElements);

On the P600 that value is 544. Under some workloads a significant
performance reduction may result. This patch forces the P600 to use only 32
scatter gather elements. Other controllers are not affected.

Signed-off-by: Mike Miller <[email protected]>
Signed-off-by: Dwight (Bud) Brown <[email protected]>
Signed-off-by: Tomas Henzl <[email protected]>
Acked-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 62b6c2c..d2d95ff 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -4258,6 +4258,13 @@ static void cciss_find_board_params(ctlr_info_t *h)
h->nr_cmds = h->max_commands - 4 - cciss_tape_cmds;
h->maxsgentries = readl(&(h->cfgtable->MaxSGElements));
/*
+ * The P600 may exhibit poor performnace under some workloads
+ * if we use the value in the configuration table. Limit this
+ * controller to MAXSGENTRIES (32) instead.
+ */
+ if (h->board_id == 0x3225103C)
+ h->maxsgentries = MAXSGENTRIES;
+ /*
* Limit in-command s/g elements to 32 save dma'able memory.
* Howvever spec says if 0, use 31
*/


2013-08-14 21:27:11

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH 1/1] cciss: set max scatter gather entries to 32 on P600

On Wed, 2013-08-14 at 15:52 -0500, Mike Miller wrote:
> Patch 1/1
>
> From: Mike Miller <[email protected]>
>
> At one time we used to set the maximum number of scatter gather elements on
> all Smart Array controllers to 32. At some point in time the firmware began
> to write the "appropriate" value for each controller into the config table.
> The cciss driver would then read that and set h->maxsgentries.
>
> h->maxsgentries = readl(&(h->cfgtable->MaxSGElements);
>
> On the P600 that value is 544. Under some workloads a significant
> performance reduction may result. This patch forces the P600 to use only 32
> scatter gather elements. Other controllers are not affected.
>
> Signed-off-by: Mike Miller <[email protected]>
> Signed-off-by: Dwight (Bud) Brown <[email protected]>
> Signed-off-by: Tomas Henzl <[email protected]>
> Acked-by: Stephen M. Cameron <[email protected]>

I don't quite understand the signoff chain on this patch. For a one
line patch, are you saying it has three authors?

James

2013-08-14 22:32:59

by Mike Miller

[permalink] [raw]
Subject: RE: [PATCH 1/1] cciss: set max scatter gather entries to 32 on P600



-----Original Message-----
From: James Bottomley [mailto:[email protected]]
Sent: Wednesday, August 14, 2013 4:27 PM
To: Miller, Mike (OS Dev); [email protected]
Cc: Andrew Morton; Jens Axboe; LKML-scsi; LKML; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 1/1] cciss: set max scatter gather entries to 32 on P600

On Wed, 2013-08-14 at 15:52 -0500, Mike Miller wrote:
> Patch 1/1
>
> From: Mike Miller <[email protected]>
>
> At one time we used to set the maximum number of scatter gather
> elements on all Smart Array controllers to 32. At some point in time
> the firmware began to write the "appropriate" value for each controller into the config table.
> The cciss driver would then read that and set h->maxsgentries.
>
> h->maxsgentries = readl(&(h->cfgtable->MaxSGElements);
>
> On the P600 that value is 544. Under some workloads a significant
> performance reduction may result. This patch forces the P600 to use
> only 32 scatter gather elements. Other controllers are not affected.
>
> Signed-off-by: Mike Miller <[email protected]>
> Signed-off-by: Dwight (Bud) Brown <[email protected]>
> Signed-off-by: Tomas Henzl <[email protected]>
> Acked-by: Stephen M. Cameron <[email protected]>

I don't quite understand the signoff chain on this patch. For a one line patch, are you saying it has three authors?

James

The patch origin is unknown. I got it from Tomas and Bud who think it may have originated from HP. I cleaned it up, compile tested it, and sent it on. I'll review my procedures for future patches.

-- mikem

2013-08-15 11:24:07

by Tomas Henzl

[permalink] [raw]
Subject: Re: [PATCH 1/1] cciss: set max scatter gather entries to 32 on P600

On 08/14/2013 11:41 PM, Miller, Mike (OS Dev) wrote:
>
> -----Original Message-----
> From: James Bottomley [mailto:[email protected]]
> Sent: Wednesday, August 14, 2013 4:27 PM
> To: Miller, Mike (OS Dev); [email protected]
> Cc: Andrew Morton; Jens Axboe; LKML-scsi; LKML; [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH 1/1] cciss: set max scatter gather entries to 32 on P600
>
> On Wed, 2013-08-14 at 15:52 -0500, Mike Miller wrote:
>> Patch 1/1
>>
>> From: Mike Miller <[email protected]>
>>
>> At one time we used to set the maximum number of scatter gather
>> elements on all Smart Array controllers to 32. At some point in time
>> the firmware began to write the "appropriate" value for each controller into the config table.
>> The cciss driver would then read that and set h->maxsgentries.
>>
>> h->maxsgentries = readl(&(h->cfgtable->MaxSGElements);
>>
>> On the P600 that value is 544. Under some workloads a significant
>> performance reduction may result. This patch forces the P600 to use
>> only 32 scatter gather elements. Other controllers are not affected.
>>
>> Signed-off-by: Mike Miller <[email protected]>
>> Signed-off-by: Dwight (Bud) Brown <[email protected]>
>> Signed-off-by: Tomas Henzl <[email protected]>
>> Acked-by: Stephen M. Cameron <[email protected]>
> I don't quite understand the signoff chain on this patch. For a one line patch, are you saying it has three authors?

It look a bit funny, but from what I know everybody from that list^ was somehow involved in that issue, debugging..testing, etc,
so seeing the names there is not that bad. But I'm also not the direct author of the patch so feel free to remove my name from the list.

Tomas

>
> James
>
> The patch origin is unknown. I got it from Tomas and Bud who think it may have originated from HP. I cleaned it up, compile tested it, and sent it on. I'll review my procedures for future patches.
>
> -- mikem
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2013-08-15 20:13:42

by Bud Brown

[permalink] [raw]
Subject: Re: [PATCH 1/1] cciss: set max scatter gather entries to 32 on P600


>
> >
> > James
> >
> > The patch origin is unknown. I got it from Tomas and Bud who think it may
> > have originated from HP. I cleaned it up, compile tested it, and sent it
> > on. I'll review my procedures for future patches.
> >
> > -- mikem
> >

Patch originated from me, although Tomas did the heavy lifting via kernel bisect
to localize the specific set of changes causing the performance regression.
Its modeled on the other P600'ism in the driver.

bud