From patchwork Thu Oct 26 04:38:38 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jens Axboe X-Patchwork-Id: 10027421 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 70088601E8 for ; Thu, 26 Oct 2017 04:38:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 56BCC27CF9 for ; Thu, 26 Oct 2017 04:38:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4B66628D11; Thu, 26 Oct 2017 04:38:44 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.4 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6FA1C27CF9 for ; Thu, 26 Oct 2017 04:38:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751027AbdJZEim (ORCPT ); Thu, 26 Oct 2017 00:38:42 -0400 Received: from mail-pf0-f196.google.com ([209.85.192.196]:45000 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750919AbdJZEik (ORCPT ); Thu, 26 Oct 2017 00:38:40 -0400 Received: by mail-pf0-f196.google.com with SMTP id x7so1628821pfa.1 for ; Wed, 25 Oct 2017 21:38:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel-dk.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=kJsYeSWhDHPzQoUy2GI97xbMGJcst+BVSRaMlnGJL7k=; b=j7/QmI5QU5pFtJTMkANuhaELA/OP//QSv8+Wg1f8INSsu1aL4vukD1vgmD3XaB9hEc sWzNThqCrE4oZ2qVb/FeRJtrnK82g71eRFndzEa87v0VgbM8cr67lkdqkIah464N1xPW u7MqUPvFGE+BsDJRppeD0YBOPfOrEEWaqngbkiTKvCrbFii3G4MT8QPc4efjNFN8nYOA 0kERf9WgMeCVN9RV5JeR4Sg4OgU2WUu66F4TlmoaWcXEET/BmwEikNoUv99qccwJkywU 333DzyF2Mdl3kGyKv6S6xHo/FISDqkDnb1euJ1kaBGi2Dr3D06ad9j91dIJXW+N3XYLg /fbg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=kJsYeSWhDHPzQoUy2GI97xbMGJcst+BVSRaMlnGJL7k=; b=ljHtecXHhscVRuCY8DzsGhqGRPyS5DCYQEqvDZ6VEydJpoOfbH4ufnef8nxckjQHuU i2C63x6C3qcEwWRciJu8ncCSWmylIi0TkzrSeJvZv2TstH31Y77y+AzWl4I5jGTJbwek AE1iH9TCuNWsrHwt7Yyz5gyI+8QK7PfOFrtKcq+AyF4meOxiDGf3fzBG+1OxXBluaZ47 9aK5vM/U9ls1ZZdUXRsySmdG3fgRpdF/yyEQTzqH7BO7GgCuRg9tIB5/jzX7uCoTb/tD zcJc2xBMiftAH0ivlCt4o8qB3Imq6p5gcNV7sprtaF8D5g2ecHBac5Ae7hmXbfYDD32S 3+NA== X-Gm-Message-State: AMCzsaXTfHnR66UsErRWn1/xMqyIpz0n8NU4HTVxKdBdapf6t533Pdo+ wPN7k/8MQe4jRhwu7YiNHehJ2A== X-Google-Smtp-Source: ABhQp+T1G8GnTVPf3rbOGCRSmzAi1kT4k3ynKdbr5kwn+uEpjl3vTpmd20kxr+8CVjDpEbTiwQS5gQ== X-Received: by 10.101.65.2 with SMTP id w2mr4017676pgp.131.1508992719926; Wed, 25 Oct 2017 21:38:39 -0700 (PDT) Received: from [172.20.0.27] (50-232-67-21-static.hfc.comcastbusiness.net. [50.232.67.21]) by smtp.gmail.com with ESMTPSA id z65sm6659257pgb.50.2017.10.25.21.38.39 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 25 Oct 2017 21:38:39 -0700 (PDT) Subject: Re: [PATCH v2] virtio_blk: Fix an SG_IO regression To: Bart Van Assche , "hch@lst.de" Cc: "linux-block@vger.kernel.org" , "mst@redhat.com" , "stable@vger.kernel.org" , "dann.frazier@canonical.com" References: <20171025095617.7315-1-bart.vanassche@wdc.com> <11a35a46-dc51-c32f-cb64-793d9dd02c53@kernel.dk> <1508959556.5073.1.camel@wdc.com> <83926138-e6aa-6792-5275-31f5c1330c90@kernel.dk> <20171025193752.GA6164@lst.de> <1508992426.8734.3.camel@wdc.com> From: Jens Axboe Message-ID: Date: Wed, 25 Oct 2017 21:38:38 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <1508992426.8734.3.camel@wdc.com> Content-Language: en-US Sender: linux-block-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 10/25/2017 09:33 PM, Bart Van Assche wrote: > On Wed, 2017-10-25 at 21:37 +0200, hch@lst.de wrote: >> Honestly I think the right fix is to just kill the SCSI passthrough >> in virtio. It has been replaced by virtio-scsi a long time ago, and >> has been disabled by default in qemu for a long time (and I don't >> think any other hypervisor ever supported it). >> >> It should never have been added (saying that as the person who added >> it). > > Hello Christoph, > > Sorry but I'm not familiar enough with the virtio_blk driver to do that > removal myself. Since the patch at the start of this e-mail thread has > a "Cc: stable" tag, can you submit such a patch after this patch went > in? Something like the below. But yes, we should probably get the simple fix in, then kill it after, since the fix is targeting stable and the current series. diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig index 2dfe99b328f8..9fac2b724577 100644 --- a/drivers/block/Kconfig +++ b/drivers/block/Kconfig @@ -446,16 +446,6 @@ config VIRTIO_BLK This is the virtual block driver for virtio. It can be used with QEMU based VMMs (like KVM or Xen). Say Y or M. -config VIRTIO_BLK_SCSI - bool "SCSI passthrough request for the Virtio block driver" - depends on VIRTIO_BLK - select BLK_SCSI_REQUEST - ---help--- - Enable support for SCSI passthrough (e.g. the SG_IO ioctl) on - virtio-blk devices. This is only supported for the legacy - virtio protocol and not enabled by default by any hypervisor. - You probably want to use virtio-scsi instead. - config BLK_DEV_RBD tristate "Rados block device (RBD)" depends on INET && BLOCK diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 34e17ee799be..1764df4f1c60 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include @@ -54,11 +53,6 @@ struct virtio_blk { }; struct virtblk_req { -#ifdef CONFIG_VIRTIO_BLK_SCSI - struct scsi_request sreq; /* for SCSI passthrough, must be first */ - u8 sense[SCSI_SENSE_BUFFERSIZE]; - struct virtio_scsi_inhdr in_hdr; -#endif struct virtio_blk_outhdr out_hdr; u8 status; struct scatterlist sg[]; @@ -76,80 +70,6 @@ static inline blk_status_t virtblk_result(struct virtblk_req *vbr) } } -/* - * If this is a packet command we need a couple of additional headers. Behind - * the normal outhdr we put a segment with the scsi command block, and before - * the normal inhdr we put the sense data and the inhdr with additional status - * information. - */ -#ifdef CONFIG_VIRTIO_BLK_SCSI -static int virtblk_add_req_scsi(struct virtqueue *vq, struct virtblk_req *vbr, - struct scatterlist *data_sg, bool have_data) -{ - struct scatterlist hdr, status, cmd, sense, inhdr, *sgs[6]; - unsigned int num_out = 0, num_in = 0; - - sg_init_one(&hdr, &vbr->out_hdr, sizeof(vbr->out_hdr)); - sgs[num_out++] = &hdr; - sg_init_one(&cmd, vbr->sreq.cmd, vbr->sreq.cmd_len); - sgs[num_out++] = &cmd; - - if (have_data) { - if (vbr->out_hdr.type & cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_OUT)) - sgs[num_out++] = data_sg; - else - sgs[num_out + num_in++] = data_sg; - } - - sg_init_one(&sense, vbr->sense, SCSI_SENSE_BUFFERSIZE); - sgs[num_out + num_in++] = &sense; - sg_init_one(&inhdr, &vbr->in_hdr, sizeof(vbr->in_hdr)); - sgs[num_out + num_in++] = &inhdr; - sg_init_one(&status, &vbr->status, sizeof(vbr->status)); - sgs[num_out + num_in++] = &status; - - return virtqueue_add_sgs(vq, sgs, num_out, num_in, vbr, GFP_ATOMIC); -} - -static inline void virtblk_scsi_request_done(struct request *req) -{ - struct virtblk_req *vbr = blk_mq_rq_to_pdu(req); - struct virtio_blk *vblk = req->q->queuedata; - struct scsi_request *sreq = &vbr->sreq; - - sreq->resid_len = virtio32_to_cpu(vblk->vdev, vbr->in_hdr.residual); - sreq->sense_len = virtio32_to_cpu(vblk->vdev, vbr->in_hdr.sense_len); - sreq->result = virtio32_to_cpu(vblk->vdev, vbr->in_hdr.errors); -} - -static int virtblk_ioctl(struct block_device *bdev, fmode_t mode, - unsigned int cmd, unsigned long data) -{ - struct gendisk *disk = bdev->bd_disk; - struct virtio_blk *vblk = disk->private_data; - - /* - * Only allow the generic SCSI ioctls if the host can support it. - */ - if (!virtio_has_feature(vblk->vdev, VIRTIO_BLK_F_SCSI)) - return -ENOTTY; - - return scsi_cmd_blk_ioctl(bdev, mode, cmd, - (void __user *)data); -} -#else -static inline int virtblk_add_req_scsi(struct virtqueue *vq, - struct virtblk_req *vbr, struct scatterlist *data_sg, - bool have_data) -{ - return -EIO; -} -static inline void virtblk_scsi_request_done(struct request *req) -{ -} -#define virtblk_ioctl NULL -#endif /* CONFIG_VIRTIO_BLK_SCSI */ - static int virtblk_add_req(struct virtqueue *vq, struct virtblk_req *vbr, struct scatterlist *data_sg, bool have_data) { @@ -176,13 +96,6 @@ static inline void virtblk_request_done(struct request *req) { struct virtblk_req *vbr = blk_mq_rq_to_pdu(req); - switch (req_op(req)) { - case REQ_OP_SCSI_IN: - case REQ_OP_SCSI_OUT: - virtblk_scsi_request_done(req); - break; - } - blk_mq_end_request(req, virtblk_result(vbr)); } @@ -237,10 +150,6 @@ static blk_status_t virtio_queue_rq(struct blk_mq_hw_ctx *hctx, case REQ_OP_FLUSH: type = VIRTIO_BLK_T_FLUSH; break; - case REQ_OP_SCSI_IN: - case REQ_OP_SCSI_OUT: - type = VIRTIO_BLK_T_SCSI_CMD; - break; case REQ_OP_DRV_IN: type = VIRTIO_BLK_T_GET_ID; break; @@ -265,10 +174,7 @@ static blk_status_t virtio_queue_rq(struct blk_mq_hw_ctx *hctx, } spin_lock_irqsave(&vblk->vqs[qid].lock, flags); - if (blk_rq_is_scsi(req)) - err = virtblk_add_req_scsi(vblk->vqs[qid].vq, vbr, vbr->sg, num); - else - err = virtblk_add_req(vblk->vqs[qid].vq, vbr, vbr->sg, num); + err = virtblk_add_req(vblk->vqs[qid].vq, vbr, vbr->sg, num); if (err) { virtqueue_kick(vblk->vqs[qid].vq); blk_mq_stop_hw_queue(hctx); @@ -336,7 +242,6 @@ static int virtblk_getgeo(struct block_device *bd, struct hd_geometry *geo) } static const struct block_device_operations virtblk_fops = { - .ioctl = virtblk_ioctl, .owner = THIS_MODULE, .getgeo = virtblk_getgeo, }; @@ -579,9 +484,6 @@ static int virtblk_init_request(struct blk_mq_tag_set *set, struct request *rq, struct virtio_blk *vblk = set->driver_data; struct virtblk_req *vbr = blk_mq_rq_to_pdu(rq); -#ifdef CONFIG_VIRTIO_BLK_SCSI - vbr->sreq.sense = vbr->sense; -#endif sg_init_table(vbr->sg, vblk->sg_elems); return 0; } @@ -871,9 +773,6 @@ static const struct virtio_device_id id_table[] = { static unsigned int features_legacy[] = { VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY, VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, -#ifdef CONFIG_VIRTIO_BLK_SCSI - VIRTIO_BLK_F_SCSI, -#endif VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE, VIRTIO_BLK_F_MQ, }