Linux-SCSI Archive on lore.kernel.org
 help / color / Atom feed
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: Wed, 29 Jul 2020 14:25:15 -0400
Message-ID: <20200729182515.GB1580638@rowland.harvard.edu> (raw)
In-Reply-To: <A1653792-B7E5-46A9-835B-7FA85FCD0378@puri.sm>

On Wed, Jul 29, 2020 at 06:43:48PM +0200, Martin Kepplinger wrote:
> 
> 
> Am 29. Juli 2020 17:44:42 MESZ schrieb James Bottomley <James.Bottomley@HansenPartnership.com>:
> >On Wed, 2020-07-29 at 17:40 +0200, Martin Kepplinger wrote:
> >> On 29.07.20 16:53, James Bottomley wrote:
> >> > On Wed, 2020-07-29 at 07:46 -0700, James Bottomley wrote:
> >> > > On Wed, 2020-07-29 at 10:32 -0400, Alan Stern wrote:
> >[...]
> >> > > > This error report comes from the SCSI layer, not the block
> >> > > > layer.
> >> > > 
> >> > > That sense code means "NOT READY TO READY CHANGE, MEDIUM MAY HAVE
> >> > > CHANGED" so it sounds like it something we should be
> >> > > ignoring.  Usually this signals a problem, like you changed the
> >> > > medium manually (ejected the CD).  But in this case you can tell
> >> > > us to expect this by setting
> >> > > 
> >> > > sdev->expecting_cc_ua
> >> > > 
> >> > > And we'll retry.  I think you need to set this on all resumed
> >> > > devices.
> >> > 
> >> > Actually, it's not quite that easy, we filter out this ASC/ASCQ
> >> > combination from the check because we should never ignore medium
> >> > might have changed events on running devices.  We could ignore it
> >> > if we had a flag to say the power has been yanked (perhaps an
> >> > additional sdev flag you set on resume) but we would still miss the
> >> > case where you really had powered off the drive and then changed
> >> > the media ... if you can regard this as the user's problem, then we
> >> > might have a solution.
> >> > 
> >> > James
> >> >  
> >> 
> >> oh I see what you mean now, thanks for the ellaboration.
> >> 
> >> if I do the following change, things all look normal and runtime pm
> >> works. I'm not 100% sure if just setting expecting_cc_ua in resume()
> >> is "correct" but that looks like it is what you're talking about:
> >> 
> >> (note that this is of course with the one block layer diff applied
> >> that Alan posted a few emails back)
> >> 
> >> 
> >> --- a/drivers/scsi/scsi_error.c
> >> +++ b/drivers/scsi/scsi_error.c
> >> @@ -554,16 +554,8 @@ int scsi_check_sense(struct scsi_cmnd *scmd)
> >>                  * so that we can deal with it there.
> >>                  */
> >>                 if (scmd->device->expecting_cc_ua) {
> >> -                       /*
> >> -                        * Because some device does not queue unit
> >> -                        * attentions correctly, we carefully check
> >> -                        * additional sense code and qualifier so as
> >> -                        * not to squash media change unit attention.
> >> -                        */
> >> -                       if (sshdr.asc != 0x28 || sshdr.ascq != 0x00)
> >> {
> >> -                               scmd->device->expecting_cc_ua = 0;
> >> -                               return NEEDS_RETRY;
> >> -                       }
> >> +                       scmd->device->expecting_cc_ua = 0;
> >> +                       return NEEDS_RETRY;
> >
> >Well, yes, but you can't do this because it would lose us media change
> >events in the non-suspend/resume case which we really don't want. 
> >That's why I was suggesting a new flag.
> >
> >James
> 
> also if I set expecting_cc_ua in resume() only, like I did?

That wouldn't make any difference.  The information sent by your card 
reader has sshdr.asc == 0x28 and sshdr.ascq == 0x00 (you can see it in 
the log).  So because of the code here in scsi_check_sense(), which you 
can't change, the Unit Attention sent by the card reader would not be 
retried even if you do set the flag in resume().

Alan Stern

  reply index

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-23 11:10 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 [this message]
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
2020-08-11  7:55                                                                       ` Martin Kepplinger
2020-08-11 13:48                                                                         ` Alan Stern
2020-08-23 14:57                                                                         ` [PATCH] block: Fix bug in runtime-resume handling Alan Stern
2020-08-24 17:48                                                                           ` Bart Van Assche
2020-08-24 20:13                                                                             ` Alan Stern
2020-08-26  7:48                                                                               ` Martin Kepplinger
2020-08-27 17:42                                                                                 ` Martin Kepplinger
2020-08-27 20:29                                                                                   ` Alan Stern
2020-08-29  7:24                                                                                     ` Martin Kepplinger
2020-08-29 15:26                                                                                       ` Alan Stern
2020-08-29 16:33                                                                                         ` Martin Kepplinger
2020-08-29 18:56                                                                                           ` Alan Stern
2020-08-30  0:38                                                                                             ` Bart Van Assche
2020-08-30  1:06                                                                                               ` Alan Stern
2020-07-29 15:40                                       ` [PATCH] scsi: sd: add runtime pm to open / release 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=20200729182515.GB1580638@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

Linux-SCSI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-scsi/0 linux-scsi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-scsi linux-scsi/ https://lore.kernel.org/linux-scsi \
		linux-scsi@vger.kernel.org
	public-inbox-index linux-scsi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-scsi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git