Re: Hotplug borked after suspend/resume in Linux-3.3 ?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Lin,

In the patch d9027470b88631d0956ac37cdadfdeb9cdcf2c99, I did limit the
amount of data cleaning in some of the ata objects.

If setting err_mask to 0 masks the regression I introduced in the
patch, I may have altered too much how ata_device object is
reinitialized when a device is found. I am digging deeper, I may have
change the code to try to preserve the ering as much as possible.

Concerning your patch, isn't adding a test (ent->eflags &
ATA_EFLAG_OLD_ER) in ata_count_probe_trials_cb() more in line with
speed_down_verdict_cb() code?

Gwendal.

On Fri, Apr 20, 2012 at 10:37 AM, Grant Grundler <grundler@xxxxxxxxxx> wrote:
> On Thu, Apr 19, 2012 at 1:16 AM, Lin Ming <ming.m.lin@xxxxxxxxx> wrote:
> ...
>> I did bisect and found that this is a really old regression introduced
>> in 2.6.37-rc1 with below commit.
>>
>> commit d9027470b88631d0956ac37cdadfdeb9cdcf2c99
>> Author: Gwendal Grignou <gwendal@xxxxxxxxxx>
>> Date:   Tue May 25 12:31:38 2010 -0700
>>
>>    [libata] Add ATA transport class
>>
>>    This is a scheleton for libata transport class.
>>    All information is read only, exporting information from libata:
>>    - ata_port class: one per ATA port
>>    - ata_link class: one per ATA port or 15 for SATA Port Multiplier
>>    - ata_device class: up to 2 for PATA link, usually one for SATA.
>>
>>    Signed-off-by: Gwendal Grignou <gwendal@xxxxxxxxxx>
>>    Reviewed-by: Grant Grundler <grundler@xxxxxxxxxx>
>>    Signed-off-by: Jeff Garzik <jgarzik@xxxxxxxxxx>
>>
>>
>> Here is the patch to fix it.
>>
>> Gwendal and Grant,
>>
>> Would you help to review it?
>>
>>
>> From f696daec7ff63e9b3697e8f7ef8f985152667965 Mon Sep 17 00:00:00 2001
>> From: Lin Ming <ming.m.lin@xxxxxxxxx>
>> Date: Thu, 19 Apr 2012 15:45:51 +0800
>> Subject: [PATCH] libata: clear error mask of old error history
>>
>> The old error history was cleared in ata_ering_clear().
>> It only sets ATA_EFLAG_OLD_ER eflags, but the err_mask was not cleared.
>> So ata_ering_map() still iterates the old error history.
>>
>> This causes problem, for example, wrong probe trials count were returned in
>> ata_eh_schedule_probe(), which in turn causes SATA link speed to be slowed down
>> to 1.5Gbps.
>>
>> Reported-by: Martin Mokrejs <mmokrejs@xxxxxxxxxxxxxxxxxx>
>> Signed-off-by: Lin Ming <ming.m.lin@xxxxxxxxx>
>
> Reviewed-by: Grant Grundler <grundler@xxxxxxxxxx>
>
> LGTM. Caveat is I have looked at libata code only once or twice since
> reviewing this patch for Gwendal.
>
> cheers,
> grant
>
>> ---
>>  drivers/ata/libata-eh.c |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
>> index c61316e..4c6f49b 100644
>> --- a/drivers/ata/libata-eh.c
>> +++ b/drivers/ata/libata-eh.c
>> @@ -419,9 +419,10 @@ int ata_ering_map(struct ata_ering *ering,
>>        return rc;
>>  }
>>
>> -int ata_ering_clear_cb(struct ata_ering_entry *ent, void *void_arg)
>> +static int ata_ering_clear_cb(struct ata_ering_entry *ent, void *void_arg)
>>  {
>>        ent->eflags |= ATA_EFLAG_OLD_ER;
>> +       ent->err_mask = 0;
>>        return 0;
>>  }
>>
>> --
>> 1.7.2.5
>>
>>
>>
>>>
>>> [  146.876489] ata6: exception Emask 0x10 SAct 0x0 SErr 0x4050000 action 0xe frozen
>>> [  146.876499] ata6: irq_stat 0x00400040, connection status changed
>>> [  146.876508] ata6: SError: { PHYRdyChg CommWake DevExch }
>>> [  146.876527] ata6: hard resetting link
>>> [  147.619956] ata6: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
>>> [  147.869349] ata6.00: ATA-8: ST3000DM001-9YN166, CC4C, max UDMA/133
>>> [  147.869360] ata6.00: 5860533168 sectors, multi 0: LBA48 NCQ (depth 31/32), AA
>>> [  147.870126] ata6.00: configured for UDMA/133
>>> [  147.870131] ata6: EH complete
>>> [  147.870220] scsi 5:0:0:0: Direct-Access     ATA      ST3000DM001-9YN1 CC4C PQ: 0 ANSI: 5
>>> [  147.870391] sd 5:0:0:0: [sdc] 5860533168 512-byte logical blocks: (3.00 TB/2.72 TiB)
>>> [  147.870393] sd 5:0:0:0: [sdc] 4096-byte physical blocks
>>> [  147.870396] sd 5:0:0:0: Attached scsi generic sg3 type 0
>>> [  147.870434] sd 5:0:0:0: [sdc] Write Protect is off
>>> [  147.870436] sd 5:0:0:0: [sdc] Mode Sense: 00 3a 00 00
>>> [  147.870460] sd 5:0:0:0: [sdc] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
>>> [  147.904848]  sdc: sdc1
>>> [  147.905196] sd 5:0:0:0: [sdc] Attached SCSI disk
>>>
>>>
>>> Here is what happens on re-plug of the device. It is a 3.5" drive and the line
>>> [  617.838013] ata6: hard resetting link
>>> happens too early. I can hear the drive is still spinning up, it can't be ready yet.
>>> I think the delay should be increased.
>>>
>>>
>>> [  617.837966] ata6: exception Emask 0x10 SAct 0x0 SErr 0x4050002 action 0xe frozen
>>> [  617.837976] ata6: irq_stat 0x00400040, connection status changed
>>> [  617.837984] ata6: SError: { RecovComm PHYRdyChg CommWake DevExch }
>>> [  617.838004] ata6: limiting SATA link speed to 1.5 Gbps
>>> [  617.838013] ata6: hard resetting link
>>> [  623.610941] ata6: link is slow to respond, please be patient (ready=0)
>>> [  627.864604] ata6: COMRESET failed (errno=-16)
>>> [  627.864615] ata6: hard resetting link
>>> [  629.931538] ata6: SATA link up 1.5 Gbps (SStatus 113 SControl 310)
>>> [  629.932355] ata6.00: ATA-8: ST3000DM001-9YN166, CC4C, max UDMA/133
>>> [  629.932365] ata6.00: 5860533168 sectors, multi 0: LBA48 NCQ (depth 31/32), AA
>>> [  629.933170] ata6.00: configured for UDMA/133
>>> [  629.951629] ata6: EH complete
>>> [  629.951700] scsi 5:0:0:0: Direct-Access     ATA      ST3000DM001-9YN1 CC4C PQ: 0 ANSI: 5
>>> [  629.951816] sd 5:0:0:0: [sdc] 5860533168 512-byte logical blocks: (3.00 TB/2.72 TiB)
>>> [  629.951819] sd 5:0:0:0: [sdc] 4096-byte physical blocks
>>> [  629.951842] sd 5:0:0:0: Attached scsi generic sg3 type 0
>>> [  629.951875] sd 5:0:0:0: [sdc] Write Protect is off
>>> [  629.951877] sd 5:0:0:0: [sdc] Mode Sense: 00 3a 00 00
>>> [  629.951901] sd 5:0:0:0: [sdc] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
>>> [  629.995970]  sdc: sdc1
>>> [  629.996359] sd 5:0:0:0: [sdc] Attached SCSI disk
>>>
>>>
>>> Martin
>>
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux