Yeah, that's a great point. Now that I've reviewed the code a little more, I understand how it's not safe to do the thing I described in the first place, and how this change is safe and correct. Please feel free to add my Reviewed-by: John Dorminy <jdorminy@xxxxxxxxxx> Thanks! On Fri, Feb 7, 2020 at 3:07 PM Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote: > > > > On Fri, 7 Feb 2020, John Dorminy wrote: > > > There's no obvious way for the other process to wait for the discard > > to be complete at the present time, though -- suppose there were two > > holders of a given buffer, and one decided to discard -- how will the > > second one to wait for the discard to complete, or even tell that it's > > currently being discarded from another thread? I would naively guess > > that __write_dirty_buffer() waits via wait_on_bit_io(&b->state, > > B_WRITING, ...) to make sure nobody else is doing IO on the buffer > > location at the moment, but a discard doesn't currently set that bit, > > as far as I can see. > > > > (If there is a way to wait, perhaps it should be documented at > > dm_bufio_discard_buffers() -- "If there is another process holding the > > buffer, the other process should be sure to [do stuff] before issuing > > a write, lest the write potentially be dropped or corrupted." > > If two processes write to the same buffer, it is undefined behavior. If > both of them do this: > 1) dm_bufio_get > 2) ... write to the buffer > 3) dm_bufio_mark_buffer_dirty > 4) dm_bufio_release > it is undefined what data the buffer would hold. It can even hold mixture > of data written by those two processes. You must design your code in such > a way that this doesn't happen. > > The same is with discards - if you want to use them, you must design your > code so that it doesn't overlay discards with other I/O. If you can't > design it this way, then don't use discard. > > Mikulas > -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
