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 -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
