Re: [PATCH v2] virtio-blk: Remove BUG_ON() in virtio_queue_rq()

[Yongji Xie <xieyongji@bytedance.com>]

On Wed, Mar 2, 2022 at 9:33 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Mar 02, 2022 at 03:24:51PM +0200, Max Gurtovoy wrote:
> >
> > On 3/2/2022 3:17 PM, Michael S. Tsirkin wrote:
> > > On Wed, Mar 02, 2022 at 11:51:27AM +0200, Max Gurtovoy wrote:
> > > > On 3/1/2022 5:43 PM, Michael S. Tsirkin wrote:
> > > > > On Mon, Feb 28, 2022 at 02:57:20PM +0800, Xie Yongji wrote:
> > > > > > Currently we have a BUG_ON() to make sure the number of sg
> > > > > > list does not exceed queue_max_segments() in virtio_queue_rq().
> > > > > > However, the block layer uses queue_max_discard_segments()
> > > > > > instead of queue_max_segments() to limit the sg list for
> > > > > > discard requests. So the BUG_ON() might be triggered if
> > > > > > virtio-blk device reports a larger value for max discard
> > > > > > segment than queue_max_segments().
> > > > > Hmm the spec does not say what should happen if max_discard_seg
> > > > > exceeds seg_max. Is this the config you have in mind? how do you
> > > > > create it?
> > > > I don't think it's hard to create it. Just change some registers in the
> > > > device.
> > > >
> > > > But with the dynamic sgl allocation that I added recently, there is no
> > > > problem with this scenario.
> > > Well the problem is device says it can't handle such large descriptors,
> > > I guess it works anyway, but it seems scary.
> >
> > I don't follow.
> >
> > The only problem this patch solves is when a virtio blk device reports
> > larger value for max_discard_segments than max_segments.
> >
>
> No, the peroblem reported is when virtio blk device reports
> max_segments < 256 but not max_discard_segments.
> I would expect discard to follow max_segments restrictions then.
>

I think one point is whether we want to allow the corner case that the
device reports a larger value for max_discard_segments than
max_segments. For example, queue size is 256, max_segments is 128 - 2,
max_discard_segments is 256 - 2.

Thanks,
Yongji