On Mon, Aug 20, 2018 at 11:46:10AM +0200, Paolo Bonzini wrote: > On 20/08/2018 09:54, Peter Xu wrote: > > +/* Size of the testing memory slot */ > > +#define TEST_MEM_PAGES (1ULL << 18) /* 1G for 4K pages */ > > +/* How many pages to dirty for each guest loop */ > > +#define TEST_PAGES_PER_LOOP 1024 > > +/* How many host loops to run (one KVM_GET_DIRTY_LOG for each loop) */ > > +#define TEST_HOST_LOOP_N 16 > > +/* Interval for each host loop (ms) */ > > +#define TEST_HOST_LOOP_INTERVAL 10 > > How long does the test run overall? Do you get coverage for the > set_bit(page, bmap_track) statement? It's okay to increase the values, > it's not a problem if the test runs for 10-20 seconds. I'm adding some statistics for the bits we tracked (dirtied, cleared, check_next_dirty) and dump it out before test quits. Currently with existing parameters I get: Total bits checked: dirty (131649), clear (3800511), track_next (43245) So that path is covered. Regarding to the test length, I'll use 1 second by default as we discussed offlist, and I'll parse argc/argv to allow someone to pass in some customized numbers. > > > + > > + /* > > + * The default size is (slightly) not enough to put the page > > + * tables for the dirty logging memory region. Extend it > > + * depending on how big a region we're testing upon > > + */ > > + mem_size = DEFAULT_GUEST_PHY_PAGES + TEST_MEM_PAGES / 512 * 2; > > Hmm, now I see why you need the argument; I thought you were just adding > more memory for the test pages there but you are (correctly) using a > separate memory slot. If it's about the page tables, maybe you could > pass TEST_MEM_PAGES to vm_create_default and then do the computation in > vm_create_default? (Therefore scrapping my suggestion in patch 4/5). This computation totally comes from the requirement for bigger page tables (e.g., 4K page contains 512 page entries on 64bit x86 hosts), I'm just afraid that people might need to expand that for other reasons. One thing I can think of is when the ELF is too big to be put into the VM memory due to some reason, then the VM memory size will depend on something else rather than the size of page tables. Considering that I would still slightly prefer to just allow the tests to pass in VM sizes (or as you suggested in patch 4/5). What do you think? Regards, -- Peter Xu