Michael S. Tsirkin
2022-Mar-01 15:43 UTC
[PATCH v2] virtio-blk: Remove BUG_ON() in virtio_queue_rq()
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?> To fix it, let's simply > remove the BUG_ON() which has become unnecessary after commit > 02746e26c39e("virtio-blk: avoid preallocating big SGL for data"). > And the unused vblk->sg_elems can also be removed together. > > Fixes: 1f23816b8eb8 ("virtio_blk: add discard and write zeroes support") > Suggested-by: Christoph Hellwig <hch at infradead.org> > Signed-off-by: Xie Yongji <xieyongji at bytedance.com> > --- > drivers/block/virtio_blk.c | 10 +--------- > 1 file changed, 1 insertion(+), 9 deletions(-) > > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c > index c443cd64fc9b..a43eb1813cec 100644 > --- a/drivers/block/virtio_blk.c > +++ b/drivers/block/virtio_blk.c > @@ -76,9 +76,6 @@ struct virtio_blk { > */ > refcount_t refs; > > - /* What host tells us, plus 2 for header & tailer. */ > - unsigned int sg_elems; > - > /* Ida index - used to track minor number allocations. */ > int index; > > @@ -322,8 +319,6 @@ static blk_status_t virtio_queue_rq(struct blk_mq_hw_ctx *hctx, > blk_status_t status; > int err; > > - BUG_ON(req->nr_phys_segments + 2 > vblk->sg_elems); > - > status = virtblk_setup_cmd(vblk->vdev, req, vbr); > if (unlikely(status)) > return status; > @@ -783,8 +778,6 @@ static int virtblk_probe(struct virtio_device *vdev) > /* Prevent integer overflows and honor max vq size */ > sg_elems = min_t(u32, sg_elems, VIRTIO_BLK_MAX_SG_ELEMS - 2); > > - /* We need extra sg elements at head and tail. */ > - sg_elems += 2; > vdev->priv = vblk = kmalloc(sizeof(*vblk), GFP_KERNEL); > if (!vblk) { > err = -ENOMEM; > @@ -796,7 +789,6 @@ static int virtblk_probe(struct virtio_device *vdev) > mutex_init(&vblk->vdev_mutex); > > vblk->vdev = vdev; > - vblk->sg_elems = sg_elems; > > INIT_WORK(&vblk->config_work, virtblk_config_changed_work); > > @@ -853,7 +845,7 @@ static int virtblk_probe(struct virtio_device *vdev) > set_disk_ro(vblk->disk, 1); > > /* We can handle whatever the host told us to handle. */ > - blk_queue_max_segments(q, vblk->sg_elems-2); > + blk_queue_max_segments(q, sg_elems); > > /* No real sector limit. */ > blk_queue_max_hw_sectors(q, -1U); > -- > 2.20.1