qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH for-5.0 v3 2/3] block: Increase BB.in_flight for coroutine an


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH for-5.0 v3 2/3] block: Increase BB.in_flight for coroutine and sync interfaces
Date: Tue, 7 Apr 2020 20:36:50 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1

07.04.2020 19:27, Kevin Wolf wrote:
Am 07.04.2020 um 16:56 hat Vladimir Sementsov-Ogievskiy geschrieben:
07.04.2020 17:42, Kevin Wolf wrote:
Am 07.04.2020 um 16:22 hat Vladimir Sementsov-Ogievskiy geschrieben:
07.04.2020 15:12, Kevin Wolf wrote:
External callers of blk_co_*() and of the synchronous blk_*() functions
don't currently increase the BlockBackend.in_flight counter, but calls
from blk_aio_*() do, so there is an inconsistency whether the counter
has been increased or not.

This patch moves the actual operations to static functions that can
later know they will always be called with in_flight increased exactly
once, even for external callers using the blk_co_*() coroutine
interfaces.

If the public blk_co_*() interface is unused, remove it.

Signed-off-by: Kevin Wolf <address@hidden>

Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>

side question:

Should we inc/dec in blk_make_zero, blk_truncate?

I don't think it's necessary. They call into their bdrv_* counterpart
immediately, so the node-level counter should be enough.


bdrv_make_zero is not one request, it does block_status/pwrite_zeroes
in a loop. So drained section may occur during bdrv_make_zero.
Possibly, nothing bad in it?

It would potentially be a problem if it were called in coroutine
context. But it's a synchronous function that must be called in the main
thread (and also only used in qemu-img), so I don't see how drain could
happen while it runs.

If we did want to make it safe for use in coroutine context, it would be
by using bdrv_inc/dec_in_flight() in bdrv_make_zero().

blk_truncate may do coroutine_enter before incrementing node-level
counter, which may only schedule it..

This is bdrv_truncate(), not blk_truncate(). If you address it in
blk_truncate(), you miss the direct callers of bdrv_truncate().

But you're right that it could potentially be a problem. Not sure if it
really is, but maybe better safe than sorry, so if you want to send a
patch, go ahead.


Hmm. Same thing may be said about all other coroutine-enter wrappers in 
block/io.c. OK, I'll make a patch.


--
Best regards,
Vladimir



reply via email to

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