On Wed, Nov 18, 2020 at 10:05:59AM +0100, Andrew Jones wrote: > On Wed, Nov 18, 2020 at 09:38:31AM +0100, Andrew Jones wrote: > > On Mon, Nov 16, 2020 at 01:40:11PM -0500, Peter Xu wrote: > > > On Mon, Nov 16, 2020 at 07:16:50PM +0100, Paolo Bonzini wrote: > > > > On 16/11/20 13:19, Andrew Jones wrote: > > > > > This series attempts to clean up demand_paging_test, dirty_log_perf_test, > > > > > and dirty_log_test by factoring out common code, creating some new API > > > > > along the way. It also splits include/perf_test_util.h into a more > > > > > conventional header and source pair. > > > > > > > > > > I've tested on x86 and AArch64 (one config each), but not s390x. > > > > > > > > > > v3: > > > > > - Rebased remaining four patches from v2 onto kvm/queue > > > > > - Picked up r-b's from Peter and Ben > > > > > > > > > > v2: https://www.spinics.net/lists/kvm/msg228711.html > > > > > > > > Unfortunately patch 2 is still broken: > > > > > > > > $ ./dirty_log_test -M dirty-ring > > > > Setting log mode to: 'dirty-ring' > > > > Test iterations: 32, interval: 10 (ms) > > > > Testing guest mode: PA-bits:ANY, VA-bits:48, 4K pages > > > > ==== Test Assertion Failure ==== > > > > lib/kvm_util.c:85: ret == 0 > > > > pid=2010122 tid=2010122 - Invalid argument > > > > 1 0x0000000000402ee7: vm_enable_cap at kvm_util.c:84 > > > > 2 0x0000000000403004: vm_enable_dirty_ring at kvm_util.c:124 > > > > 3 0x00000000004021a5: log_mode_create_vm_done at dirty_log_test.c:453 > > > > 4 (inlined by) run_test at dirty_log_test.c:683 > > > > 5 0x000000000040b643: for_each_guest_mode at guest_modes.c:37 > > > > 6 0x00000000004019c2: main at dirty_log_test.c:864 > > > > 7 0x00007fe3f48207b2: ?? ??:0 > > > > 8 0x0000000000401aad: _start at ??:? > > > > KVM_ENABLE_CAP IOCTL failed, > > > > rc: -1 errno: 22 > > > > > > > > (Also fails without -M). > > > > > > It should be because of the ordering of creating vcpu and enabling dirty rings, > > > since currently for simplicity when enabling dirty ring we must have not > > > created any vcpus: > > > > > > + if (kvm->created_vcpus) { > > > + /* We don't allow to change this value after vcpu created */ > > > + r = -EINVAL; > > > + } else { > > > + kvm->dirty_ring_size = size; > > > + r = 0; > > > + } > > > > > > We may need to call log_mode_create_vm_done() before creating any vcpus > > > somehow. Sorry to not have noticed that when reviewing it. > > > > > > > And sorry for having not tested with '-M dirty-ring'. I thought we were > > trying to ensure each unique test type had its own test file (even if we > > have to do the weird inclusion of C files). Doing that, the command line > > options are then only used to change stuff like verbosity or to experiment > > with tweaked configurations. > > > > If we're not doing that, then I think we should. We don't want to try and > > explain to all the CI people how each test should be run. It's much easier > > to say "run all the binaries, no parameters necessary". Each binary with > > no parameters should run the test(s) using a good default or by executing > > all possible configurations. > > > > I just double checked and we are running all modes by default. This is > the output I get > > Test iterations: 32, interval: 10 (ms) > Testing Log Mode 'dirty-log' > Testing guest mode: PA-bits:ANY, VA-bits:48, 4K pages > guest physical test memory offset: 0x7fbfffc000 > Dirtied 1024 pages > Total bits checked: dirty (209373), clear (7917184), track_next (38548) > Testing Log Mode 'clear-log' > Testing guest mode: PA-bits:ANY, VA-bits:48, 4K pages > guest physical test memory offset: 0x7fbfffc000 > Dirtied 1024 pages > Total bits checked: dirty (464547), clear (7662010), track_next (37553) > Testing Log Mode 'dirty-ring' > Log mode 'dirty-ring' not supported, skipping test > > which matches the output before this patch, except for minor differences > in the numbers. > > I'm not sure how this is failing in your environment and not mine. > Doh, sorry. I didn't actually read the output. I was only looking for an assert and the string 'dirty-ring'. It looks like the test is skipping for me. That would explain why it "works" for me... drew
