From: "Michael S. Tsirkin" <mst@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: jejb@linux.vnet.ibm.com, linux-scsi@vger.kernel.org,
martin.petersen@oracle.com, linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, hch@lst.de
Subject: Re: [PATCH 1/2] virtio: Reduce BUG if total_sg > virtqueue size to WARN.
Date: Fri, 11 Aug 2017 20:23:58 +0300 [thread overview]
Message-ID: <20170811201843-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <64d3c483-076b-1362-b284-30dda704f80c@redhat.com>
On Fri, Aug 11, 2017 at 04:09:26PM +0200, Paolo Bonzini wrote:
> On 10/08/2017 23:41, Michael S. Tsirkin wrote:
> >>> Then we probably should fail probe if vq size is too small.
> >> What does this mean?
> >
> > We must prevent driver from submitting s/g lists > vq size to device.
>
> What is the rationale for the limit?
So the host knows what it needs to support.
> It makes no sense if indirect
> descriptors are available, especially because...
>
> > Either tell linux to avoid s/g lists that are too long, or
> > simply fail request if this happens, or refuse to attach driver to device.
> >
> > Later option would look something like this within probe:
> >
> > for (i = VIRTIO_SCSI_VQ_BASE; i < num_vqs; i++)
> > if (vqs[i]->num < MAX_SG_USED_BY_LINUX)
> > goto err;
> >
> >
> > I don't know what's MAX_SG_USED_BY_LINUX though.
> >
>
> ... both virtio-blk and virtio-scsi transmit their own value for the
> maximum sg list size (max_seg in virtio-scsi, seg_max in virtio-blk).
>
> Paolo
No other device has it, and it seemed like a good idea to
limit it generally at the time.
we can fix the spec to relax the requirement for blk and scsi -
want to submit a proposal? Alternatively, add a generic field
for that.
For a quick fix, make sure vq size is >= max sg.
--
MST
WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Richard W.M. Jones" <rjones@redhat.com>,
jejb@linux.vnet.ibm.com, martin.petersen@oracle.com,
jasowang@redhat.com, linux-scsi@vger.kernel.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, hch@lst.de
Subject: Re: [PATCH 1/2] virtio: Reduce BUG if total_sg > virtqueue size to WARN.
Date: Fri, 11 Aug 2017 20:23:58 +0300 [thread overview]
Message-ID: <20170811201843-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <64d3c483-076b-1362-b284-30dda704f80c@redhat.com>
On Fri, Aug 11, 2017 at 04:09:26PM +0200, Paolo Bonzini wrote:
> On 10/08/2017 23:41, Michael S. Tsirkin wrote:
> >>> Then we probably should fail probe if vq size is too small.
> >> What does this mean?
> >
> > We must prevent driver from submitting s/g lists > vq size to device.
>
> What is the rationale for the limit?
So the host knows what it needs to support.
> It makes no sense if indirect
> descriptors are available, especially because...
>
> > Either tell linux to avoid s/g lists that are too long, or
> > simply fail request if this happens, or refuse to attach driver to device.
> >
> > Later option would look something like this within probe:
> >
> > for (i = VIRTIO_SCSI_VQ_BASE; i < num_vqs; i++)
> > if (vqs[i]->num < MAX_SG_USED_BY_LINUX)
> > goto err;
> >
> >
> > I don't know what's MAX_SG_USED_BY_LINUX though.
> >
>
> ... both virtio-blk and virtio-scsi transmit their own value for the
> maximum sg list size (max_seg in virtio-scsi, seg_max in virtio-blk).
>
> Paolo
No other device has it, and it seemed like a good idea to
limit it generally at the time.
we can fix the spec to relax the requirement for blk and scsi -
want to submit a proposal? Alternatively, add a generic field
for that.
For a quick fix, make sure vq size is >= max sg.
--
MST
next prev parent reply other threads:[~2017-08-11 17:23 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-10 16:40 [PATCH 0/2] virtio_scsi: Set can_queue based on size of virtqueue Richard W.M. Jones
2017-08-10 16:40 ` [PATCH 1/2] virtio: Reduce BUG if total_sg > virtqueue size to WARN Richard W.M. Jones
2017-08-10 16:40 ` Richard W.M. Jones
2017-08-10 16:47 ` Paolo Bonzini
2017-08-10 16:47 ` Paolo Bonzini
2017-08-10 21:21 ` Michael S. Tsirkin
2017-08-10 21:30 ` Richard W.M. Jones
2017-08-10 21:30 ` Richard W.M. Jones
2017-08-10 21:31 ` Michael S. Tsirkin
2017-08-10 21:31 ` Michael S. Tsirkin
2017-08-10 21:35 ` Richard W.M. Jones
2017-08-10 21:35 ` Richard W.M. Jones
2017-08-10 21:41 ` Michael S. Tsirkin
2017-08-10 21:41 ` Michael S. Tsirkin
2017-08-10 21:50 ` Richard W.M. Jones
2017-08-10 21:50 ` Richard W.M. Jones
2017-08-11 14:09 ` Paolo Bonzini
2017-08-11 17:23 ` Michael S. Tsirkin [this message]
2017-08-11 17:23 ` Michael S. Tsirkin
2017-08-11 18:36 ` Paolo Bonzini
2017-08-11 18:36 ` Paolo Bonzini
2017-08-11 14:09 ` Paolo Bonzini
2017-08-10 21:21 ` Michael S. Tsirkin
2017-08-10 16:40 ` [PATCH 2/2] virtio: virtio_scsi: Set can_queue to the length of the virtqueue Richard W.M. Jones
2017-08-10 16:40 ` Richard W.M. Jones
2017-08-10 16:46 ` Paolo Bonzini
2017-08-10 16:46 ` Paolo Bonzini
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=20170811201843-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=hch@lst.de \
--cc=jejb@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=pbonzini@redhat.com \
--cc=virtualization@lists.linux-foundation.org \
/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.