| Message ID | 20220725163539.3145690-1-dmatlack@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | KVM: selftests: Rename perf_test_util to memstress | expand |
On Mon, Jul 25, 2022 at 04:35:37PM +0000, David Matlack wrote: > This series renames the perf_test_util to memstress. patch 1 renames the files > perf_test_util.[ch] to memstress.[ch], and patch 2 replaces the perf_test_ > prefix on symbols with memstress_. > > The reason for this rename, as with any rename, is to improve readability. > perf_test_util is too generic and does not describe at all what the library > does, other than being used for perf tests. > > I considered a lot of different names (naming is hard) and eventually settled > on memstress for a few reasons: > > - "memstress" better describes the functionality proveded by this library, > which is to run a VM that reads/writes to memory from all vCPUs in parallel > (i.e. stressing VM memory). > > - "memstress" contains the same number of characters as "perf_test", making > it a drop in replacement in symbols wihout changing line lengths. > > - The lack of underscore between "mem" and "stress" makes it clear "memstress" > is a noun, avoiding confusion in function names. Naming is hard, acking rename patches that have good justifications is easy. For the series, Acked-by: Andrew Jones <andrew.jones@linux.dev>
On Mon, Jul 25, 2022, David Matlack wrote: > This series renames the perf_test_util to memstress. patch 1 renames the files > perf_test_util.[ch] to memstress.[ch], and patch 2 replaces the perf_test_ > prefix on symbols with memstress_. > > The reason for this rename, as with any rename, is to improve readability. > perf_test_util is too generic and does not describe at all what the library > does, other than being used for perf tests. > > I considered a lot of different names (naming is hard) and eventually settled > on memstress for a few reasons: > > - "memstress" better describes the functionality proveded by this library, > which is to run a VM that reads/writes to memory from all vCPUs in parallel > (i.e. stressing VM memory). Hmm, but the purpose of the library isn't to stress VM memory so much as it is to stress KVM's MMU. And typically "stress" tests just hammer a resource to try and make it fail, whereas measuring performance is one of the main In other words, IMO it would be nice to keep "perf" in there somehwere. Maybe mmu_perf or something along those lines? I wouldn't worry too much about changing the number of chars, the churn wouldn't be thaaat bad.
On Mon, Jul 25, 2022 at 3:45 PM Sean Christopherson <seanjc@google.com> wrote: > > On Mon, Jul 25, 2022, David Matlack wrote: > > This series renames the perf_test_util to memstress. patch 1 renames the files > > perf_test_util.[ch] to memstress.[ch], and patch 2 replaces the perf_test_ > > prefix on symbols with memstress_. > > > > The reason for this rename, as with any rename, is to improve readability. > > perf_test_util is too generic and does not describe at all what the library > > does, other than being used for perf tests. > > > > I considered a lot of different names (naming is hard) and eventually settled > > on memstress for a few reasons: > > > > - "memstress" better describes the functionality proveded by this library, > > which is to run a VM that reads/writes to memory from all vCPUs in parallel > > (i.e. stressing VM memory). > > Hmm, but the purpose of the library isn't to stress VM memory so much as it is to > stress KVM's MMU. And typically "stress" tests just hammer a resource to try and > make it fail, whereas measuring performance is one of the main > > In other words, IMO it would be nice to keep "perf" in there somehwere. The reasons I leaned toward "stress" rather than "perf" is that this library itself does not measure performance (it's just a workload) and it's not always used for performance tests (e.g. memslot_modification_stress_test.c). > > Maybe mmu_perf or something along those lines? How about "memperf"? "mmu_perf" makes me think it'd be explicitly measuring the performance of MMU operations. Another contender was "memstorm", but I thought it might be too cute. > I wouldn't worry too much about > changing the number of chars, the churn wouldn't be thaaat bad. Heh. The line lengths were getting long when I played with "memory_stress" :). But yeah it's not really that much churn.
On Tue, Jul 26, 2022, David Matlack wrote: > On Mon, Jul 25, 2022 at 3:45 PM Sean Christopherson <seanjc@google.com> wrote: > > > > On Mon, Jul 25, 2022, David Matlack wrote: > > > This series renames the perf_test_util to memstress. patch 1 renames the files > > > perf_test_util.[ch] to memstress.[ch], and patch 2 replaces the perf_test_ > > > prefix on symbols with memstress_. > > > > > > The reason for this rename, as with any rename, is to improve readability. > > > perf_test_util is too generic and does not describe at all what the library > > > does, other than being used for perf tests. > > > > > > I considered a lot of different names (naming is hard) and eventually settled > > > on memstress for a few reasons: > > > > > > - "memstress" better describes the functionality proveded by this library, > > > which is to run a VM that reads/writes to memory from all vCPUs in parallel > > > (i.e. stressing VM memory). > > > > Hmm, but the purpose of the library isn't to stress VM memory so much as it is to > > stress KVM's MMU. And typically "stress" tests just hammer a resource to try and > > make it fail, whereas measuring performance is one of the main > > > > In other words, IMO it would be nice to keep "perf" in there somehwere. > > The reasons I leaned toward "stress" rather than "perf" is that this > library itself does not measure performance (it's just a workload) and > it's not always used for performance tests (e.g. > memslot_modification_stress_test.c). Yeah, I don't disagree on any point, it's purely that memstress makes me think of memtest (the pre-boot test that blasts memory with patterns to detect bad DRAM). > > Maybe mmu_perf or something along those lines? > > How about "memperf"? "mmu_perf" makes me think it'd be explicitly > measuring the performance of MMU operations. Let's go with your original "memstress". I like how it looks in code, and once I get past the initial "memtest" association, it's a good fit. > Another contender was "memstorm", but I thought it might be too cute. Heh. mem_minions? Then we can have "mm_args" and really confuse everyone.
This series renames the perf_test_util to memstress. patch 1 renames the files perf_test_util.[ch] to memstress.[ch], and patch 2 replaces the perf_test_ prefix on symbols with memstress_. The reason for this rename, as with any rename, is to improve readability. perf_test_util is too generic and does not describe at all what the library does, other than being used for perf tests. I considered a lot of different names (naming is hard) and eventually settled on memstress for a few reasons: - "memstress" better describes the functionality proveded by this library, which is to run a VM that reads/writes to memory from all vCPUs in parallel (i.e. stressing VM memory). - "memstress" contains the same number of characters as "perf_test", making it a drop in replacement in symbols wihout changing line lengths. - The lack of underscore between "mem" and "stress" makes it clear "memstress" is a noun, avoiding confusion in function names. Looking to the future, I think "memstress" will remain a good name. Specifically there are some in-flight improvements that will make this library even more of a "memory stress tester": - A proposed series by yours truly [1] extends memstress/perf_test_util to support execute from memory, in addition to reading/writing. - Colton Lewis within Google is looking into adding support for more complex memory access patterns. [1] https://lore.kernel.org/kvm/20220401233737.3021889-2-dmatlack@google.com/ David Matlack (2): KVM: selftests: Rename perf_test_util.[ch] to memstress.[ch] KVM: selftests: Rename perf_test symbols to memstress tools/testing/selftests/kvm/Makefile | 4 +- .../selftests/kvm/access_tracking_perf_test.c | 20 +++---- .../selftests/kvm/demand_paging_test.c | 20 +++---- .../selftests/kvm/dirty_log_perf_test.c | 22 ++++---- .../include/{perf_test_util.h => memstress.h} | 34 ++++++------ .../kvm/lib/{perf_test_util.c => memstress.c} | 54 +++++++++---------- .../x86_64/{perf_test_util.c => memstress.c} | 36 ++++++------- .../kvm/memslot_modification_stress_test.c | 16 +++--- 8 files changed, 103 insertions(+), 103 deletions(-) rename tools/testing/selftests/kvm/include/{perf_test_util.h => memstress.h} (50%) rename tools/testing/selftests/kvm/lib/{perf_test_util.c => memstress.c} (83%) rename tools/testing/selftests/kvm/lib/x86_64/{perf_test_util.c => memstress.c} (68%) base-commit: 1a4d88a361af4f2e91861d632c6a1fe87a9665c2 prerequisite-patch-id: 8c230105c8a2f1245dedb5b386327d98865d0bb2 prerequisite-patch-id: 9b4329037e2e880db19f3221e47d956b78acadc8