2011/9/13 Kevin Wolf <kw...@redhat.com>: > Am 13.09.2011 09:53, schrieb Frediano Ziglio: >> These patches try to trade-off between leaks and speed for clusters >> refcounts. >> >> Refcount increments (REF+ or refp) are handled in a different way from >> decrements (REF- or refm). The reason it that posting or not flushing >> a REF- cause "just" a leak while posting a REF+ cause a corruption. >> >> To optimize REF- I just used an array to store offsets then when a >> flush is requested or array reach a limit (currently 1022) the array >> is sorted and written to disk. I use an array with offset instead of >> ranges to support compression (an offset could appear multiple times >> in the array). >> I consider this patch quite ready. > > Ok, first of all let's clarify what this optimises. I don't think it > changes anything at all for the writeback cache modes, because these > already do most operations in memory only. So this must be about > optimising some operations with cache=writethrough. REF- isn't about > normal cluster allocation, it is about COW with internal snapshots or > bdrv_discard. Do you have benchmarks for any of them? > > I strongly disagree with your approach for REF-. We already have a > cache, and introducing a second one sounds like a bad idea. I think we > could get a very similar effect if we introduced a > qcow2_cache_entry_mark_dirty_wb() that marks a given refcount block as > dirty, but at the same time tells the cache that even in write-through > mode it can still treat this block as write-back. This should require > much less code changes. > > But let's measure the effects first, I suspect that for cluster > allocation it doesn't help much because every REF- comes with a REF+. > >> To optimize REF+ I mark a range as allocated and use this range to >> get new ones (avoiding writing refcount to disk). When a flush is >> requested or in some situations (like snapshot) this cache is disabled >> and flushed (written as REF-). >> I do not consider this patch ready, it works and pass all io-tests >> but for instance I would avoid allocating new clusters for refcount >> during preallocation. > > The only question here is if improving cache=writethrough cluster > allocation performance is worth the additional complexity in the already > complex refcounting code. > > The alternative that was discussed before is the dirty bit approach that > is used in QED and would allow us to use writeback for all refcount > blocks, regardless of REF- or REF+. It would be an easier approach > requiring less code changes, but it comes with the cost of requiring an > fsck after a qemu crash. > >> End speed up is quite visible allocating clusters (more then 20%). > > What benchmark do you use for testing this? > > Kevin >
Here you are some results (kb/s) with patch (ref-, ref+), qcow2s is qcow2 with a snapshot run raw qcow2 qcow2s 1 22748 4878 4792 2 29557 15839 23144 without run raw qcow2 qcow2s 1 21979 4308 1021 2 26249 13776 24182 Frediano