From: Alan Stern <stern@rowland.harvard.edu>
To: Martin Kepplinger <martin.kepplinger@puri.sm>
Cc: James Bottomley <James.Bottomley@hansenpartnership.com>,
Bart Van Assche <bvanassche@acm.org>,
Can Guo <cang@codeaurora.org>,
martin.petersen@oracle.com, linux-scsi@vger.kernel.org,
linux-kernel@vger.kernel.org, kernel@puri.sm
Subject: Re: [PATCH] scsi: sd: add runtime pm to open / release
Date: Mon, 10 Aug 2020 10:13:43 -0400 [thread overview]
Message-ID: <20200810141343.GA299045@rowland.harvard.edu> (raw)
In-Reply-To: <60150284-be13-d373-5448-651b72a7c4c9@puri.sm>
On Mon, Aug 10, 2020 at 02:03:17PM +0200, Martin Kepplinger wrote:
> On 09.08.20 17:26, Alan Stern wrote:
> > This is a somewhat fragile approach. You don't know for certain that
> > scsi_noretry_cmd will be called. Also, scsi_noretry_cmd can be called
> > from other places.
> >
> > It would be better to clear the expecting_media_change flag just before
> > returning from scsi_decide_disposition. That way its use is localized
> > to one routine, not spread out between two.
> >
> > Alan Stern
> >
>
> Hi Alan,
>
> maybe you're right. I initially just thought that I'd allow for specific
> error codes in scsi_noretry_cmd() to return non-NULL (BUS_BUSY, PARITY,
> ERROR) despite having the flag set.
>
> The below version works equally fine for me but I'm not sure if it's
> actually more safe.
>
> James, when exposing a new writable sysfs option like
> "suspend_no_media_change"(?) that drivers can check before setting the
> new "expecting_media_change" flag (during resume), would this addition
> make sense to you?
>
> thanks,
>
> martin
>
>
>
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -565,6 +565,18 @@ int scsi_check_sense(struct scsi_cmnd *scmd)
> return NEEDS_RETRY;
> }
> }
> + if (scmd->device->expecting_media_change) {
> + if (sshdr.asc == 0x28 && sshdr.ascq == 0x00) {
> + /*
> + * clear the expecting_media_change in
> + * scsi_decide_disposition() because we
> + * need to catch possible "fail fast" overrides
> + * that block readahead can cause.
> + */
> + return NEEDS_RETRY;
> + }
> + }
> +
> /*
> * we might also expect a cc/ua if another LUN on the target
> * reported a UA with an ASC/ASCQ of 3F 0E -
> @@ -1944,9 +1956,19 @@ int scsi_decide_disposition(struct scsi_cmnd *scmd)
> * the request was not marked fast fail. Note that above,
> * even if the request is marked fast fail, we still requeue
> * for queue congestion conditions (QUEUE_FULL or BUSY) */
> - if ((++scmd->retries) <= scmd->allowed
> - && !scsi_noretry_cmd(scmd)) {
> - return NEEDS_RETRY;
> + if ((++scmd->retries) <= scmd->allowed) {
> + /*
> + * but scsi_noretry_cmd() cannot override the
> + * expecting_media_change flag.
> + */
> + if (!scsi_noretry_cmd(scmd) ||
> + scmd->device->expecting_media_change) {
> + scmd->device->expecting_media_change = 0;
> + return NEEDS_RETRY;
> + } else {
> + /* marked fast fail and not expected. */
> + return SUCCESS;
> + }
> } else {
This may not matter... but it's worth pointing out that
expecting_media_change doesn't get cleared if ++scmd->retries >
scmd->allowed.
It also doesn't get cleared in cases where the device _doesn't_
report a Unit Attention.
Alan Stern
next prev parent reply other threads:[~2020-08-10 14:13 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-23 11:10 [PATCH] scsi: sd: add runtime pm to open / release Martin Kepplinger
2020-06-24 13:33 ` Bart Van Assche
2020-06-25 8:16 ` Martin Kepplinger
2020-06-25 14:52 ` Alan Stern
2020-06-26 3:53 ` Bart Van Assche
2020-06-26 15:07 ` Bart Van Assche
2020-06-26 15:44 ` Alan Stern
2020-06-28 2:37 ` Bart Van Assche
2020-06-28 13:10 ` Alan Stern
2020-06-29 9:42 ` Martin Kepplinger
2020-06-29 16:15 ` Alan Stern
2020-06-29 16:56 ` Bart Van Assche
2020-06-29 17:40 ` Alan Stern
2020-06-30 3:33 ` Martin Kepplinger
2020-06-30 13:38 ` Alan Stern
2020-06-30 15:59 ` Bart Van Assche
2020-06-30 18:02 ` Alan Stern
2020-06-30 19:23 ` Bart Van Assche
2020-06-30 19:38 ` Alan Stern
2020-06-30 23:31 ` Bart Van Assche
2020-07-01 0:49 ` Alan Stern
2020-07-06 16:41 ` Alan Stern
2020-07-28 7:02 ` Martin Kepplinger
2020-07-28 20:02 ` Alan Stern
2020-07-29 14:12 ` Martin Kepplinger
2020-07-29 14:32 ` Alan Stern
2020-07-29 14:44 ` Martin K. Petersen
2020-07-29 14:56 ` Alan Stern
2020-07-29 14:46 ` James Bottomley
2020-07-29 14:53 ` James Bottomley
2020-07-29 15:40 ` Martin Kepplinger
2020-07-29 15:44 ` James Bottomley
2020-07-29 16:43 ` Martin Kepplinger
2020-07-29 18:25 ` Alan Stern
2020-07-29 18:29 ` James Bottomley
2020-07-30 8:52 ` Martin Kepplinger
2020-07-30 8:54 ` Martin Kepplinger
2020-07-30 15:10 ` Alan Stern
2020-08-04 9:39 ` Martin Kepplinger
2020-08-07 9:51 ` Martin Kepplinger
2020-08-07 14:30 ` Alan Stern
2020-08-08 6:59 ` Martin Kepplinger
2020-08-08 15:05 ` Alan Stern
2020-08-09 9:20 ` Martin Kepplinger
2020-08-09 15:26 ` Alan Stern
2020-08-10 12:03 ` Martin Kepplinger
2020-08-10 14:13 ` Alan Stern [this message]
2020-08-11 7:55 ` Martin Kepplinger
2020-08-11 13:48 ` Alan Stern
2020-07-29 15:40 ` Alan Stern
2020-07-29 15:49 ` James Bottomley
2020-07-29 16:17 ` Alan Stern
2020-07-29 15:52 ` Martin Kepplinger
2020-07-29 18:10 ` Douglas Gilbert
2020-07-30 8:05 ` Martin Kepplinger
2020-07-30 15:14 ` Alan Stern
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200810141343.GA299045@rowland.harvard.edu \
--to=stern@rowland.harvard.edu \
--cc=James.Bottomley@hansenpartnership.com \
--cc=bvanassche@acm.org \
--cc=cang@codeaurora.org \
--cc=kernel@puri.sm \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.kepplinger@puri.sm \
--cc=martin.petersen@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).