All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	Max Reitz <mreitz@redhat.com>,
	qemu-block@nongnu.org
Cc: qemu-devel@nongnu.org, kwolf@redhat.com, fam@euphon.net,
	stefanha@redhat.com, armbru@redhat.com, jsnow@redhat.com,
	libvir-list@redhat.com, eblake@redhat.com, den@openvz.org
Subject: Re: [PATCH v11 09/13] copy-on-read: skip non-guest reads if no copy needed
Date: Thu, 22 Oct 2020 10:50:39 +0300	[thread overview]
Message-ID: <4ae47559-af39-1dcc-5e22-f7259b55dfee@virtuozzo.com> (raw)
In-Reply-To: <519fd52f-cb9e-0ab1-6d50-a9b3004d86fe@virtuozzo.com>


On 21.10.2020 23:43, Andrey Shinkevich wrote:
> On 14.10.2020 18:22, Vladimir Sementsov-Ogievskiy wrote:
>> 14.10.2020 15:51, Max Reitz wrote:
>>> On 12.10.20 19:43, Andrey Shinkevich wrote:
>>>> If the flag BDRV_REQ_PREFETCH was set, pass it further to the
>>>> COR-driver to skip unneeded reading. It can be taken into account for
>>>> the COR-algorithms optimization. That check is being made during the
>>>> block stream job by the moment.
>>>>
>>>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>>>> ---

[...]

