On Thu, Nov 12, 2020 at 01:50:06PM -0500, Peter Xu wrote: > On Thu, Nov 12, 2020 at 10:34:11AM -0800, Ben Gardon wrote: > > I didn't review this patch closely enough, and assumed the clear dirty > > log was still being done because of > > afdb19600719 KVM: selftests: Use a single binary for dirty/clear log test > > > > Looking back now, I see that that is not the case. > > > > I'd like to retract my endorsement in that case. I'd prefer to leave > > the dead code in and I'll send another series to actually use it once > > this series is merged. I've already written the code to use it and > > time the clearing, so it seems a pity to remove it now. > > > > Alternatively I could just revert this commit in that future series, > > though I suspect not removing the dead code would reduce the chances > > of merge conflicts. Either way works. > > > > I can extend the dirty log mode functions from dirty_log_test for > > dirty_log_perf_test in that series too. > > Or... we can just remove all the "#ifdef" lines but assuming clear dirty log is > always there? :) Assuming that is still acceptable as long as the test is > matching latest kernel which definitely has the clear dirty log capability. > > It's kind of weird to test get-dirty-log perf without clear dirty log, since > again if anyone really cares about the perf of that, then imho they should > first switch to a new kernel with clear dirty log, rather than measuring the > world without clear dirty log.. > I have no opinion on this other than I'd prefer KVM selftests to not have deadcode. If it makes more sense to fix the deadcode by bringing it back to life than to drop the code, then please send patches to do that, at which point I'd be happy to recommend dropping this patch. Thanks, drew
