linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: David Finkel <davidf@vimeo.com>
To: "Michal Koutný" <mkoutny@suse.com>
Cc: Muchun Song <muchun.song@linux.dev>, Tejun Heo <tj@kernel.org>,
	 Roman Gushchin <roman.gushchin@linux.dev>,
	Andrew Morton <akpm@linux-foundation.org>,
	 core-services@vimeo.com, Jonathan Corbet <corbet@lwn.net>,
	Michal Hocko <mhocko@kernel.org>,
	 Shakeel Butt <shakeel.butt@linux.dev>,
	Shuah Khan <shuah@kernel.org>,
	 Johannes Weiner <hannes@cmpxchg.org>,
	Zefan Li <lizefan.x@bytedance.com>,
	cgroups@vger.kernel.org,  linux-doc@vger.kernel.org,
	linux-mm@kvack.org,  linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v6 2/2] mm, memcg: cg2 memory{.swap,}.peak write tests
Date: Tue, 30 Jul 2024 11:52:19 -0400	[thread overview]
Message-ID: <CAFUnj5O7+kH1bZRs9=AB6hx8hjkygwmGRe3khSK0mFLZc=yehg@mail.gmail.com> (raw)
In-Reply-To: <arkcd6cjf42zq62maqsbjzvimxwozrkukusgxhd54v6eyd6ylq@aurn3mek6hr2>

Thanks for checking!

On Tue, Jul 30, 2024 at 11:46 AM Michal Koutný <mkoutny@suse.com> wrote:
>
> Hello.
>
> On Mon, Jul 29, 2024 at 10:37:43AM GMT, David Finkel <davidf@vimeo.com> wrote:
> > Extend two existing tests to cover extracting memory usage through the
> > newly mutable memory.peak and memory.swap.peak handlers.
>
> BTW do the tests pass for you?

