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

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

[Mike Miller]

Patch 1/1

From: Mike Miller <mike.miller@hp.com>

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 <mike.miller@hp.com>
Signed-off-by: Dwight (Bud) Brown <bubrown@redhat.com>
Signed-off-by: Tomas Henzl <thenzl@redhat.com>
Acked-by: Stephen M. Cameron <steve.cameron@hp.com>
---
 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
 	 */

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

[James Bottomley]

On Wed, 2013-08-14 at 15:52 -0500, Mike Miller wrote:
> Patch 1/1
> 
> From: Mike Miller <mike.miller@hp.com>
> 
> 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 <mike.miller@hp.com>
> Signed-off-by: Dwight (Bud) Brown <bubrown@redhat.com>
> Signed-off-by: Tomas Henzl <thenzl@redhat.com>
> Acked-by: Stephen M. Cameron <steve.cameron@hp.com>

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

James

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

[Miller, Mike (OS Dev)]

-----Original Message-----
From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com] 
Sent: Wednesday, August 14, 2013 4:27 PM
To: Miller, Mike (OS Dev); mikem@thumper.usa.hp.com
Cc: Andrew Morton; Jens Axboe; LKML-scsi; LKML; thenzl@redhat.com; bubrown@redhat.com; scameron@beardog.cce.hp.com
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 <mike.miller@hp.com>
> 
> 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 <mike.miller@hp.com>
> Signed-off-by: Dwight (Bud) Brown <bubrown@redhat.com>
> Signed-off-by: Tomas Henzl <thenzl@redhat.com>
> Acked-by: Stephen M. Cameron <steve.cameron@hp.com>

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

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

[Tomas Henzl]

On 08/14/2013 11:41 PM, Miller, Mike (OS Dev) wrote:
>
> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com] 
> Sent: Wednesday, August 14, 2013 4:27 PM
> To: Miller, Mike (OS Dev); mikem@thumper.usa.hp.com
> Cc: Andrew Morton; Jens Axboe; LKML-scsi; LKML; thenzl@redhat.com; bubrown@redhat.com; scameron@beardog.cce.hp.com
> 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 <mike.miller@hp.com>
>>
>> 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 <mike.miller@hp.com>
>> Signed-off-by: Dwight (Bud) Brown <bubrown@redhat.com>
>> Signed-off-by: Tomas Henzl <thenzl@redhat.com>
>> Acked-by: Stephen M. Cameron <steve.cameron@hp.com>
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Bud Brown]

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