Re: [PATCH] dm-bufio: implement discard

[John Dorminy <jdorminy@redhat.com>]

> +/*
> + * Free the specified range of buffers. If a buffer is held by other process, it
> + * is not freed. If a buffer is dirty, it is discarded without writeback.
> + * Finally, send the discard request to the device.
Might be clearer to say "After freeing, send a discard request for the
specified range to the device." to clarify that it's all cases, not
just the dirty-buffer case mentioned in the previous sentence.

> + */
> +int dm_bufio_discard_buffers(struct dm_bufio_client *c, sector_t block, sector_t count)
> +{
> +       sector_t i;
> +
> +       for (i = block; i < block + count; i++) {
> +               struct dm_buffer *b;
> +               dm_bufio_lock(c);
> +               b = __find(c, i);
> +               if (b && likely(!b->hold_count)) {
> +                       wait_on_bit_io(&b->state, B_READING, TASK_UNINTERRUPTIBLE);
> +                       wait_on_bit_io(&b->state, B_WRITING, TASK_UNINTERRUPTIBLE);
> +                       __unlink_buffer(b);
> +                       __free_buffer_wake(b);
> +               }
> +               dm_bufio_unlock(c);
> +       }
> +
> +       return dm_bufio_issue_discard(c, block, count);
> +}
> +EXPORT_SYMBOL_GPL(dm_bufio_discard_buffers);

This seems dangerous -- if another process is holding the buffer, you
could be issuing a discard while they are reading or writing, or vice
versa.

Discards whose lifetime overlaps with the lifetime of a read or write
to the same region have undefined behavior, as far as I know.

Perhaps dm_bufio_issue_discard() should allow the caller to specify a
callback, in which case dm_bufio_discard_buffers could unlock any
buffers in the range upon completion; or dm_bufio_discard_buffers
should not issue discards for blocks held by other processes.