[dm-devel] [PATCH] dm-bufio: implement discard

[John Dorminy]

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 at redhat.com>

Thanks!


On Fri, Feb 7, 2020 at 3:07 PM Mikulas Patocka <mpatocka at redhat.com> 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
>