From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753339AbdC0O5r (ORCPT ); Mon, 27 Mar 2017 10:57:47 -0400 Received: from verein.lst.de ([213.95.11.211]:41673 "EHLO newverein.lst.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751063AbdC0O51 (ORCPT ); Mon, 27 Mar 2017 10:57:27 -0400 Date: Mon, 27 Mar 2017 16:56:26 +0200 From: Christoph Hellwig To: Changpeng Liu Cc: virtio-dev@lists.oasis-open.org, virtualization@lists.linux-foundation.org, linux-kernel@vger.kernel.org, hch@lst.de, qemu-devel@nongnu.org Subject: Re: [PATCH] virtio-blk: add DISCARD support to virtio-blk driver Message-ID: <20170327145625.GA24952@lst.de> References: <1490690365-21109-1-git-send-email-changpeng.liu@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1490690365-21109-1-git-send-email-changpeng.liu@intel.com> User-Agent: Mutt/1.5.17 (2007-11-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 28, 2017 at 04:39:25PM +0800, Changpeng Liu wrote: > Currently virtio-blk driver does not provide discard feature flag, so the > filesystems which built on top of the block device will not send discard > command. This is okay for HDD backend, but it will impact the performance > for SSD backend. > > Add a feature flag VIRTIO_BLK_F_DISCARD and command VIRTIO_BLK_T_DISCARD > to extend exist virtio-blk protocol. virtio-blk protocol uses a single > 8 bytes descriptor containing type,reserved and sector, currently Linux > uses the reserved field as IO priority, here we also re-use the reserved > field as number of discard sectors. Do you have a link to the specification for this feature? At least virtio-v1.0 does not seem to specify a discard feature. Note that Linux 4.11 and later have support for multi-range discard ala ATA TRIM, SCSI UNMAP and NVMe deallocate which might be useful here, too. > + q->limits.discard_zeroes_data = 0; No need to clear this. Also hopefully this field goes away for 4.12 > + blk_queue_max_discard_segments(q, 1); No need to set this.