Message ID | 20191127180840.11937-4-vsementsov@virtuozzo.com |
---|---|
State | New |
Headers | show |
Series | block-copy improvements: part I | expand |
On 27/11/2019 21:08, Vladimir Sementsov-Ogievskiy wrote: > Split block_copy_find_inflight_req to be used in seprate. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > block/block-copy.c | 31 +++++++++++++++++++------------ > 1 file changed, 19 insertions(+), 12 deletions(-) > > diff --git a/block/block-copy.c b/block/block-copy.c > index 74295d93d5..94e7e855ef 100644 > --- a/block/block-copy.c > +++ b/block/block-copy.c > @@ -24,23 +24,30 @@ > #define BLOCK_COPY_MAX_BUFFER (1 * MiB) > #define BLOCK_COPY_MAX_MEM (128 * MiB) > > +static BlockCopyInFlightReq *block_copy_find_inflight_req(BlockCopyState *s, > + int64_t start, > + int64_t end) > +{ > + BlockCopyInFlightReq *req; > + > + QLIST_FOREACH(req, &s->inflight_reqs, list) { > + if (end > req->start_byte && start < req->end_byte) { > + return req; > + } > + } > + > + return NULL; > +} > + > static void coroutine_fn block_copy_wait_inflight_reqs(BlockCopyState *s, > int64_t start, > int64_t end) > { > BlockCopyInFlightReq *req; > - bool waited; > - > - do { > - waited = false; > - QLIST_FOREACH(req, &s->inflight_reqs, list) { > - if (end > req->start_byte && start < req->end_byte) { > - qemu_co_queue_wait(&req->wait_queue, NULL); > - waited = true; > - break; > - } > - } > - } while (waited); > + > + while ((req = block_copy_find_inflight_req(s, start, end))) { > + qemu_co_queue_wait(&req->wait_queue, NULL); > + } > } > > static void block_copy_inflight_req_begin(BlockCopyState *s, > Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
On 27.11.19 19:08, Vladimir Sementsov-Ogievskiy wrote: > Split block_copy_find_inflight_req to be used in seprate. *separate, but separate what? > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > block/block-copy.c | 31 +++++++++++++++++++------------ > 1 file changed, 19 insertions(+), 12 deletions(-) > > diff --git a/block/block-copy.c b/block/block-copy.c > index 74295d93d5..94e7e855ef 100644 > --- a/block/block-copy.c > +++ b/block/block-copy.c > @@ -24,23 +24,30 @@ > #define BLOCK_COPY_MAX_BUFFER (1 * MiB) > #define BLOCK_COPY_MAX_MEM (128 * MiB) > > +static BlockCopyInFlightReq *block_copy_find_inflight_req(BlockCopyState *s, Hm. Long already, but the name begs the question what in-flight requests this is about. Maybe drop the block_copy prefix (unless you plan to make this function global) and call it “find_inflight_conflict”? (Or “find_conflicting_inflight_req” to be more verbose) > + int64_t start, > + int64_t end) > +{ > + BlockCopyInFlightReq *req; > + > + QLIST_FOREACH(req, &s->inflight_reqs, list) { > + if (end > req->start_byte && start < req->end_byte) { > + return req; > + } > + } > + > + return NULL; > +} > + > static void coroutine_fn block_copy_wait_inflight_reqs(BlockCopyState *s, > int64_t start, > int64_t end) > { > BlockCopyInFlightReq *req; > - bool waited; > - > - do { > - waited = false; > - QLIST_FOREACH(req, &s->inflight_reqs, list) { > - if (end > req->start_byte && start < req->end_byte) { > - qemu_co_queue_wait(&req->wait_queue, NULL); > - waited = true; > - break; > - } > - } > - } while (waited); > + > + while ((req = block_copy_find_inflight_req(s, start, end))) { > + qemu_co_queue_wait(&req->wait_queue, NULL); > + } > } The change itself looks rather nice to me. Even without other users of this new function, it turns what’s happening into a more obvious pattern. Max
diff --git a/block/block-copy.c b/block/block-copy.c index 74295d93d5..94e7e855ef 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -24,23 +24,30 @@ #define BLOCK_COPY_MAX_BUFFER (1 * MiB) #define BLOCK_COPY_MAX_MEM (128 * MiB) +static BlockCopyInFlightReq *block_copy_find_inflight_req(BlockCopyState *s, + int64_t start, + int64_t end) +{ + BlockCopyInFlightReq *req; + + QLIST_FOREACH(req, &s->inflight_reqs, list) { + if (end > req->start_byte && start < req->end_byte) { + return req; + } + } + + return NULL; +} + static void coroutine_fn block_copy_wait_inflight_reqs(BlockCopyState *s, int64_t start, int64_t end) { BlockCopyInFlightReq *req; - bool waited; - - do { - waited = false; - QLIST_FOREACH(req, &s->inflight_reqs, list) { - if (end > req->start_byte && start < req->end_byte) { - qemu_co_queue_wait(&req->wait_queue, NULL); - waited = true; - break; - } - } - } while (waited); + + while ((req = block_copy_find_inflight_req(s, start, end))) { + qemu_co_queue_wait(&req->wait_queue, NULL); + } } static void block_copy_inflight_req_begin(BlockCopyState *s,
Split block_copy_find_inflight_req to be used in seprate. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- block/block-copy.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-)