| Message ID | 20230911040217.253905-18-dlemoal@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix libata suspend/resume handling and code cleanup | expand |
On 9/11/23 06:02, Damien Le Moal wrote: > Some drives are really slow to spinup on resume, resulting is a very > slow response to COMRESET and to error messages such as: > > ata1: COMRESET failed (errno=-16) > ata1: link is slow to respond, please be patient (ready=0) > ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300) > ata1.00: configured for UDMA/133 > > Given that the slowness of the response is indicated with the message > "link is slow to respond..." and that resets are retried until the > device is detected as online after up to 1min (ata_eh_reset_timeouts), > there is no point in printing the "COMRESET failed" error message. Let's > not scare the user with non fatal errors and only warn about reset > failures in ata_eh_reset() when all reset retries have been exhausted. > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > --- > drivers/ata/libata-eh.c | 2 ++ > drivers/ata/libata-sata.c | 1 - > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c > index 43b0a1509548..bbc522d16f44 100644 > --- a/drivers/ata/libata-eh.c > +++ b/drivers/ata/libata-eh.c > @@ -2919,6 +2919,8 @@ int ata_eh_reset(struct ata_link *link, int classify, > */ > if (ata_is_host_link(link)) > ata_eh_thaw_port(ap); > + ata_link_warn(link, "%sreset failed\n", > + reset == hardreset ? "hard" : "soft"); > goto out; > } > > diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c > index 0748e9ea4f5f..00674aae1696 100644 > --- a/drivers/ata/libata-sata.c > +++ b/drivers/ata/libata-sata.c > @@ -608,7 +608,6 @@ int sata_link_hardreset(struct ata_link *link, const unsigned int *timing, > /* online is set iff link is online && reset succeeded */ > if (online) > *online = false; > - ata_link_err(link, "COMRESET failed (errno=%d)\n", rc); > } > return rc; > } Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
On 11/09/2023 05:02, Damien Le Moal wrote: > Some drives are really slow to spinup on resume, resulting is a very > slow response to COMRESET and to error messages such as: > > ata1: COMRESET failed (errno=-16) > ata1: link is slow to respond, please be patient (ready=0) > ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300) > ata1.00: configured for UDMA/133 > > Given that the slowness of the response is indicated with the message > "link is slow to respond..." and that resets are retried until the > device is detected as online after up to 1min (ata_eh_reset_timeouts), > there is no point in printing the "COMRESET failed" error message. Let's > not scare the user with non fatal errors and only warn about reset > failures in ata_eh_reset() when all reset retries have been exhausted. > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > --- > drivers/ata/libata-eh.c | 2 ++ > drivers/ata/libata-sata.c | 1 - > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c > index 43b0a1509548..bbc522d16f44 100644 > --- a/drivers/ata/libata-eh.c > +++ b/drivers/ata/libata-eh.c > @@ -2919,6 +2919,8 @@ int ata_eh_reset(struct ata_link *link, int classify, > */ > if (ata_is_host_link(link)) > ata_eh_thaw_port(ap); > + ata_link_warn(link, "%sreset failed\n", > + reset == hardreset ? "hard" : "soft"); nit: how about spell out the full reset type string ata_link_warn(link, "%s failed\n", > + reset == hardreset ? "hardreset" : "softreset"); which may make grep'ing slightly easier. > goto out; > } > > diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c > index 0748e9ea4f5f..00674aae1696 100644 > --- a/drivers/ata/libata-sata.c > +++ b/drivers/ata/libata-sata.c > @@ -608,7 +608,6 @@ int sata_link_hardreset(struct ata_link *link, const unsigned int *timing, > /* online is set iff link is online && reset succeeded */ > if (online) > *online = false; > - ata_link_err(link, "COMRESET failed (errno=%d)\n", rc); > } > return rc; > }
On Mon, Sep 11, 2023 at 01:02:15PM +0900, Damien Le Moal wrote: > Some drives are really slow to spinup on resume, resulting is a very > slow response to COMRESET and to error messages such as: > > ata1: COMRESET failed (errno=-16) > ata1: link is slow to respond, please be patient (ready=0) > ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300) > ata1.00: configured for UDMA/133 > > Given that the slowness of the response is indicated with the message > "link is slow to respond..." and that resets are retried until the > device is detected as online after up to 1min (ata_eh_reset_timeouts), > there is no point in printing the "COMRESET failed" error message. Let's > not scare the user with non fatal errors and only warn about reset > failures in ata_eh_reset() when all reset retries have been exhausted. > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Tested-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 43b0a1509548..bbc522d16f44 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -2919,6 +2919,8 @@ int ata_eh_reset(struct ata_link *link, int classify, */ if (ata_is_host_link(link)) ata_eh_thaw_port(ap); + ata_link_warn(link, "%sreset failed\n", + reset == hardreset ? "hard" : "soft"); goto out; } diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c index 0748e9ea4f5f..00674aae1696 100644 --- a/drivers/ata/libata-sata.c +++ b/drivers/ata/libata-sata.c @@ -608,7 +608,6 @@ int sata_link_hardreset(struct ata_link *link, const unsigned int *timing, /* online is set iff link is online && reset succeeded */ if (online) *online = false; - ata_link_err(link, "COMRESET failed (errno=%d)\n", rc); } return rc; }
Some drives are really slow to spinup on resume, resulting is a very slow response to COMRESET and to error messages such as: ata1: COMRESET failed (errno=-16) ata1: link is slow to respond, please be patient (ready=0) ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300) ata1.00: configured for UDMA/133 Given that the slowness of the response is indicated with the message "link is slow to respond..." and that resets are retried until the device is detected as online after up to 1min (ata_eh_reset_timeouts), there is no point in printing the "COMRESET failed" error message. Let's not scare the user with non fatal errors and only warn about reset failures in ata_eh_reset() when all reset retries have been exhausted. Signed-off-by: Damien Le Moal <dlemoal@kernel.org> --- drivers/ata/libata-eh.c | 2 ++ drivers/ata/libata-sata.c | 1 - 2 files changed, 2 insertions(+), 1 deletion(-)