Re: [Qemu-block] [PATCH v4 00/39] Allow configuring the qcow2 L2 cache entry size

2018-02-13 Thread Max Reitz
On 2018-02-05 15:33, Alberto Garcia wrote:
> this is the new revision of the patch series to allow configuring the
> entry size of the qcow2 L2 cache. Follow this link for the full
> description from the first version:
> 
>https://lists.gnu.org/archive/html/qemu-block/2017-10/msg00458.html
> 
> And here are some numbers showing the performance improvements:
> 
>https://lists.gnu.org/archive/html/qemu-block/2017-12/msg00507.html
> 
> Regards,
> 
> Berto

Thanks, applied to my block branch
(with s/intead/instead/ and s/=300/=4242/):

https://github.com/XanClic/qemu/commits/block

Max



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-block] [PATCH v4 00/39] Allow configuring the qcow2 L2 cache entry size

2018-02-13 Thread Kevin Wolf
Am 05.02.2018 um 17:31 hat Max Reitz geschrieben:
> On 2018-02-05 15:33, Alberto Garcia wrote:
> > this is the new revision of the patch series to allow configuring the
> > entry size of the qcow2 L2 cache. Follow this link for the full
> > description from the first version:
> > 
> >https://lists.gnu.org/archive/html/qemu-block/2017-10/msg00458.html
> > 
> > And here are some numbers showing the performance improvements:
> > 
> >https://lists.gnu.org/archive/html/qemu-block/2017-12/msg00507.html
> > 
> > Regards,
> > 
> > Berto
> 
> Looks good to me, and all qcow2 iotests pass when the default L2 slice
> size is changed (in the code) to 512, so it should be good.
> 
> I'll still give it a couple of days for others to protest. :-)

I didn't review the series in detail, but the approach looks sane to me,
so no objections from my side.

Kevin


signature.asc
Description: PGP signature


Re: [Qemu-block] [PATCH v4 00/39] Allow configuring the qcow2 L2 cache entry size

2018-02-05 Thread Max Reitz
On 2018-02-05 15:33, Alberto Garcia wrote:
> this is the new revision of the patch series to allow configuring the
> entry size of the qcow2 L2 cache. Follow this link for the full
> description from the first version:
> 
>https://lists.gnu.org/archive/html/qemu-block/2017-10/msg00458.html
> 
> And here are some numbers showing the performance improvements:
> 
>https://lists.gnu.org/archive/html/qemu-block/2017-12/msg00507.html
> 
> Regards,
> 
> Berto

Looks good to me, and all qcow2 iotests pass when the default L2 slice
size is changed (in the code) to 512, so it should be good.

I'll still give it a couple of days for others to protest. :-)

Max



signature.asc
Description: OpenPGP digital signature


[Qemu-block] [PATCH v4 00/39] Allow configuring the qcow2 L2 cache entry size

2018-02-05 Thread Alberto Garcia
this is the new revision of the patch series to allow configuring the
entry size of the qcow2 L2 cache. Follow this link for the full
description from the first version:

   https://lists.gnu.org/archive/html/qemu-block/2017-10/msg00458.html

And here are some numbers showing the performance improvements:

   https://lists.gnu.org/archive/html/qemu-block/2017-12/msg00507.html

Regards,

Berto

Changes:

v4:
- Rebased on top of f24ee107a07f093bd7ed475dd48d7ba57ea3d8fe
- Patch 17: Fix typo in comment
- Patch 24: Rename discard_single_l2() to discard_in_l2_slice() [Max]
- Patch 25: Rename zero_single_l2() to zero_in_l2_slice() [Max]
- Patch 27: Add comment clarifying the l2_index variable [Max]
- Patch 30: Remove unneeded void * casts [Eric, Max]
- Patch 31: Use offset_to_l2_slice_index() instead of doing it manually [Max]

v3: https://lists.gnu.org/archive/html/qemu-block/2018-01/msg00635.html
- Rebased on top of 1867d97b372d452184c65e8fcc273cfc45937541
- Patch 2: Use QEMU_IS_ALIGNED() instead of doing the bit operations
   manually [Eric]
- Patch 13: Clarify that l2_slice_size means size in entries, not
bytes [Eric].
- Patch 16-17: The previous code has been splitted into patches 16 and
   17 for clarity.