>>>> diff --git a/block/io.c b/block/io.c
>>>> index 11df188..bff1808 100644
>>>> --- a/block/io.c
>>>> +++ b/block/io.c
>>>> @@ -1512,7 +1512,8 @@ static int coroutine_fn 
>>>> bdrv_aligned_preadv(BdrvChild *child,
>>>>       max_bytes = ROUND_UP(MAX(0, total_bytes - offset), align);
>>>>       if (bytes <= max_bytes && bytes <= max_transfer) {
>>>> -        ret = bdrv_driver_preadv(bs, offset, bytes, qiov, 
>>>> qiov_offset, 0);
>>>> +        ret = bdrv_driver_preadv(bs, offset, bytes, qiov, qiov_offset,
>>>> +                                 flags & bs->supported_read_flags);
>>
>>
>> When BDRV_REQ_PREFETCH is passed, qiov may be (and generally should 
>> be) NULL. This means, that we can't just drop the flag when call the 
>> driver that doesn't support it.
>>
>> Actually, if driver doesn't support the PREFETCH flag we should do 
>> nothing.
>>
>>
>>>
>>> Ah, OK.  I see.  I expected this to be a separate patch.  I still wonder
>>> why it isn’t.
>>>
>>
>>
>> Could it be part of patch 07? I mean introduce new field 
>> supported_read_flags and handle it in generic code in one patch, prior 
>> to implementing support for it in COR driver.
>>
>>
> 
> We have to add the supported flags for the COR driver in the same patch. 
> Or before handling the supported_read_flags at the generic layer 
> (handling zero does not make a sence). Otherwise, the test #216 (where 
> the COR-filter is applied) will not pass.
> 
> Andrey

I have found a workaround and am going to send all the related patches 
as a separate series.

Andrey


  reply	other threads:[~2020-10-22  7:57 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-12 17:43 [PATCH v11 00/13] Apply COR-filter to the block-stream permanently Andrey Shinkevich via
2020-10-12 17:43 ` [PATCH v11 01/13] copy-on-read: Support preadv/pwritev_part functions Andrey Shinkevich via
2020-10-12 17:43 ` [PATCH v11 02/13] copy-on-read: add filter append/drop functions Andrey Shinkevich via
2020-10-14 10:44   ` Max Reitz
2020-10-14 14:28     ` Andrey Shinkevich
2020-10-14 16:26       ` Max Reitz
2020-10-12 17:43 ` [PATCH v11 03/13] qapi: add filter-node-name to block-stream Andrey Shinkevich via
2020-10-12 17:43 ` [PATCH v11 04/13] copy-on-read: pass overlay base node name to COR driver Andrey Shinkevich via
2020-10-14 11:09   ` Max Reitz
2020-10-14 11:57     ` Max Reitz
2020-10-14 14:56       ` Vladimir Sementsov-Ogievskiy
2020-10-14 16:27         ` Max Reitz
2020-10-14 16:08     ` Andrey Shinkevich
2020-10-14 16:18       ` Vladimir Sementsov-Ogievskiy
2020-10-14 16:36       ` Max Reitz
2020-10-12 17:43 ` [PATCH v11 05/13] copy-on-read: limit COR operations to base in " Andrey Shinkevich via
2020-10-14 11:59   ` Max Reitz
2020-10-14 17:43     ` Andrey Shinkevich
2020-10-14 12:01   ` Max Reitz
2020-10-14 18:57     ` Andrey Shinkevich
2020-10-15 15:56       ` Max Reitz
2020-10-15 17:37         ` Andrey Shinkevich
2020-10-16 14:28           ` Vladimir Sementsov-Ogievskiy
2020-10-12 17:43 ` [PATCH v11 06/13] block: modify the comment for BDRV_REQ_PREFETCH flag Andrey Shinkevich via
2020-10-14 12:22   ` Max Reitz
2020-10-14 15:04     ` Vladimir Sementsov-Ogievskiy
2020-10-14 19:57     ` Andrey Shinkevich
2020-10-12 17:43 ` [PATCH v11 07/13] block: include supported_read_flags into BDS structure Andrey Shinkevich via
2020-10-14 12:31   ` Max Reitz
2020-10-12 17:43 ` [PATCH v11 08/13] copy-on-read: add support for BDRV_REQ_PREFETCH to COR-filter Andrey Shinkevich via
2020-10-14 12:40   ` Max Reitz
2020-10-12 17:43 ` [PATCH v11 09/13] copy-on-read: skip non-guest reads if no copy needed Andrey Shinkevich via
2020-10-14 12:51   ` Max Reitz
2020-10-14 15:22     ` Vladimir Sementsov-Ogievskiy
2020-10-14 16:30       ` Max Reitz
2020-10-14 16:39         ` Vladimir Sementsov-Ogievskiy
2020-10-15 15:49           ` Max Reitz
2020-10-21 20:43       ` Andrey Shinkevich
2020-10-22  7:50         ` Andrey Shinkevich [this message]
2020-10-22  8:56           ` Vladimir Sementsov-Ogievskiy
2020-10-14 20:49     ` Andrey Shinkevich
2020-10-12 17:43 ` [PATCH v11 10/13] stream: skip filters when writing backing file name to QCOW2 header Andrey Shinkevich via
2020-10-14 15:02   ` Max Reitz
2020-10-14 15:40     ` Vladimir Sementsov-Ogievskiy
2020-10-12 17:43 ` [PATCH v11 11/13] stream: mark backing-file argument as deprecated Andrey Shinkevich via
2020-10-14 15:03   ` Max Reitz
2020-10-14 15:43     ` Vladimir Sementsov-Ogievskiy
2020-10-15  9:01       ` Andrey Shinkevich
2020-10-12 17:43 ` [PATCH v11 12/13] stream: remove unused backing-file name parameter Andrey Shinkevich via
2020-10-14 15:05   ` Max Reitz
2020-10-12 17:43 ` [PATCH v11 13/13] block: apply COR-filter to block-stream jobs Andrey Shinkevich via
2020-10-14 16:24   ` Max Reitz
2020-10-15 17:16     ` Andrey Shinkevich
2020-10-16 15:06       ` Andrey Shinkevich
2020-10-16 15:45       ` Vladimir Sementsov-Ogievskiy
2020-10-20 19:43         ` Andrey Shinkevich

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=4ae47559-af39-1dcc-5e22-f7259b55dfee@virtuozzo.com \
    --to=andrey.shinkevich@virtuozzo.com \
    --cc=armbru@redhat.com \
    --cc=den@openvz.org \
    --cc=eblake@redhat.com \
    --cc=fam@euphon.net \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@virtuozzo.com \
    /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.