Re: [PATCH] sata_sx4: speed up ECC initialization

[Jeff Garzik <jeff@garzik.org>]

Alexander Beregalov wrote:
> On Tue, Apr 14, 2009 at 07:57:57AM -0400, Jeff Garzik wrote:
>> Alexander Beregalov wrote:
>>> ECC initialization takes too long. It writes zeroes by portions of 4
>>> byte, it takes more than 6 minutes to initialize 512Mb DIMM module.
>>> Change portion to 1Mb.
>>>
>>> before:
>>> [10857.207576] pdc20621_dimm_init: Start ECC initialization
>>> [11235.333118] pdc20621_dimm_init: Finish ECC initialization
>>>
>>> after:
>>> [ 1005.126437] pdc20621_dimm_init: Local DIMM Speed = 100
>>> [ 1005.128111] pdc20621_dimm_init: Local DIMM Size = 512MB
>>> [ 1005.133508] Local DIMM ECC Enabled
>>> [ 1005.236482] pdc20621_dimm_init: Start ECC initialization
>>> [ 1031.278098] pdc20621_dimm_init: Finish ECC initialization
>>>
>> Comments:
>>
>> 1) on an older machine, where these things might be found, 1MB might be 
>> a lot -- particularly if there is a lot of VM fragmentation.  Please 
>> reduce the size a bit -- I suppose 128k is not unbearably slow?
>>
>> 2) Rather than repeating 'page size * 32' calculation multiple times, 
>> put it into a named constant somewhere.
>>
>> Other than that, looks ok!
> 
> 
>>From 00e980a4651ef12811fbe9a893a7e72510b955d1 Mon Sep 17 00:00:00 2001
> From: Alexander Beregalov <a.beregalov@gmail.com>
> Date: Tue, 14 Apr 2009 23:55:52 +0400
> Subject: [PATCH v2] sata_sx4: speed up ECC initialization
> 
> ECC initialization takes too long. It writes zeroes by portions
> of 4 byte, it takes more than 6 minutes on my machine to initialize
> 512Mb ECC DIMM module. Change portion to 128Kb - it significantly
> reduces initialization time.
> 
> Signed-off-by: Alexander Beregalov <a.beregalov@gmail.com>
> ---
>  drivers/ata/sata_sx4.c |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)


Applied, to libata-dev.git#upstream

Thanks!

So...   the $64,000 question:  does sata_sx4 work for you?


> diff --git a/drivers/ata/sata_sx4.c b/drivers/ata/sata_sx4.c
> index dce3dcc..197a4d7 100644
> --- a/drivers/ata/sata_sx4.c
> +++ b/drivers/ata/sata_sx4.c
> @@ -193,6 +193,7 @@ enum {
>  					  PDC_TIMER_MASK_INT,
>  };
>  
> +#define ECC_ERASE_BUF_SZ (128 * 1024)

I made two minor edits:

1) Macros should always be guarded by parens

2) Added "_SZ" suffix to make purpose even more obvious