[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/5] block/io: introduce bdrv_try_mark_request_serialising

From: Stefan Hajnoczi
Subject: Re: [PATCH 1/5] block/io: introduce bdrv_try_mark_request_serialising
Date: Tue, 7 Jul 2020 16:56:40 +0100

On Sat, Jun 20, 2020 at 05:36:45PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Introduce a function to mark the request serialising only if there are
> no conflicting request to wait for.
> The function is static, so mark it unused. The attribute is to be
> dropped in the next commit.
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/io.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 51 insertions(+), 7 deletions(-)

I found this patch difficult to understand because there are multiple
levels of functions passing flags to ultimiately do different things in
a common function.

Here are some ideas if you have time to rework this patch:

1. Introduce a bdrv_find_overlapping_request() function that does most
   of bdrv_wait_serialising_requests_locked() but does not wait. Then
   bdrv_wait_serialising_requests_locked() can call that function in a
   loop and wait if an overlapping request is found.

2. Pass overlap_offset/overlap_bytes arguments to
   bdrv_find_overlapping_request() instead of changing and restoring the
   value in bdrv_do_mark_request_serialising().

3. Use consistent names for flags: wait/blocking, found/success

I'm not sure if all these ideas will work, but I get the feeling this
code can be refactored to make it easier to understand. Since I don't
have a concrete suggestion and the code looks correct:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Attachment: signature.asc
Description: PGP signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]