Re: [patch] Intel SATA controller hiccups, locking

On 20.07.2011 23:36, Andrew Boyer wrote:
> Thank you for looking into it!  I would really like to get Alexander's feedback before it gets committed.

Generally it looks good, but I would tune few minor places. I'll look on 
it closer tomorrow. Thank you.

> On Jul 20, 2011, at 4:18 PM, Vogel, Jack wrote:
>
>> Ran it by the chipset contact internally and he said go for it, you need me to check it in Andrew?
>>
>> -----Original Message-----
>> From: Andrew Boyer [mailto:aboyer_at_averesystems.com]
>> Sent: Wednesday, July 20, 2011 7:45 AM
>> To: mav_at_freebsd.org
>> Cc: freebsd-current_at_freebsd.org; Vogel, Jack
>> Subject: [patch] Intel SATA controller hiccups, locking
>>
>> Hello Alexander,
>> I am using the latest ata driver from stable/8 on a system with an Intel ICH10 controller.  ATA_CAM and ATA_STATIC_ID are both off.  There is one drive connected to port 3.  SATA is set to Enhanced / IDE mode (not AHCI) in the BIOS.
>>> atapci0_at_pci0:0:31:2:	class=0x01018f card=0x060d15d9 chip=0x3a208086 rev=0x00 hdr=0x00
>>> atapci1_at_pci0:0:31:5:	class=0x010185 card=0x060d15d9 chip=0x3a268086 rev=0x00 hdr=0x00
>>
>>> atapci0:<Intel ICH10 SATA300 controller>  port 0xbff0-0xbff7,0xbf7c-0xbf7f,0xbfe0-0xbfe7,0xbef4-0xbef7,0xbfa0-0xbfaf,0xbf60-0xbf6f irq 19 at device 31.2 on pci0
>>> atapci0: Reserved 0x10 bytes for rid 0x20 type 4 at 0xbfa0
>>> atapci0: [MPSAFE]
>>> atapci0: [ITHREAD]
>>> atapci0: Reserved 0x10 bytes for rid 0x24 type 4 at 0xbf60
>>> ata2:<ATA channel 0>  on atapci0
>>> atapci0: Reserved 0x8 bytes for rid 0x10 type 4 at 0xbff0
>>> atapci0: Reserved 0x4 bytes for rid 0x14 type 4 at 0xbf7c
>>> ata2: SATA reset: ports status=0x00
>>> ata2: p0: SATA connect timeout status=00000000
>>> ata2: p1: SATA connect timeout status=00000000
>>> ata2: [MPSAFE]
>>> ata2: [ITHREAD]
>>> ata3:<ATA channel 1>  on atapci0
>>> atapci0: Reserved 0x8 bytes for rid 0x18 type 4 at 0xbfe0
>>> atapci0: Reserved 0x4 bytes for rid 0x1c type 4 at 0xbef4
>>> ata3: SATA reset: ports status=0x08
>>> ata3: p0: SATA connect timeout status=00000000
>>> ata3: p1: SATA connect time=0ms status=00000123
>>> ata3: reset tp1 mask=03 ostat0=7f ostat1=50
>>> ata3: stat0=0x7f err=0x00 lsb=0xff msb=0xff
>>> ata3: stat1=0x50 err=0x01 lsb=0x00 msb=0x00
>>> ata3: reset tp2 stat0=7f stat1=50 devices=0x2
>>> ata3: [MPSAFE]
>>> ata3: [ITHREAD]
>>
>>
>> When under heavy load, the 'atacontrol mode ad0' command sometimes fails to determine the SATA speed; the drive appears to be missing.  I think the root cause is that chipsets/ata-intel.c does not do any locking on the ata_intel_sata_sidpr_* routines.  The (write address register) + (access data register) model isn't safe without locking because two channels share the registers.  The ata_intel_sata_cscr_* routines have the same problem.
>>
>> Adding a mutex to a structure stored in ctlr->chipset_data makes the hiccups go away; see the attached patch.  Please advise if this is something you would like to fix.

-- 
Alexander Motin