- Patch 17: Use slice_size2 for storing the size of the slice in bytes
(keeping the existing convention used in l1_size2 or
refcount_table_size2)
- Patch 17: Flush the cache only once [Anton, Eric]
- Patch 17: Call l2_load() after qcow2_free_clusters() [Anton]
- Patch 18 [new]: Refactor the l2_load() call in get_cluster_table()
   [Anton]
- Patch 19: Fix typo [Eric]
- Patch 26-27: The previous code has been splitted into patches 26 and
   27 for clarity.
- Patch 27: Use slice_size2 for storing the size of the slice in bytes
- Patch 28-30: The previous code has been splitted into patches 28, 29
   and 30 for clarity.
- Patch 30: Use slice_size2 for storing the size of the slice in bytes
- Patch 30: Replace bdrv_read/write() with bdrv_pread/pwrite() [Eric]
- Patch 30: Initialize l2_dirty inside the inner loop [Anton]
- Patch 36: Fix maybe-uninitialized warning of l2_cache_entry_size
[Anton]
- Patch 36: Make MIN_L2_CACHE_SIZE the minimum number of cache
entries, not clusters [Anton]
- Patch 36: Add l2-cache-entry-size to BlockdevOptionsQcow2 [Eric]
- Patch 37-39: New iotests.

v2: https://lists.gnu.org/archive/html/qemu-block/2017-12/msg00507.html
- Rebased after the v2.11.0 release.
- Patch 2: Adjust the unaligned access check introduce by Max in 4efb1f7c612
- Patch 18: Prevent overflow when computing bytes_available in
  qcow2_get_cluster_offset()
- Patch 31: Fix typo in error message in read_cache_sizes()
- Patch 32 [new]: Add test for l2-cache-entry-size'

v1: https://lists.gnu.org/archive/html/qemu-block/2017-10/msg00458.html
- Initial version

Here's the ouput of git backport-diff against v3:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/39:[] [--] 'qcow2: Fix documentation of get_cluster_table()'
002/39:[] [--] 'qcow2: Add table size field to Qcow2Cache'
003/39:[] [--] 'qcow2: Remove BDS parameter from 
qcow2_cache_get_table_addr()'
004/39:[] [--] 'qcow2: Remove BDS parameter from 
qcow2_cache_get_table_idx()'
005/39:[] [--] 'qcow2: Remove BDS parameter from 
qcow2_cache_table_release()'
006/39:[] [--] 'qcow2: Remove BDS parameter from 
qcow2_cache_entry_mark_dirty()'
007/39:[] [--] 'qcow2: Remove BDS parameter from qcow2_cache_put()'
008/39:[] [--] 'qcow2: Remove BDS parameter from qcow2_cache_destroy()'
009/39:[] [--] 'qcow2: Remove BDS parameter from qcow2_cache_clean_unused()'
010/39:[] [--] 'qcow2: Remove BDS parameter from qcow2_cache_discard()'
011/39:[] [--] 'qcow2: Remove BDS parameter from 
qcow2_cache_is_table_offset()'
012/39:[] [--] 'qcow2: Add offset_to_l1_index()'
013/39:[] [--] 'qcow2: Add l2_slice_size field to BDRVQcow2State'
014/39:[] [--] 'qcow2: Add offset_to_l2_slice_index()'
015/39:[] [--] 'qcow2: Update l2_load() to support L2 slices'
016/39:[] [--] 'qcow2: Prepare l2_allocate() for adding L2 slice support'
017/39:[0002] [FC] 'qcow2: Update l2_allocate() to support L2 slices'
018/39:[] [--] 'qcow2: Refactor get_cluster_table()'
019/39:[] [--] 'qcow2: Update get_cluster_table() to support L2 slices'
020/39:[] [--] 'qcow2: Update qcow2_get_cluster_offset() to support L2 
slices'
021/39:[] [--] 'qcow2: Update qcow2_alloc_cluster_link_l2() to support L2 
slices'
022/39:[] [--] 'qcow2: Update handle_copied() to support L2 slices'
023/39:[] [--] 'qcow2: Update handle_alloc() to support L2 slices'
024/39:[0012] [FC] 'qcow2: Update discard_single_l2() to support L2 slices'
025/39:[0008] [FC] 'qcow2: Update