Re: [Qemu-block] [Qemu-devel] [PATCH v2 00/13] blockjob: refactor mirror_throttle
ping; I won't respin for patchew until this gets looked over again, sorry :) On 01/19/2018 03:58 PM, John Snow wrote: > mirror_throttle attempts to make sure we yield every so often when we're > doing operations that may not otherwise be as cooperative as we'd like. > > This pattern is useful to other jobs like commit as well; if commit > continuously decides not to copy data (and calculates delay_ns to be 0), > we'll frequently and aggressively yield, only to be rescheduled immediately. > > This causes those "WARNING: I/O thread spun for 1000 iterations" > warnings that everyone loves so much. > > Split out the mirror_throttle function to become a new internal > BlockJob API function, block_job_throttle; then use it for all > the other jobs in their runtime loops. > > I decided to use it in stream and backup as well, so that regardless of if the > loop structure has the chance to be greedy or not (I did not audit this), the > BlockJob has some kind of failsafe that will occasionally perform a 0ns sleep > every SLICE_TIME. > > v2: > - Fixed spacing consistency (Paolo) > - Renamed last_yield_ns to last_enter_ns (Stefan) > - Renamed block_job_throttle to block_job_relax (Stefan) > - Removed external usages of block_job_pause_point and >block_job_sleep_ns (Paolo) > - Further cut down block/backup code to use block_job_relax > > > > For convenience, this branch is available at: > https://github.com/jnsnow/qemu.git branch block-job-yield > https://github.com/jnsnow/qemu/tree/block-job-yield > > This version is tagged block-job-yield-v2: > https://github.com/jnsnow/qemu/releases/tag/block-job-yield-v2 > > John Snow (13): > blockjob: record time of last entrance > blockjob: consolidate SLICE_TIME definition > blockjob: create block_job_relax > blockjob: allow block_job_throttle to take delay_ns > block/commit: use block_job_relax > block/stream: use block_job_relax > block/backup: use block_job_relax > allow block_job_relax to return -ECANCELED > block/backup: remove yield_and_check > block/mirror: condense cancellation and relax calls > block/mirror: remove block_job_sleep_ns calls > blockjob: privatize block_job_sleep_ns > blockjob: remove block_job_pause_point from interface > > block/backup.c | 27 ++- > block/commit.c | 4 +--- > block/mirror.c | 52 > +++- > block/stream.c | 4 +--- > block/trace-events | 2 +- > blockjob.c | 51 --- > include/block/blockjob.h | 5 + > include/block/blockjob_int.h | 37 +++ > tests/test-bdrv-drain.c | 2 +- > tests/test-blockjob-txn.c| 2 +- > 10 files changed, 94 insertions(+), 92 deletions(-) >
Re: [Qemu-block] [Qemu-devel] [PATCH v2 00/13] blockjob: refactor mirror_throttle
Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20180119205847.7141-1-js...@redhat.com Subject: [Qemu-devel] [PATCH v2 00/13] blockjob: refactor mirror_throttle === TEST SCRIPT BEGIN === #!/bin/bash BASE=base n=1 total=$(git log --oneline $BASE.. | wc -l) failed=0 git config --local diff.renamelimit 0 git config --local diff.renames True commits="$(git log --format=%H --reverse $BASE..)" for c in $commits; do echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..." if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then failed=1 echo fi n=$((n+1)) done exit $failed === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu t [tag update] patchew/20180119162554.27270-1-marcandre.lur...@redhat.com -> patchew/20180119162554.27270-1-marcandre.lur...@redhat.com * [new tag] patchew/20180119205847.7141-1-js...@redhat.com -> patchew/20180119205847.7141-1-js...@redhat.com Switched to a new branch 'test' 969dddc4fd blockjob: remove block_job_pause_point from interface 792539a07e blockjob: privatize block_job_sleep_ns ea6f8f2359 block/mirror: remove block_job_sleep_ns calls 7f2ee60c8b block/mirror: condense cancellation and relax calls 9a16d6778e block/backup: remove yield_and_check bbfcfe9c89 allow block_job_relax to return -ECANCELED b1324b111c block/backup: use block_job_relax d208db6fca block/stream: use block_job_relax 788ece436a block/commit: use block_job_relax 4e209e7aed blockjob: allow block_job_throttle to take delay_ns 76ce42710b blockjob: create block_job_relax 9771e84ab8 blockjob: consolidate SLICE_TIME definition 2a1eec68ac blockjob: record time of last entrance === OUTPUT BEGIN === Checking PATCH 1/13: blockjob: record time of last entrance... WARNING: line over 80 characters #52: FILE: block/mirror.c:803: +delta = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - s->common.last_enter_ns; total: 0 errors, 1 warnings, 63 lines checked Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. Checking PATCH 2/13: blockjob: consolidate SLICE_TIME definition... Checking PATCH 3/13: blockjob: create block_job_relax... Checking PATCH 4/13: blockjob: allow block_job_throttle to take delay_ns... Checking PATCH 5/13: block/commit: use block_job_relax... Checking PATCH 6/13: block/stream: use block_job_relax... Checking PATCH 7/13: block/backup: use block_job_relax... Checking PATCH 8/13: allow block_job_relax to return -ECANCELED... Checking PATCH 9/13: block/backup: remove yield_and_check... Checking PATCH 10/13: block/mirror: condense cancellation and relax calls... Checking PATCH 11/13: block/mirror: remove block_job_sleep_ns calls... ERROR: do not initialise statics to 0 or NULL #30: FILE: block/mirror.c:764: +static uint64_t delay_ns = 0; total: 1 errors, 0 warnings, 50 lines checked Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. Checking PATCH 12/13: blockjob: privatize block_job_sleep_ns... Checking PATCH 13/13: blockjob: remove block_job_pause_point from interface... === OUTPUT END === Test command exited with code: 1 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@freelists.org
