All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anton Nefedov <anton.nefedov@virtuozzo.com>
To: Alberto Garcia <berto@igalia.com>, Max Reitz <mreitz@redhat.com>,
	qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, "Denis V . Lunev" <den@openvz.org>,
	qemu-block@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v3 17/39] qcow2: Update l2_allocate() to support L2 slices
Date: Thu, 1 Feb 2018 18:23:09 +0300	[thread overview]
Message-ID: <d81eae0f-1481-9dd3-0df9-40063c215784@virtuozzo.com> (raw)
In-Reply-To: <w51r2q46eg2.fsf@maestria.local.igalia.com>



On 1/2/2018 4:13 PM, Alberto Garcia wrote:
> On Wed 31 Jan 2018 09:07:48 PM CET, Max Reitz wrote:
>> On 2018-01-26 15:59, Alberto Garcia wrote:
>>> This patch updates l2_allocate() to support the qcow2 cache returning
>>> L2 slices instead of full L2 tables.
>>>
>>> The old code simply gets an L2 table from the cache and initializes it
>>> with zeroes or with the contents of an existing table. With a cache
>>> that returns slices instead of tables the idea remains the same, but
>>> the code must now iterate over all the slices that are contained in an
>>> L2 table.
>>>
>>> Since now we're operating with slices the function can no longer
>>> return the newly-allocated table, so it's up to the caller to retrieve
>>> the appropriate L2 slice after calling l2_allocate() (note that with
>>> this patch the caller is still loading full L2 tables, but we'll deal
>>> with that in a separate patch).
>>>
>>> Signed-off-by: Alberto Garcia <berto@igalia.com>
>>> ---
>>>   block/qcow2-cluster.c | 56 +++++++++++++++++++++++++++++++--------------------
>>>   1 file changed, 34 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
>>> index 57349928a9..2a53c1dc5f 100644
>>> --- a/block/qcow2-cluster.c
>>> +++ b/block/qcow2-cluster.c
>>> @@ -264,11 +264,12 @@ int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index)
>>>    *
>>>    */
>>>   
>>> -static int l2_allocate(BlockDriverState *bs, int l1_index, uint64_t **table)
>>> +static int l2_allocate(BlockDriverState *bs, int l1_index)
>>>   {
>>>       BDRVQcow2State *s = bs->opaque;
>>>       uint64_t old_l2_offset;
>>> -    uint64_t *l2_table = NULL;
>>> +    uint64_t *l2_slice = NULL;
>>> +    unsigned slice, slice_size2, n_slices;
>>
>> I'd personally prefer size_t, but oh well.
> 
> I don't see any reason not to use int / unsigned. The size of a slice is
> always <= cluster_size (an int), and both slice and n_slices are smaller
> than that.
> 
>> However, I'm wondering whether this is the best approach.  The old L2
>> table is probably not going to be used after this function, so we're
>> basically polluting the cache here.  That was bad enough so far, but
>> now that actually means wasting multiple cache entries on it.
>>
>> Sure, the code is simpler this way.  But maybe it would still be
>> better to manually copy the data over from the old offset...  (As long
>> as it's not much more complicated.)
> 
> You mean bypassing the cache altogether?
> 
>      qcow2_cache_flush(bs, s->l2_table_cache);
>      new_table = g_malloc(s->cluster_size);
>      if (old_l2_offset & L1E_OFFSET_MASK) {
>          bdrv_pread(bs->file, old_l2_offset, new_table, s->cluster_size);
>      } else {
>          memset(new_table, 0, s->cluster_size);
>      }
>      bdrv_pwrite(bs->file, new_l2_offset, new_table, s->cluster_size);
>      g_free(new_table);
> 
> ??
> 
> Berto
> 

(I know it's a draft so you probably just skipped that but just in case)
It seems ok to bypass the cache read - perhaps even a flush is
not necessary: old_l2_offset must be read-only and flushed at this
point; I believe new_l2_offset might be cached too, so it
needs to be updated.

  reply	other threads:[~2018-02-01 15:23 UTC|newest]

Thread overview: 108+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-26 14:59 [Qemu-devel] [PATCH v3 00/39] Allow configuring the qcow2 L2 cache entry size Alberto Garcia
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 01/39] qcow2: Fix documentation of get_cluster_table() Alberto Garcia
2018-01-31 19:16   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 02/39] qcow2: Add table size field to Qcow2Cache Alberto Garcia
2018-01-31 19:25   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 03/39] qcow2: Remove BDS parameter from qcow2_cache_get_table_addr() Alberto Garcia
2018-01-31 19:26   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 04/39] qcow2: Remove BDS parameter from qcow2_cache_get_table_idx() Alberto Garcia
2018-01-31 19:27   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 05/39] qcow2: Remove BDS parameter from qcow2_cache_table_release() Alberto Garcia
2018-01-31 19:28   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 06/39] qcow2: Remove BDS parameter from qcow2_cache_entry_mark_dirty() Alberto Garcia
2018-01-31 19:30   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 07/39] qcow2: Remove BDS parameter from qcow2_cache_put() Alberto Garcia
2018-01-31 19:33   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 08/39] qcow2: Remove BDS parameter from qcow2_cache_destroy() Alberto Garcia
2018-01-31 19:35   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 09/39] qcow2: Remove BDS parameter from qcow2_cache_clean_unused() Alberto Garcia
2018-01-31 19:37   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 10/39] qcow2: Remove BDS parameter from qcow2_cache_discard() Alberto Garcia
2018-01-31 19:38   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 11/39] qcow2: Remove BDS parameter from qcow2_cache_is_table_offset() Alberto Garcia
2018-01-31 19:39   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 12/39] qcow2: Add offset_to_l1_index() Alberto Garcia
2018-01-31 19:43   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 13/39] qcow2: Add l2_slice_size field to BDRVQcow2State Alberto Garcia
2018-01-31 19:48   ` Max Reitz
2018-02-01  9:51     ` Alberto Garcia
2018-02-01 18:09       ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 14/39] qcow2: Add offset_to_l2_slice_index() Alberto Garcia
2018-01-31 19:49   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 15/39] qcow2: Update l2_load() to support L2 slices Alberto Garcia
2018-01-31 19:56   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 16/39] qcow2: Prepare l2_allocate() for adding L2 slice support Alberto Garcia
2018-01-26 16:24   ` Eric Blake
2018-01-31 19:57   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 17/39] qcow2: Update l2_allocate() to support L2 slices Alberto Garcia
2018-01-26 16:30   ` Eric Blake
2018-01-31 20:07   ` Max Reitz
2018-02-01 13:13     ` Alberto Garcia
2018-02-01 15:23       ` Anton Nefedov [this message]
2018-02-01 15:43         ` Alberto Garcia
2018-02-01 18:22           ` Max Reitz
2018-02-02  8:08             ` Alberto Garcia
2018-02-01 18:15       ` Max Reitz
2018-02-02  9:41         ` Alberto Garcia
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 18/39] qcow2: Refactor get_cluster_table() Alberto Garcia
2018-01-26 16:37   ` Eric Blake
2018-01-31 20:11   ` Max Reitz
2018-02-01 10:40     ` Alberto Garcia
2018-02-01 18:10       ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 19/39] qcow2: Update get_cluster_table() to support L2 slices Alberto Garcia
2018-01-26 16:39   ` Eric Blake
2018-01-31 20:14   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 20/39] qcow2: Update qcow2_get_cluster_offset() " Alberto Garcia
2018-01-31 20:24   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 21/39] qcow2: Update qcow2_alloc_cluster_link_l2() " Alberto Garcia
2018-02-01 18:44   ` Max Reitz
2018-02-02  9:43     ` Alberto Garcia
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 22/39] qcow2: Update handle_copied() " Alberto Garcia
2018-02-01 18:54   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 23/39] qcow2: Update handle_alloc() " Alberto Garcia
2018-02-01 18:56   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 24/39] qcow2: Update discard_single_l2() " Alberto Garcia
2018-02-01 19:07   ` Max Reitz
2018-02-02  9:46     ` Alberto Garcia
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 25/39] qcow2: Update zero_single_l2() " Alberto Garcia
2018-02-01 19:08   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 26/39] qcow2: Prepare qcow2_update_snapshot_refcount() for adding L2 slice support Alberto Garcia
2018-01-26 19:38   ` Eric Blake
2018-02-01 19:21   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 27/39] qcow2: Update qcow2_update_snapshot_refcount() to support L2 slices Alberto Garcia
2018-01-26 19:39   ` Eric Blake
2018-02-01 19:26   ` Max Reitz
2018-02-02  9:56     ` Alberto Garcia
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 28/39] qcow2: Read refcount before L2 table in expand_zero_clusters_in_l1() Alberto Garcia
2018-01-26 19:41   ` Eric Blake
2018-02-01 19:29   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 29/39] qcow2: Prepare expand_zero_clusters_in_l1() for adding L2 slice support Alberto Garcia
2018-01-26 19:42   ` Eric Blake
2018-02-01 19:34   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 30/39] qcow2: Update expand_zero_clusters_in_l1() to support L2 slices Alberto Garcia
2018-01-26 19:46   ` Eric Blake
2018-01-29 12:06     ` Alberto Garcia
2018-02-01 19:44   ` Max Reitz
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 31/39] qcow2: Update qcow2_truncate() " Alberto Garcia
2018-02-01 19:46   ` Max Reitz
2018-02-02 10:10     ` Alberto Garcia
2018-01-26 14:59 ` [Qemu-devel] [PATCH v3 32/39] qcow2: Rename l2_table in qcow2_alloc_compressed_cluster_offset() Alberto Garcia
2018-02-01 19:48   ` Max Reitz
2018-01-26 15:00 ` [Qemu-devel] [PATCH v3 33/39] qcow2: Rename l2_table in count_contiguous_clusters() Alberto Garcia
2018-02-01 19:49   ` Max Reitz
2018-01-26 15:00 ` [Qemu-devel] [PATCH v3 34/39] qcow2: Rename l2_table in count_contiguous_clusters_unallocated() Alberto Garcia
2018-02-01 19:49   ` Max Reitz
2018-01-26 15:00 ` [Qemu-devel] [PATCH v3 35/39] qcow2: Rename l2_table in count_cow_clusters() Alberto Garcia
2018-02-01 19:50   ` Max Reitz
2018-01-26 15:00 ` [Qemu-devel] [PATCH v3 36/39] qcow2: Allow configuring the L2 slice size Alberto Garcia
2018-01-31 19:20   ` Eric Blake
2018-02-01 20:04   ` Max Reitz
2018-01-26 15:00 ` [Qemu-devel] [PATCH v3 37/39] iotests: Test valid values of l2-cache-entry-size Alberto Garcia
2018-01-31 19:21   ` Eric Blake
2018-02-01 20:05   ` Max Reitz
2018-01-26 15:00 ` [Qemu-devel] [PATCH v3 38/39] iotests: Test downgrading an image using a small L2 slice size Alberto Garcia
2018-01-31 19:23   ` Eric Blake
2018-02-01 20:11   ` Max Reitz
2018-01-26 15:00 ` [Qemu-devel] [PATCH v3 39/39] iotests: Add l2-cache-entry-size to iotest 137 Alberto Garcia
2018-01-31 19:23   ` Eric Blake
2018-02-01 20:12   ` Max Reitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d81eae0f-1481-9dd3-0df9-40063c215784@virtuozzo.com \
    --to=anton.nefedov@virtuozzo.com \
    --cc=berto@igalia.com \
    --cc=den@openvz.org \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.