From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:33314 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752497AbcJ1LkC (ORCPT ); Fri, 28 Oct 2016 07:40:02 -0400 Date: Fri, 28 Oct 2016 07:39:57 -0400 (EDT) From: Mikulas Patocka To: Bart Van Assche cc: Jens Axboe , linux-block@vger.kernel.org, Mike Snitzer , Christoph Hellwig , dm-devel@redhat.com, "Alasdair G. Kergon" Subject: Re: [dm-devel] [PATCH 1/4] brd: handle misaligned discard In-Reply-To: <9ee4e796-2a85-3b81-ebaa-90ed50f8bf66@sandisk.com> Message-ID: References: <20161021200022.GA12580@redhat.com> <20161024155756.GA48306@redhat.com> <20161025130712.GA12717@infradead.org> <20161025143719.GA51266@redhat.com> <710b07a3-9091-6935-37c4-ea1dcedcab4f@kernel.dk> <338b76f0-40d1-904f-66e0-f6455c5287e7@sandisk.com> <9ee4e796-2a85-3b81-ebaa-90ed50f8bf66@sandisk.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-block-owner@vger.kernel.org List-Id: linux-block@vger.kernel.org On Wed, 26 Oct 2016, Bart Van Assche wrote: > On 10/26/2016 02:46 PM, Mikulas Patocka wrote: > > I don't like the idea of complicating the code by turning discards into > > writes. > > That's not what my patch series does. The only writes added by my patch series > are those for the non-aligned head and tail of the range passed to > blkdev_issue_zeroout(). The purpose of discard is to improve SSD performance and reduce wear. Generating more write requests for discard does quite the opposite - it reduces performance (discard + two small writes is slower than just discard) and it also causes more wear. > > The flag "discard_zeroes_data" is actually misdesigned, because the > > storage stack can change dynamically while bios are in progress. You can > > send a discard bio to a device mapper device that has > > "discard_zeroes_data" - and while the bio is in progress, the device > > mapper stack can be reconfigured to redirect the bio to another device > > that doesn't have "discard_zeroes_data" - and the bio won't zero data and > > the caller that issued it has no way to find it out. > > > > I think the proper thing would be to move "discard_zeroes_data" flag into > > the bio itself - there would be REQ_OP_DISCARD and REQ_OP_DISCARD_ZERO - > > and if the device doesn't support REQ_OP_DISCARD_ZERO, it rejects the bio > > and the caller is supposed to do zeroing manually. > > Sorry but I don't like this proposal. I think that a much better solution > would be to pause I/O before making any changes to the discard_zeroes_data > flag. The device mapper pauses all bios when it switches the table - but the bio was submitted with assumption that it goes to a device withe "discard_zeroes_data" set, then the bio is paused, device mapper table is switched, the bio is unpaused, and now it can go to a device without "discard_zeroes_data". > Bart. Mikulas