Yeah, my tests pass when running on top of an ext2 mount.
At least one of the existing tests failed when running out of tmpfs,
so I've been testing with an ext2 mount in UML.
>
> I gave it a try (v6.11-rc1+your patches)
>
> $ grep "not ok 2" -B30 test.strace
>
> ...
> 315   15:19:13.990351 read(6, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 4096) = 4096
> 315   15:19:13.994457 read(6, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 4096) = 4096
> 315   15:19:13.998562 read(6, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 4096) = 4096
> 315   15:19:13.998652 read(6, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 4096) = 4096
> 315   15:19:14.002759 read(6, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 4096) = 4096
> 315   15:19:14.006864 openat(AT_FDCWD, "/sys/fs/cgroup/memcg_test/memory.current", O_RDONLY) = 7
> 315   15:19:14.006989 read(7, "270336\n", 127) = 7
> 315   15:19:14.011114 close(7)          = 0
> 315   15:19:14.015262 close(6)          = 0
> 315   15:19:14.015448 exit_group(-1)    = ?
> 315   15:19:14.019753 +++ exited with 255 +++
> 313   15:19:14.019820 <... wait4 resumed>[{WIFEXITED(s) && WEXITSTATUS(s) == 255}], 0, NULL) = 315
> 313   15:19:14.019878 --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=315, si_uid=0, si_status=255, si_utime=1 /* 0.01 s */, -
> 313   15:19:14.019926 close(3)          = 0
> 313   15:19:14.020001 close(5)          = 0
> 313   15:19:14.020072 close(4)          = 0
> 313   15:19:14.024173 rmdir("/sys/fs/cgroup/memcg_test") = 0
> 313   15:19:14.028517 write(1, "not ok 2 test_memcg_current_peak"..., 33) = 33
>
> grep "^315 .*read.*4096" -c test.strace
> 12800
>
> Hopefully, unrelated to your changes. I ran this within initrd (rapido
> image) so it may be an issue how rootfs pagecache is undercharged (due
> to sharing?), instead of 50M, there's only ~256k.
>
> To verify, I also tried with memory.peak patch reverted, failing
> differently:
>
> ...
> 238   15:30:29.034623 openat(AT_FDCWD, "/sys/fs/cgroup/memcg_test/memory.current", O_RDONLY) = 3
> 238   15:30:29.034766 read(3, "52801536\n", 127) = 9
> 238   15:30:29.038895 close(3)          = 0
> 238   15:30:29.043048 openat(AT_FDCWD, "/sys/fs/cgroup/memcg_test/memory.stat", O_RDONLY) = 3
> 238   15:30:29.043230 read(3, "anon 52436992\nfile 0\nkernel 1105"..., 4095) = 870
> 238   15:30:29.047379 close(3)          = 0
> 238   15:30:29.051491 munmap(0x7f2473600000, 52432896) = 0
> 238   15:30:29.058516 exit_group(0)     = ?
> 238   15:30:29.062992 +++ exited with 0 +++
> 237   15:30:29.067054 <... wait4 resumed>[{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 238
> 237   15:30:29.067136 --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=238, si_uid=0, si_status=0, si_utime=1 /* 0.01 s */, si-
> 237   15:30:29.067210 openat(AT_FDCWD, "/sys/fs/cgroup/memcg_test/memory.peak", O_RDONLY) = 3
> 237   15:30:29.071349 read(3, "52805632\n", 127) = 9
> 237   15:30:29.075470 close(3)          = 0
> 237   15:30:29.075562 openat(AT_FDCWD, "/sys/fs/cgroup/memcg_test/memory.peak", O_RDWR|O_APPEND|O_CLOEXEC) = 3
> 237   15:30:29.079712 openat(AT_FDCWD, "/sys/fs/cgroup/memcg_test/memory.peak", O_RDWR|O_APPEND|O_CLOEXEC) = 4
> 237   15:30:29.083848 openat(AT_FDCWD, "/sys/fs/cgroup/memcg_test/memory.peak", O_RDWR|O_APPEND|O_CLOEXEC) = 5
> 237   15:30:29.083970 write(3, "reset\n\0", 7) = -1 EINVAL (Invalid argument)
> 237   15:30:29.088095 close(3)          = 0
> 237   15:30:29.092209 close(4)          = 0
> 237   15:30:29.092295 close(5)          = 0
> 237   15:30:29.096398 close(-1)         = -1 EBADF (Bad file descriptor)
> 237   15:30:29.100497 rmdir("/sys/fs/cgroup/memcg_test") = 0
> 237   15:30:29.100760 write(1, "not ok 2 test_memcg_current_peak"..., 33) = 33
>
> This failure makes sense but it reminded me --
> could you please modify the test so that it checks write permission
> of memory.peak and skips the reset testing on old(er) kernels? That'd be
> in accordance with other cgroup selftest that maintain partial backwards
> compatibility when possible.

Sure, I'll add that to the tests in a bit.
>
> Thanks,
> Michal



-- 
David Finkel
Senior Principal Software Engineer, Core Services


      reply	other threads:[~2024-07-30 15:52 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-29 14:37 [PATCH v6] mm, memcg: cg2 memory{.swap,}.peak write handlers David Finkel
2024-07-29 14:37 ` [PATCH v6 1/2] " David Finkel
2024-07-30  2:38   ` Johannes Weiner
2024-07-30 15:48   ` Michal Koutný
2024-07-29 14:37 ` [PATCH v6 2/2] mm, memcg: cg2 memory{.swap,}.peak write tests David Finkel
2024-07-30 15:46   ` Michal Koutný
2024-07-30 15:52     ` David Finkel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAFUnj5O7+kH1bZRs9=AB6hx8hjkygwmGRe3khSK0mFLZc=yehg@mail.gmail.com' \
    --to=davidf@vimeo.com \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=corbet@lwn.net \
    --cc=core-services@vimeo.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lizefan.x@bytedance.com \
    --cc=mhocko@kernel.org \
    --cc=mkoutny@suse.com \
    --cc=muchun.song@linux.dev \
    --cc=roman.gushchin@linux.dev \
    --cc=shakeel.butt@linux.dev \
    --cc=shuah@kernel.org \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox