[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <tcz7mmykp7wi4h3cezhbh53wmsabzvd5shejae6vrku7haynzl@4te6hgxve3s2>
Date: Tue, 12 Nov 2024 13:14:48 -0500
From: Kent Overstreet <kent.overstreet@...ux.dev>
To: Hao Ge <hao.ge@...ux.dev>
Cc: Suren Baghdasaryan <surenb@...gle.com>, akpm@...ux-foundation.org,
linux-kernel@...r.kernel.org, linux-mm@...ck.org, Hao Ge <gehao@...inos.cn>
Subject: Re: [PATCH] lib/alloc_tag: Remove the sysctl configuration to
prevent users from disabling it at runtime
On Tue, Nov 12, 2024 at 11:30:39AM +0800, Hao Ge wrote:
> Hi Suren
>
>
> Firstly, please forgive me for my improper wording in the commit message.
>
> After sending it, I realized that I should have used "suggestion" instead of
> "decided".
>
> Secondly, please forgive me for taking a few days to respond. I've been
> quite busy these days.
>
>
> Let's continue to discuss this issue.
>
>
> On 11/9/24 02:16, Suren Baghdasaryan wrote:
> > On Thu, Nov 7, 2024 at 11:50 PM Hao Ge <hao.ge@...ux.dev> wrote:
> > > From: Hao Ge <gehao@...inos.cn>
> > >
> > > After much consideration,I have decided to remove
> > > the "mem_profiling" sysctl interface to prevent
> > > users from dynamically enabling or disabling the
> > > MEMORY ALLOCATION PROFILING feature at runtime.
> > >
> > > I have taken the following actions: I set
> > > CONFIG_MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT=y to
> > > enable memory allocation profiling by default,
> > > and then made adjustments to mem_profiling dynamically
> > > during runtime.
> > >
> > > When I ran the OOM test program, I obtained useful
> > > information that was indeed very helpful for debugging.
> > >
> > > [ 1023.065402] Memory allocations:
> > > [ 1023.065407] 12.8 GiB 6546 mm/huge_memory.c:1328 func:do_huge_pmd_anonymous_page
> > > [ 1023.065412] 873 MiB 229985 arch/arm64/mm/fault.c:986 func:vma_alloc_zeroed_movable_folio
> > > [ 1023.065415] 187 MiB 29732 mm/slub.c:2412 func:alloc_slab_page
> > > [ 1023.065418] 99.8 MiB 25560 mm/memory.c:1065 func:folio_prealloc
> > > [ 1023.065421] 47.2 MiB 3189 mm/readahead.c:434 func:ra_alloc_folio
> > > [ 1023.065424] 30.0 MiB 15 mm/khugepaged.c:1072 func:alloc_charge_folio
> > > [ 1023.065428] 28.6 MiB 514 mm/compaction.c:1880 func:compaction_alloc
> > > [ 1023.065430] 25.8 MiB 6592 mm/page_ext.c:271 func:alloc_page_ext
> > > [ 1023.065433] 25.6 MiB 6546 mm/huge_memory.c:1161 func:__do_huge_pmd_anonymous_page
> > > [ 1023.065436] 23.5 MiB 6017 mm/shmem.c:1771 func:shmem_alloc_folio
> > >
> > > After running echo 0 > /proc/sys/vm/mem_profiling
> > > and then executing the same test program,
> > > I obtained the following results
> > >
> > > [ 1156.509699] Memory allocations:
> > > [ 1156.509703] 187 MiB 29645 mm/slub.c:2412 func:alloc_slab_page
> > > [ 1156.509707] 142 MiB 9357 mm/readahead.c:434 func:ra_alloc_folio
> > > [ 1156.509710] 136 MiB 41325 arch/arm64/mm/fault.c:986 func:vma_alloc_zeroed_movable_folio
> > > [ 1156.509713] 99.7 MiB 25531 mm/memory.c:1065 func:folio_prealloc
> > > [ 1156.509716] 56.0 MiB 28 mm/huge_memory.c:1328 func:do_huge_pmd_anonymous_page
> > > [ 1156.509719] 30.0 MiB 15 mm/khugepaged.c:1072 func:alloc_charge_folio
> > > [ 1156.509723] 28.6 MiB 514 mm/compaction.c:1880 func:compaction_alloc
> > > [ 1156.509725] 26.3 MiB 7460 mm/readahead.c:264 func:page_cache_ra_unbounded
> > > [ 1156.509728] 25.8 MiB 6592 mm/page_ext.c:271 func:alloc_page_ext
> > > [ 1156.509730] 23.5 MiB 6016 mm/shmem.c:1771 func:shmem_alloc_folio
> > >
> > > Because mem_profiling was disabled by executing
> > > echo 0 > /proc/sys/vm/mem_profiling,we are unable to
> > > record memory allocation information after the disablement.
> > Naturally you are unable to track the allocations after disabling it.
> > You disabled it as root, so I assume you know what you are doing.
> >
> > > These output logs can mislead users. And similarly, the same
> > > applies to alloc_info.
> > I would understand if you made /proc/allocinfo empty after disabling
> > it to avoid confusing the user, but ripping out the ability to
> > enable/disable profiling at runtime does not make sense to me. Once
> > you collect required data, disabling profiling gets you back the
> > performance that you pay for it. There are usecases when a program on
> > a remote device periodically enables profiling for some time, records
> > the difference in allocations and then disables it. Your change breaks
> > such users.
>
>
> Actually, my original intention was also to make /proc/allocinfo empty when
> disabling it,
>
> but I considered the following scenario: after we disable it and clear
> /proc/allocinfo,
>
> we then start a memory-intensive application,
>
> such as our OOM (Out-Of-Memory) test program.
>
> If we later enable it again, the issue described in my commit message would
> still arise.
>
> Perhaps we need to further consider how to handle this situation.
Why would you do such a thing?
We put a lot of effort into making memory allocation profiling cheap
enough to leave on, and I haven't seen a single complaint about
performance overhead.
Powered by blists - more mailing lists