[dm-devel] [PATCH 1/4] brd: handle misaligned discard

[Bart Van Assche]

On 10/28/2016 04:39 AM, Mikulas Patocka wrote:
> On Wed, 26 Oct 2016, Bart Van Assche wrote:
>> On 10/26/2016 02:46 PM, Mikulas Patocka wrote:
>>> 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".

Hello Mikulas,

Sorry if I wasn't clear enough. What I meant is to wait until all 
outstanding requests have finished before modifying the 
discard_zeroes_data flag - the kind of operation that is performed by 
e.g. blk_mq_freeze_queue(). Modifying the discard_zeroes_data flag after 
a bio has been submitted and before it has completed could lead to 
several classes of subtle bugs. Functions like __blkdev_issue_discard() 
assume that the value of the discard_zeroes_data flag does not change 
after this function has been called and before the submitted requests 
completed.

Bart.