All of lore.kernel.org
 help / color / mirror / Atom feed
From: Fam Zheng <famz@redhat.com>
To: Eric Farman <farman@linux.vnet.ibm.com>
Cc: linux-scsi@vger.kernel.org, jejb@linux.vnet.ibm.com,
	martin.petersen@oracle.com
Subject: Re: [PATCH] virtio_scsi: Reject commands when virtqueue is broken
Date: Thu, 12 Jan 2017 21:45:55 +0800	[thread overview]
Message-ID: <20170112134555.GF26504@lemon> (raw)
In-Reply-To: <aa461af2-6fc2-9ece-3b8f-9b6e07268c2f@linux.vnet.ibm.com>

On Thu, 01/12 08:28, Eric Farman wrote:
> > > -	if (virtscsi_kick_cmd(req_vq, cmd, req_size, sizeof(cmd->resp.cmd)) != 0)
> > > +	ret = virtscsi_kick_cmd(req_vq, cmd, req_size, sizeof(cmd->resp.cmd));
> > > +	if (ret == -EIO) {
> > > +		cmd->resp.cmd.response = VIRTIO_SCSI_S_BAD_TARGET;
> > > +		virtscsi_complete_cmd(vscsi, cmd);
> > 
> > Is this safe? Calling virtscsi_complete_cmd requires vq_lock but we don't seem
> > to have it here.
> 
> Hrm...  Didn't notice that, and can't speak to its safety.  I had a bit of
> an I/O workload going to other disks, and things seemed okay, but it was by
> no means an exhaustive test.
> 
> I can't use virtscsi_vq_done, which normally handles that acquire/release.
> It calls virtqueue_get_buf prior to calling virtscsi_complete_cmd, which
> returns NULL because the virtqueue is broken.    Thus, no call to
> virtscsi_complete_cmd.
> 
> Can I mock up a wrapping routine that only handles the lock and complete_cmd
> call, and ignore the virtqueue components that virtscsi_vq_done does?

That sounds good to me, taking the vq_lock here around the call to
virtscsi_complete_cmd, just like virtscsi_kick_cmd().

Fam

> 
> Eric
> 
> > 
> > Fam
> > 
> > > +	} else if (ret != 0) {
> > >  		return SCSI_MLQUEUE_HOST_BUSY;
> > > +	}
> > >  	return 0;
> > >  }
> > 
> 

  reply	other threads:[~2017-01-12 13:45 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-11 22:02 [PATCH] virtio_scsi: Reject commands when virtqueue is broken Eric Farman
2017-01-11 22:02 ` Eric Farman
2017-01-12  3:11   ` Fam Zheng
2017-01-12 13:28     ` Eric Farman
2017-01-12 13:45       ` Fam Zheng [this message]
2017-01-12 14:02         ` Eric Farman

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=20170112134555.GF26504@lemon \
    --to=famz@redhat.com \
    --cc=farman@linux.vnet.ibm.com \
    --cc=jejb@linux.vnet.ibm.com \
    --cc=linux-scsi@vger.kernel.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.