| Message ID | 20200325134639.16337-6-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | block-copy: use aio-task-pool | expand |
On 25.03.20 14:46, Vladimir Sementsov-Ogievskiy wrote: > Simple movement without any change. It's needed for the following > patch, as this function will need to use some staff which is currently *stuff > below it. Wouldn’t it be simpler to just declare block_copy_task_entry()? Max > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > block/block-copy.c | 66 +++++++++++++++++++++++----------------------- > 1 file changed, 33 insertions(+), 33 deletions(-)
28.04.2020 12:06, Max Reitz wrote: > On 25.03.20 14:46, Vladimir Sementsov-Ogievskiy wrote: >> Simple movement without any change. It's needed for the following >> patch, as this function will need to use some staff which is currently > > *stuff > >> below it. > > Wouldn’t it be simpler to just declare block_copy_task_entry()? > I just think, that it's good to keep native order of functions and avoid extra declarations. Still, may be I care too much. No actual difference, if you prefer declaration, I can drop this patch. > >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> block/block-copy.c | 66 +++++++++++++++++++++++----------------------- >> 1 file changed, 33 insertions(+), 33 deletions(-) >
On 28.04.20 11:17, Vladimir Sementsov-Ogievskiy wrote: > 28.04.2020 12:06, Max Reitz wrote: >> On 25.03.20 14:46, Vladimir Sementsov-Ogievskiy wrote: >>> Simple movement without any change. It's needed for the following >>> patch, as this function will need to use some staff which is currently >> >> *stuff >> >>> below it. >> >> Wouldn’t it be simpler to just declare block_copy_task_entry()? >> > > I just think, that it's good to keep native order of functions and avoid > extra declarations. Still, may be I care too much. No actual difference, > if you prefer declaration, I can drop this patch. Personally, the native order doesn’t do me any good (cscope doesn’t really care where the definition is), and also having functions in order seems just like a C artifact. I just prefer declarations because otherwise we end up moving functions all the time with no real benefit. Furthermore, moving functions has the drawback of polluting git blame. Max
diff --git a/block/block-copy.c b/block/block-copy.c index dd406eb4bb..910947cb43 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -106,39 +106,6 @@ static bool coroutine_fn block_copy_wait_one(BlockCopyState *s, int64_t offset, return true; } -static BlockCopyTask *block_copy_task_create(BlockCopyState *s, - int64_t offset, int64_t bytes) -{ - int64_t next_zero; - BlockCopyTask *task = g_new(BlockCopyTask, 1); - - assert(bdrv_dirty_bitmap_get(s->copy_bitmap, offset)); - - bytes = MIN(bytes, s->copy_size); - next_zero = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, offset, bytes); - if (next_zero >= 0) { - assert(next_zero > offset); /* offset is dirty */ - assert(next_zero < offset + bytes); /* no need to do MIN() */ - bytes = next_zero - offset; - } - - /* region is dirty, so no existent tasks possible in it */ - assert(!find_conflicting_task(s, offset, bytes)); - - bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, bytes); - s->in_flight_bytes += bytes; - - *task = (BlockCopyTask) { - .s = s, - .offset = offset, - .bytes = bytes, - }; - qemu_co_queue_init(&task->wait_queue); - QLIST_INSERT_HEAD(&s->tasks, task, list); - - return task; -} - /* * block_copy_task_shrink * @@ -361,6 +328,39 @@ out: return ret; } +static BlockCopyTask *block_copy_task_create(BlockCopyState *s, + int64_t offset, int64_t bytes) +{ + int64_t next_zero; + BlockCopyTask *task = g_new(BlockCopyTask, 1); + + assert(bdrv_dirty_bitmap_get(s->copy_bitmap, offset)); + + bytes = MIN(bytes, s->copy_size); + next_zero = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, offset, bytes); + if (next_zero >= 0) { + assert(next_zero > offset); /* offset is dirty */ + assert(next_zero < offset + bytes); /* no need to do MIN() */ + bytes = next_zero - offset; + } + + /* region is dirty, so no existent tasks possible in it */ + assert(!find_conflicting_task(s, offset, bytes)); + + bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, bytes); + s->in_flight_bytes += bytes; + + *task = (BlockCopyTask) { + .s = s, + .offset = offset, + .bytes = bytes, + }; + qemu_co_queue_init(&task->wait_queue); + QLIST_INSERT_HEAD(&s->tasks, task, list); + + return task; +} + static int block_copy_block_status(BlockCopyState *s, int64_t offset, int64_t bytes, int64_t *pnum) {
Simple movement without any change. It's needed for the following patch, as this function will need to use some staff which is currently below it. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- block/block-copy.c | 66 +++++++++++++++++++++++----------------------- 1 file changed, 33 insertions(+), 33 deletions(-)