On Tue, Nov 03, 2020 at 02:17:53PM -0800, Ben Gardon wrote:
On Mon, Nov 2, 2020 at 5:12 PM Peter Xu peterx@redhat.com wrote:
On Mon, Nov 02, 2020 at 03:56:05PM -0800, Ben Gardon wrote:
On Mon, Nov 2, 2020 at 2:21 PM Peter Xu peterx@redhat.com wrote:
On Tue, Oct 27, 2020 at 04:37:33PM -0700, Ben Gardon wrote:
The dirty log perf test will time verious dirty logging operations (enabling dirty logging, dirtying memory, getting the dirty log, clearing the dirty log, and disabling dirty logging) in order to quantify dirty logging performance. This test can be used to inform future performance improvements to KVM's dirty logging infrastructure.
One thing to mention is that there're a few patches in the kvm dirty ring series that reworked the dirty log test quite a bit (to add similar test for dirty ring). For example:
https://lore.kernel.org/kvm/20201023183358.50607-11-peterx@redhat.com/
Just a FYI if we're going to use separate test programs. Merging this tests should benefit in many ways, of course (e.g., dirty ring may directly runnable with the perf tests too; so we can manually enable this "perf mode" as a new parameter in dirty_log_test, if possible?), however I don't know how hard - maybe there's some good reason to keep them separate...
Absolutely, we definitely need a performance test for both modes. I'll take a look at the patch you linked and see what it would take to support dirty ring in this test.
That would be highly appreciated.
Do you think that should be done in this series, or would it make sense to add as a follow up?
To me I slightly lean toward working upon those patches, since we should potentially share quite some code there (e.g., the clear dirty log cleanup seems necessary, or not easy to add the dirty ring tests anyway). But current one is still ok to me at least as initial version - we should always be more tolerant for test cases, aren't we? :)
So maybe we can wait for a 3rd opinion before you change the direction.
I took a look at your patches for dirty ring and dirty logging modes and thought about this some more. I think your patch to merge the get and clear dirty log tests is great, and I can try to include it and build on it in my series as well if desired. I don't think it would be hard to use the same mode approach in the dirty log perf test. That said, I think it would be easier to keep the functional test (dirty_log_test, clear_dirty_log_test) separate from the performance test because the dirty log validation is extra time and complexity not needed in the dirty log perf test. I did try building them in the same test initially, but it was really ugly. Perhaps a future refactoring could merge them better.
We can conditionally bypass the validation part. Let's keep it separate for now - which is totally fine by me. Actually I also don't want the dirty ring series to block your series since I still don't know when it'll land. That'll be unnecessary depencency. Thanks,