Data corruption when using INIC-1623TA2 controller

Data corruption when using INIC-1623TA2 controller

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 4493 bytes --]

Martin Braure de Calignon wrote:
> I'm experiencing frequent data corruption on my raid1 ext4 fs.
> The error is not always the same.
> 
> I first thought it was due to a previous resize of the FS I've done.
> I had multiple times some message about huge amount of multiply claimed blocks in inode xxxx.
> fsck.ext4 was not working fully and was always ending with a message like "FS still have error".
> I was unable to copy all the files to another FS to save it.
> So I end up checking the badblocks (that's where I've been dumb, I choose a non data conservative way).
> However, badblock was succesful without errors.
> So in the end I lost some data, however, I don't know if it's due to the bug or the the badblocks check. So feel free to readjust severity.
> 
> Since theni, I have bought two brand new disks, created a completly new ext4 FS, and copied the files that I had succesfully recovered.
> Then I run fsck.ext4 on the FS... it seems it is almost working.
> I'm remounting the /dev/md0... And each time I start using the system seriously, I have new errors, like the one I had today:
> (I was just copying files on it)
[...]
> [1436849.120036] EXT4-fs (md0): error count: 6
> [1436849.120044] EXT4-fs (md0): initial error at 1371763084: htree_dirblock_to_tree:587: inode 20971803: block 83894316
> [1436849.120054] EXT4-fs (md0): last error at 1371765809: htree_dirblock_to_tree:587: inode 41813096: block 167256110
> [1446656.923648] EXT4-fs error (device md0): htree_dirblock_to_tree:587: inode #52698372: block 210773049: comm smbd: bad entry in directory: directory entry across blocks - offset=1052(9244), inode=1949184565, rec_len=29816, name_len=24
[...]

The kernel log also showed the CPU was reaching its temperature limit,
but after he cleaned out the CPU cooler and corrected the CPU frequency
the problem persisted.  I suggested swapping disks between controllers:

On Fri, 2013-06-28 at 15:55 +0200, Martin Braure de Calignon wrote:
[...]
> So as planned I unplugged the working non RAID1 disk from their
> controller, and connect the ext4 RAID1 and the ext3 RAID1 disk to it
> (yeah these are 2 powerful RAID1 with 1 device only ;) for testing
> purposes).
> I also tried to re-plug each PCI card, and connect the video card fan
> that was not connected (yeah it was a bad idea to limit the noise level
> few years ago).
> 
> I did all the tests I could to try to overheat the system (same as
> yesterday):
> * 4 running dd if=/dev/urandom | gzip >/dev/null for the cpu
> * massive copy from one disk to the other 
> * delete of duplicates between two directories (with many duplicates)
> 
> All that in parallel. Everything seems to work fine. No corruption nor
> CPU overheating message (yesterday I still had some even after remove
> the overclock of the CPU).
[...]
> Here's the lspci -vvvv for this card (if I'm not wrong):
> 
>                 02:09.0 SATA controller: Initio Corporation INI-1623 PCI
>                 SATA-II Controller (rev 02) (prog-if 00 [Vendor
>                 specific])
>                         Subsystem: Initio Corporation Device 1626
>                         Control: I/O+ Mem+ BusMaster+ SpecCycle-
>                 MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
>                 DisINTx-
>                         Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr-
>                 DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
>                 INTx-
>                         Latency: 32, Cache Line Size: 32 bytes
>                         Interrupt: pin A routed to IRQ 17
>                         Region 0: I/O ports at 9000 [size=256]
>                         Region 1: Memory at ef022000 (32-bit,
>                 non-prefetchable) [size=4K]
>                         [virtual] Expansion ROM at 80000000 [disabled]
>                 [size=128K]
>                         Capabilities: [dc] Power Management version 2
>                                 Flags: PMEClk+ DSI- D1+ D2+
>                 AuxCurrent=0mA PME(D0-,D1+,D2+,D3hot+,D3cold-)
>                                 Status: D0 NoSoftRst- PME-Enable- DSel=0
>                 DScale=0 PME-
>                         Kernel driver in use: sata_inic162x
[...]

So this does seem to be a fault in either this card or the driver.  Can
you suggest any further tests that Martin could do?

Ben.

-- 
Ben Hutchings
Sturgeon's Law: Ninety percent of everything is crap.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: Data corruption when using INIC-1623TA2 controller

[Tejun Heo]

Hello,

On Sun, Jun 30, 2013 at 03:49:24PM +0100, Ben Hutchings wrote:
> So this does seem to be a fault in either this card or the driver.  Can
> you suggest any further tests that Martin could do?

Unfortunately, I don't have any idea.  That driver never really
matured enough.  I couldn't find enough information and no one from
initio responded, so....

-- 
tejun

Re: Data corruption when using INIC-1623TA2 controller

[Martin Braure de Calignon]

[-- Attachment #1: Type: text/plain, Size: 827 bytes --]

Hello,

On Sun., 2013-06-30 at 23:17 -0700, Tejun Heo wrote:
> Unfortunately, I don't have any idea.  That driver never really
> matured enough.  I couldn't find enough information and no one from
> initio responded, so....

Thank you Tejun and Ben,

that's totally suxx. I'm gonna try to contact them too, but I doubt it's
gonna change anything :(
I was hopping that we could turn on some logging so that we understand,
at least, what is happening (even if we can't solve it), and I could
then have made some tests by modifying code, recompiling module, ...
If the driver could cause data loss, shouldn't it be flagged as
experimental?
In the meantime, I'm probably going to purchase a new SATA card :(

Thanks again guys for your hard work! I really appreciate!

Martin

-- 
Martin Braure de Calignon

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH libata/for-3.11-fixes] libata: make it clear that sata_inic162x is experimental

[Tejun Heo]

Hello, guys.

I've committed the following patch to for-3.11-fixes.  It's
unfortunate but I can't see a good way out at this point.  :(

Thanks.

------------- 8< ----------------

>From bb9696192826a7d9279caf872e95b41bc26c7eff Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj@kernel.org>
Date: Mon, 22 Jul 2013 16:53:36 -0400

sata_inic162x never reached a state where it's reliable enough for
production use and data corruption is a relatively common occurrence.
Make the driver generate warning about the issues and mark the Kconfig
option as experimental.

If the situation doesn't improve, we'd be better off making it depend
on CONFIG_BROKEN.  Let's wait for several cycles and see if the kernel
message draws any attention.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Martin Braure de Calignon <braurede@free.fr>
Reported-by: Ben Hutchings <ben@decadent.org.uk>
Reported-by: risc4all@yahoo.com
---
 drivers/ata/Kconfig         |  2 +-
 drivers/ata/sata_inic162x.c | 14 ++++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 80dc988..5cddaf8 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -107,7 +107,7 @@ config SATA_FSL
 	  If unsure, say N.
 
 config SATA_INIC162X
-	tristate "Initio 162x SATA support"
+	tristate "Initio 162x SATA support (Very Experimental)"
 	depends on PCI
 	help
 	  This option enables support for Initio 162x Serial ATA.
diff --git a/drivers/ata/sata_inic162x.c b/drivers/ata/sata_inic162x.c
index e451317..5c54d95 100644
--- a/drivers/ata/sata_inic162x.c
+++ b/drivers/ata/sata_inic162x.c
@@ -6,6 +6,18 @@
  *
  * This file is released under GPL v2.
  *
+ * **** WARNING ****
+ *
+ * This driver never worked properly and unfortunately data corruption is
+ * relatively common.  There isn't anyone working on the driver and there's
+ * no support from the vendor.  Do not use this driver in any production
+ * environment.
+ *
+ * http://thread.gmane.org/gmane.linux.debian.devel.bugs.rc/378525/focus=54491
+ * https://bugzilla.kernel.org/show_bug.cgi?id=60565
+ *
+ * *****************
+ *
  * This controller is eccentric and easily locks up if something isn't
  * right.  Documentation is available at initio's website but it only
  * documents registers (not programming model).
@@ -807,6 +819,8 @@ static int inic_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	ata_print_version_once(&pdev->dev, DRV_VERSION);
 
+	dev_alert(&pdev->dev, "inic162x support is broken with common data corruption issues and will be disabled by default, contact linux-ide@vger.kernel.org if in production use\n");
+
 	/* alloc host */
 	host = ata_host_alloc_pinfo(&pdev->dev, ppi, NR_PORTS);
 	hpriv = devm_kzalloc(&pdev->dev, sizeof(*hpriv), GFP_KERNEL);
-- 
1.8.3.1

Re: [PATCH libata/for-3.11-fixes] libata: make it clear that sata_inic162x is experimental

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 419 bytes --]

On Mon, 2013-07-22 at 17:13 -0400, Tejun Heo wrote:
> Hello, guys.
> 
> I've committed the following patch to for-3.11-fixes.  It's
> unfortunate but I can't see a good way out at this point.  :(
[...]

This change is also appropriate for a stable update.

I will also remove the driver from Debian packages.

Ben.

-- 
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH libata/for-3.11-fixes] libata: make it clear that sata_inic162x is experimental

[Tejun Heo]

On Fri, Jul 26, 2013 at 05:57:20AM +0100, Ben Hutchings wrote:
> On Mon, 2013-07-22 at 17:13 -0400, Tejun Heo wrote:
> > Hello, guys.
> > 
> > I've committed the following patch to for-3.11-fixes.  It's
> > unfortunate but I can't see a good way out at this point.  :(
> [...]
> 
> This change is also appropriate for a stable update.

Indeed.  Will forward the patch to Greg.

> I will also remove the driver from Debian packages.

Yeap, please do so.

Thanks!

-- 
tejun

Re: [PATCH libata/for-3.11-fixes] libata: make it clear that sata_inic162x is experimental

[Tejun Heo]

Hello, Greg.

I think the following commit should go into -stable but forgot to cc
stable.  It's now in Linus' tree.  Can you please include it in
-stable?

Thanks!

On Mon, Jul 22, 2013 at 05:13:07PM -0400, Tejun Heo wrote:
> From bb9696192826a7d9279caf872e95b41bc26c7eff Mon Sep 17 00:00:00 2001
> From: Tejun Heo <tj@kernel.org>
> Date: Mon, 22 Jul 2013 16:53:36 -0400
> 
> sata_inic162x never reached a state where it's reliable enough for
> production use and data corruption is a relatively common occurrence.
> Make the driver generate warning about the issues and mark the Kconfig
> option as experimental.
> 
> If the situation doesn't improve, we'd be better off making it depend
> on CONFIG_BROKEN.  Let's wait for several cycles and see if the kernel
> message draws any attention.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: Martin Braure de Calignon <braurede@free.fr>
> Reported-by: Ben Hutchings <ben@decadent.org.uk>
> Reported-by: risc4all@yahoo.com
> ---
>  drivers/ata/Kconfig         |  2 +-
>  drivers/ata/sata_inic162x.c | 14 ++++++++++++++
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index 80dc988..5cddaf8 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -107,7 +107,7 @@ config SATA_FSL
>  	  If unsure, say N.
>  
>  config SATA_INIC162X
> -	tristate "Initio 162x SATA support"
> +	tristate "Initio 162x SATA support (Very Experimental)"
>  	depends on PCI
>  	help
>  	  This option enables support for Initio 162x Serial ATA.
> diff --git a/drivers/ata/sata_inic162x.c b/drivers/ata/sata_inic162x.c
> index e451317..5c54d95 100644
> --- a/drivers/ata/sata_inic162x.c
> +++ b/drivers/ata/sata_inic162x.c
> @@ -6,6 +6,18 @@
>   *
>   * This file is released under GPL v2.
>   *
> + * **** WARNING ****
> + *
> + * This driver never worked properly and unfortunately data corruption is
> + * relatively common.  There isn't anyone working on the driver and there's
> + * no support from the vendor.  Do not use this driver in any production
> + * environment.
> + *
> + * http://thread.gmane.org/gmane.linux.debian.devel.bugs.rc/378525/focus=54491
> + * https://bugzilla.kernel.org/show_bug.cgi?id=60565
> + *
> + * *****************
> + *
>   * This controller is eccentric and easily locks up if something isn't
>   * right.  Documentation is available at initio's website but it only
>   * documents registers (not programming model).
> @@ -807,6 +819,8 @@ static int inic_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	ata_print_version_once(&pdev->dev, DRV_VERSION);
>  
> +	dev_alert(&pdev->dev, "inic162x support is broken with common data corruption issues and will be disabled by default, contact linux-ide@vger.kernel.org if in production use\n");
> +
>  	/* alloc host */
>  	host = ata_host_alloc_pinfo(&pdev->dev, ppi, NR_PORTS);
>  	hpriv = devm_kzalloc(&pdev->dev, sizeof(*hpriv), GFP_KERNEL);
> -- 
> 1.8.3.1
> 

-- 
tejun

Re: [PATCH libata/for-3.11-fixes] libata: make it clear that sata_inic162x is experimental

[Greg Kroah-Hartman]

On Mon, Jul 29, 2013 at 12:52:29PM -0400, Tejun Heo wrote:
> Hello, Greg.
> 
> I think the following commit should go into -stable but forgot to cc
> stable.  It's now in Linus' tree.  Can you please include it in
> -stable?

Now applied, thanks.

greg k-h

Re: [PATCH libata/for-3.11-fixes] libata: make it clear that sata_inic162x is experimental

[Luis Henriques]

Tejun Heo <tj@kernel.org> writes:

> Hello, Greg.
>
> I think the following commit should go into -stable but forgot to cc
> stable.  It's now in Linus' tree.  Can you please include it in
> -stable?
>
> Thanks!
>
> On Mon, Jul 22, 2013 at 05:13:07PM -0400, Tejun Heo wrote:
>> From bb9696192826a7d9279caf872e95b41bc26c7eff Mon Sep 17 00:00:00 2001
>> From: Tejun Heo <tj@kernel.org>
>> Date: Mon, 22 Jul 2013 16:53:36 -0400
>> 
>> sata_inic162x never reached a state where it's reliable enough for
>> production use and data corruption is a relatively common occurrence.
>> Make the driver generate warning about the issues and mark the Kconfig
>> option as experimental.
>> 
>> If the situation doesn't improve, we'd be better off making it depend
>> on CONFIG_BROKEN.  Let's wait for several cycles and see if the kernel
>> message draws any attention.
>> 
>> Signed-off-by: Tejun Heo <tj@kernel.org>
>> Reported-by: Martin Braure de Calignon <braurede@free.fr>
>> Reported-by: Ben Hutchings <ben@decadent.org.uk>
>> Reported-by: risc4all@yahoo.com
>> ---
>>  drivers/ata/Kconfig         |  2 +-
>>  drivers/ata/sata_inic162x.c | 14 ++++++++++++++
>>  2 files changed, 15 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
>> index 80dc988..5cddaf8 100644
>> --- a/drivers/ata/Kconfig
>> +++ b/drivers/ata/Kconfig
>> @@ -107,7 +107,7 @@ config SATA_FSL
>>  	  If unsure, say N.
>>  
>>  config SATA_INIC162X
>> -	tristate "Initio 162x SATA support"
>> +	tristate "Initio 162x SATA support (Very Experimental)"
>>  	depends on PCI
>>  	help
>>  	  This option enables support for Initio 162x Serial ATA.
>> diff --git a/drivers/ata/sata_inic162x.c b/drivers/ata/sata_inic162x.c
>> index e451317..5c54d95 100644
>> --- a/drivers/ata/sata_inic162x.c
>> +++ b/drivers/ata/sata_inic162x.c
>> @@ -6,6 +6,18 @@
>>   *
>>   * This file is released under GPL v2.
>>   *
>> + * **** WARNING ****
>> + *
>> + * This driver never worked properly and unfortunately data corruption is
>> + * relatively common.  There isn't anyone working on the driver and there's
>> + * no support from the vendor.  Do not use this driver in any production
>> + * environment.
>> + *
>> + * http://thread.gmane.org/gmane.linux.debian.devel.bugs.rc/378525/focus=54491
>> + * https://bugzilla.kernel.org/show_bug.cgi?id=60565
>> + *
>> + * *****************
>> + *
>>   * This controller is eccentric and easily locks up if something isn't
>>   * right.  Documentation is available at initio's website but it only
>>   * documents registers (not programming model).
>> @@ -807,6 +819,8 @@ static int inic_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>  
>>  	ata_print_version_once(&pdev->dev, DRV_VERSION);
>>  
>> +	dev_alert(&pdev->dev, "inic162x support is broken with common data corruption issues and will be disabled by default, contact linux-ide@vger.kernel.org if in production use\n");
>> +
>>  	/* alloc host */
>>  	host = ata_host_alloc_pinfo(&pdev->dev, ppi, NR_PORTS);
>>  	hpriv = devm_kzalloc(&pdev->dev, sizeof(*hpriv), GFP_KERNEL);
>> -- 
>> 1.8.3.1
>> 

I'm queuing this for the 3.5 kernel as well.

Cheers,
-- 
Luis

Re: [PATCH libata/for-3.11-fixes] libata: make it clear that sata_inic162x is experimental

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 619 bytes --]

On Mon, 2013-07-29 at 12:52 -0400, Tejun Heo wrote:
> Hello, Greg.
> 
> I think the following commit should go into -stable but forgot to cc
> stable.  It's now in Linus' tree.  Can you please include it in
> -stable?
> 
> Thanks!
> 
> On Mon, Jul 22, 2013 at 05:13:07PM -0400, Tejun Heo wrote:
> > From bb9696192826a7d9279caf872e95b41bc26c7eff Mon Sep 17 00:00:00 2001
> > From: Tejun Heo <tj@kernel.org>
> > Date: Mon, 22 Jul 2013 16:53:36 -0400
[...]

I've queued this up for 3.2, thanks.

Ben.

-- 
Ben Hutchings
If God had intended Man to program,
we'd have been born with serial I/O ports.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]