| Message ID | 20220423155619.3669555-1-void@manifault.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix bugs in memcontroller cgroup tests | expand |
Hello. On Sat, Apr 23, 2022 at 08:56:15AM -0700, David Vernet <void@manifault.com> wrote: > tools/testing/selftests/cgroup/test_memcontrol.c contains a set of > testcases which validate expected behavior of the cgroup memory controller. > Roman Gushchin recently sent out a patchset that fixed a few issues in the > test. Link here https://lore.kernel.org/r/20220415000133.3955987-1-roman.gushchin@linux.dev/. > This patchset continues that effort by fixing a few more issues that > were causing non-deterministic failures in the suite. Are the Roman's patches merged anywhere? (I ran into some issues when I was rebasing your (David's) series on top of master.) I'd like to put all sensible patches in one series or stack on existing branch (if there's any). For possible v3 of this series, I did: - dropped the patch that allows non-zero memory.events:low for a sibling with memory.low=0 when mounted with memory_recursiveprot (the case needs more discussion), - added few more cleanups, convenience for debugging, - fixed comments in the first patch. Pushed here [1] before properly sending the v3 for discussion. Thanks, Michal [1] https://github.com/Werkov/linux/commits/cgroup-ml/memory.low-overreclaim-var2
Hi Michal, On Thu, May 12, 2022 at 07:04:10PM +0200, Michal Koutný wrote: > Are the Roman's patches merged anywhere? (I ran into some issues when I > was rebasing your (David's) series on top of master.) I'd like to put > all sensible patches in one series or stack on existing branch (if > there's any). Roman's patches are present on master on the linux-mm tree. See b7dbfd6553d..a131b1ed12c6. > For possible v3 of this series, I did: > - dropped the patch that allows non-zero memory.events:low for a sibling with > memory.low=0 when mounted with memory_recursiveprot (the case needs more > discussion), Ack, and thanks for keeping us steered in the right direction here. I don't see this in the patch set you linked, but I agree this commit should be reverted and the reclaim logic instead fixed. > - added few more cleanups, convenience for debugging, Are you referring to the FAIL() macro you added? I would love to Ack that, but unfortunately checkpatch.pl will probably yell at you for having a goto in that macro, per the point about avoiding macros that affect control flow [0]. I tried to do the same thing when sending out my patch set and had to revert it before sending it to upstream. Thanks, David [0] https://github.com/Werkov/linux/commit/a076339cc4825af2f22f58c1347a572b104b8221
On Thu, May 12, 2022 at 10:30:18AM -0700, David Vernet wrote: > Hi Michal, > > On Thu, May 12, 2022 at 07:04:10PM +0200, Michal Koutný wrote: > > Are the Roman's patches merged anywhere? (I ran into some issues when I > > was rebasing your (David's) series on top of master.) I'd like to put > > all sensible patches in one series or stack on existing branch (if > > there's any). > > Roman's patches are present on master on the linux-mm tree. See > b7dbfd6553d..a131b1ed12c6. > > > For possible v3 of this series, I did: > > - dropped the patch that allows non-zero memory.events:low for a sibling with > > memory.low=0 when mounted with memory_recursiveprot (the case needs more > > discussion), > > Ack, and thanks for keeping us steered in the right direction here. I don't > see this in the patch set you linked, but I agree this commit should be > reverted and the reclaim logic instead fixed. > > > - added few more cleanups, convenience for debugging, > > Are you referring to the FAIL() macro you added? I would love to Ack that, > but unfortunately checkpatch.pl will probably yell at you for having a goto > in that macro, per the point about avoiding macros that affect control flow > [0]. > > I tried to do the same thing when sending out my patch set and had to > revert it before sending it to upstream. > > Thanks, > David > > [0] https://github.com/Werkov/linux/commit/a076339cc4825af2f22f58c1347a572b104b8221 Sorry, I meant to link this: https://www.kernel.org/doc/html/latest/process/coding-style.html#macros-enums-and-rtl
tools/testing/selftests/cgroup/test_memcontrol.c contains a set of testcases which validate expected behavior of the cgroup memory controller. Roman Gushchin recently sent out a patchset that fixed a few issues in the test. This patchset continues that effort by fixing a few more issues that were causing non-deterministic failures in the suite. With this patchset, I'm unable to reproduce any more errors after running the tests in a continuous loop for many iterations. Before, I was able to reproduce at least one of the errors fixed in this patchset with just one or two runs. Changelog: v2: - Fixed the comment headers in test_memcg_min() and test_memcg_low() to reflect the new ordering of child cgroups in those tests. - Fixed the comment I added in test_memcg_oom_group_leaf_events() to use /* */ for multiline comments, as is the norm according to the kernel style guide. - Changed some of the conditional logic in test_memcg_oom_group_leaf_events() that checks for OOM event counts based on memory_localevents to be more intuitive. David Vernet (5): cgroups: Refactor children cgroups in memcg tests cgroup: Account for memory_recursiveprot in test_memcg_low() cgroup: Account for memory_localevents in test_memcg_oom_group_leaf_events() cgroup: Removing racy check in test_memcg_sock() cgroup: Fix racy check in alloc_pagecache_max_30M() helper function tools/testing/selftests/cgroup/cgroup_util.c | 12 +++ tools/testing/selftests/cgroup/cgroup_util.h | 1 + .../selftests/cgroup/test_memcontrol.c | 77 ++++++++++++------- 3 files changed, 64 insertions(+), 26 deletions(-)