Re: [PATCH, RFC] virtio_blk: add cache flush command

Avi Kivity
2009-05-11T16:50:15Z

Christoph Hellwig wrote:
> On Mon, May 11, 2009 at 06:45:50PM +0300, Avi Kivity wrote:
>
>>> Right now it's fsync. By the time I'll submit the backend change it
>>> will still be fsync, but at least called from the posix-aio-compat
>>> thread pool.
>>>
>>>
>> I think if we have cache=writeback we should ignore this.
>>
>
> It's only needed for cache=writeback, because without that there is no
> reason to flush a write cache.
>

Maybe we should add a fourth cache= mode then. But
cache=writeback+fsync doesn't correspond to any real world drive; in the
real world you're limited to power failures and a few megabytes of cache
(typically less), cache=writeback+fsync can lose hundreds of megabytes
due to power loss or software failure.

Oh, and cache=writeback+fsync doesn't work on qcow2, unless we add fsync
after metadata updates.

>> For cache=none and cache=writethrough we don't really need fsync, but we
>> do need to flush the inflight commands.
>>
>
> What we do need for those modes is the basic barrier support because
> we can currently re-order requests. The next version of my patch will
> implement a barriers without cache flush mode, although I don't think
> a fdatasync without any outstanding dirty data should cause problems.
>

Yeah. And maybe one day push the barrier into the kernel.

> (Or maybe ext3 actually is stupid enough to flush the whole fs even for
> that case

Sigh.

--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.