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

[Stefan Hajnoczi]

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>

signature.asc
Description: PGP signature