* [PATCH] mm/code_tag: Skip displaying the code_tag if it is not called @ 2024-12-11 8:56 Zhenhua Huang 2024-12-12 7:16 ` Suren Baghdasaryan 0 siblings, 1 reply; 5+ messages in thread From: Zhenhua Huang @ 2024-12-11 8:56 UTC (permalink / raw) To: akpm, surenb, kent.overstreet; +Cc: linux-kernel, linux-mm, Zhenhua Huang /proc/allocinfo is full of callsites which are not called at all. Let's only output if the callsite actually been invoked. Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com> --- lib/alloc_tag.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c index 35f7560a309a..06fb7eb5c0bc 100644 --- a/lib/alloc_tag.c +++ b/lib/alloc_tag.c @@ -95,10 +95,12 @@ static void alloc_tag_to_text(struct seq_buf *out, struct codetag *ct) struct alloc_tag_counters counter = alloc_tag_read(tag); s64 bytes = counter.bytes; - seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls); - codetag_to_text(out, ct); - seq_buf_putc(out, ' '); - seq_buf_putc(out, '\n'); + if (bytes || counter.calls) { + seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls); + codetag_to_text(out, ct); + seq_buf_putc(out, ' '); + seq_buf_putc(out, '\n'); + } } static int allocinfo_show(struct seq_file *m, void *arg) -- 2.25.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/code_tag: Skip displaying the code_tag if it is not called 2024-12-11 8:56 [PATCH] mm/code_tag: Skip displaying the code_tag if it is not called Zhenhua Huang @ 2024-12-12 7:16 ` Suren Baghdasaryan 2024-12-12 7:50 ` Zhenhua Huang 0 siblings, 1 reply; 5+ messages in thread From: Suren Baghdasaryan @ 2024-12-12 7:16 UTC (permalink / raw) To: Zhenhua Huang; +Cc: akpm, kent.overstreet, linux-kernel, linux-mm On Wed, Dec 11, 2024 at 12:56 AM Zhenhua Huang <quic_zhenhuah@quicinc.com> wrote: > > /proc/allocinfo is full of callsites which are not called at all. > Let's only output if the callsite actually been invoked. No, I disagree. Allocation that was never invoked is not the same as no allocation at all. How would we know the difference if we filter out the empty ones? If you don't want to see all the unused sites, you can filter them in the userspace. I also suspect that for practical purposes you would want to filter small ones (below some threshold) as well. > > Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com> > --- > lib/alloc_tag.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c > index 35f7560a309a..06fb7eb5c0bc 100644 > --- a/lib/alloc_tag.c > +++ b/lib/alloc_tag.c > @@ -95,10 +95,12 @@ static void alloc_tag_to_text(struct seq_buf *out, struct codetag *ct) > struct alloc_tag_counters counter = alloc_tag_read(tag); > s64 bytes = counter.bytes; > > - seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls); > - codetag_to_text(out, ct); > - seq_buf_putc(out, ' '); > - seq_buf_putc(out, '\n'); > + if (bytes || counter.calls) { > + seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls); > + codetag_to_text(out, ct); > + seq_buf_putc(out, ' '); > + seq_buf_putc(out, '\n'); > + } > } > > static int allocinfo_show(struct seq_file *m, void *arg) > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/code_tag: Skip displaying the code_tag if it is not called 2024-12-12 7:16 ` Suren Baghdasaryan @ 2024-12-12 7:50 ` Zhenhua Huang 2024-12-13 2:44 ` Suren Baghdasaryan 0 siblings, 1 reply; 5+ messages in thread From: Zhenhua Huang @ 2024-12-12 7:50 UTC (permalink / raw) To: Suren Baghdasaryan; +Cc: akpm, kent.overstreet, linux-kernel, linux-mm Hi Suren, On 2024/12/12 15:16, Suren Baghdasaryan wrote: > On Wed, Dec 11, 2024 at 12:56 AM Zhenhua Huang > <quic_zhenhuah@quicinc.com> wrote: >> >> /proc/allocinfo is full of callsites which are not called at all. >> Let's only output if the callsite actually been invoked. > > No, I disagree. Allocation that was never invoked is not the same as > no allocation at all. How would we know the difference if we filter But it doesn't affect further display when it is actually called? why we need to know the diff here... The point from me is: up to now, the callsite hasn't been invoked, so it can be ignored in the output.. The original output is really huge.. > out the empty ones? > If you don't want to see all the unused sites, you can filter them in > the userspace. I also suspect that for practical purposes you would > want to filter small ones (below some threshold) as well. Yeah, that's the expected way from us as well :) > >> >> Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com> >> --- >> lib/alloc_tag.c | 10 ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c >> index 35f7560a309a..06fb7eb5c0bc 100644 >> --- a/lib/alloc_tag.c >> +++ b/lib/alloc_tag.c >> @@ -95,10 +95,12 @@ static void alloc_tag_to_text(struct seq_buf *out, struct codetag *ct) >> struct alloc_tag_counters counter = alloc_tag_read(tag); >> s64 bytes = counter.bytes; >> >> - seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls); >> - codetag_to_text(out, ct); >> - seq_buf_putc(out, ' '); >> - seq_buf_putc(out, '\n'); >> + if (bytes || counter.calls) { >> + seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls); >> + codetag_to_text(out, ct); >> + seq_buf_putc(out, ' '); >> + seq_buf_putc(out, '\n'); >> + } >> } >> >> static int allocinfo_show(struct seq_file *m, void *arg) >> -- >> 2.25.1 >> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/code_tag: Skip displaying the code_tag if it is not called 2024-12-12 7:50 ` Zhenhua Huang @ 2024-12-13 2:44 ` Suren Baghdasaryan 2024-12-13 3:51 ` Zhenhua Huang 0 siblings, 1 reply; 5+ messages in thread From: Suren Baghdasaryan @ 2024-12-13 2:44 UTC (permalink / raw) To: Zhenhua Huang; +Cc: akpm, kent.overstreet, linux-kernel, linux-mm On Wed, Dec 11, 2024 at 11:50 PM Zhenhua Huang <quic_zhenhuah@quicinc.com> wrote: > > Hi Suren, > > On 2024/12/12 15:16, Suren Baghdasaryan wrote: > > On Wed, Dec 11, 2024 at 12:56 AM Zhenhua Huang > > <quic_zhenhuah@quicinc.com> wrote: > >> > >> /proc/allocinfo is full of callsites which are not called at all. > >> Let's only output if the callsite actually been invoked. > > > > No, I disagree. Allocation that was never invoked is not the same as > > no allocation at all. How would we know the difference if we filter > > But it doesn't affect further display when it is actually called? why we > need to know the diff here... > > The point from me is: up to now, the callsite hasn't been invoked, so it > can be ignored in the output.. The original output is really huge.. My point is that with this change we lose information which can be useful. For example if I want to analyze all the places in the kernel where memory can be potentially allocated, your change would prevent me from doing that. > > > out the empty ones? > > If you don't want to see all the unused sites, you can filter them in > > the userspace. I also suspect that for practical purposes you would > > want to filter small ones (below some threshold) as well. > > Yeah, that's the expected way from us as well :) > > > > >> > >> Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com> > >> --- > >> lib/alloc_tag.c | 10 ++++++---- > >> 1 file changed, 6 insertions(+), 4 deletions(-) > >> > >> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c > >> index 35f7560a309a..06fb7eb5c0bc 100644 > >> --- a/lib/alloc_tag.c > >> +++ b/lib/alloc_tag.c > >> @@ -95,10 +95,12 @@ static void alloc_tag_to_text(struct seq_buf *out, struct codetag *ct) > >> struct alloc_tag_counters counter = alloc_tag_read(tag); > >> s64 bytes = counter.bytes; > >> > >> - seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls); > >> - codetag_to_text(out, ct); > >> - seq_buf_putc(out, ' '); > >> - seq_buf_putc(out, '\n'); > >> + if (bytes || counter.calls) { > >> + seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls); > >> + codetag_to_text(out, ct); > >> + seq_buf_putc(out, ' '); > >> + seq_buf_putc(out, '\n'); > >> + } > >> } > >> > >> static int allocinfo_show(struct seq_file *m, void *arg) > >> -- > >> 2.25.1 > >> > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/code_tag: Skip displaying the code_tag if it is not called 2024-12-13 2:44 ` Suren Baghdasaryan @ 2024-12-13 3:51 ` Zhenhua Huang 0 siblings, 0 replies; 5+ messages in thread From: Zhenhua Huang @ 2024-12-13 3:51 UTC (permalink / raw) To: Suren Baghdasaryan; +Cc: akpm, kent.overstreet, linux-kernel, linux-mm On 2024/12/13 10:44, Suren Baghdasaryan wrote: > On Wed, Dec 11, 2024 at 11:50 PM Zhenhua Huang > <quic_zhenhuah@quicinc.com> wrote: >> >> Hi Suren, >> >> On 2024/12/12 15:16, Suren Baghdasaryan wrote: >>> On Wed, Dec 11, 2024 at 12:56 AM Zhenhua Huang >>> <quic_zhenhuah@quicinc.com> wrote: >>>> >>>> /proc/allocinfo is full of callsites which are not called at all. >>>> Let's only output if the callsite actually been invoked. >>> >>> No, I disagree. Allocation that was never invoked is not the same as >>> no allocation at all. How would we know the difference if we filter >> >> But it doesn't affect further display when it is actually called? why we >> need to know the diff here... >> >> The point from me is: up to now, the callsite hasn't been invoked, so it >> can be ignored in the output.. The original output is really huge.. > > My point is that with this change we lose information which can be > useful. For example if I want to analyze all the places in the kernel > where memory can be potentially allocated, your change would prevent > me from doing that. OK, Thanks, got it. > >> >>> out the empty ones? >>> If you don't want to see all the unused sites, you can filter them in >>> the userspace. I also suspect that for practical purposes you would >>> want to filter small ones (below some threshold) as well. >> >> Yeah, that's the expected way from us as well :) >> >>> >>>> >>>> Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com> >>>> --- >>>> lib/alloc_tag.c | 10 ++++++---- >>>> 1 file changed, 6 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c >>>> index 35f7560a309a..06fb7eb5c0bc 100644 >>>> --- a/lib/alloc_tag.c >>>> +++ b/lib/alloc_tag.c >>>> @@ -95,10 +95,12 @@ static void alloc_tag_to_text(struct seq_buf *out, struct codetag *ct) >>>> struct alloc_tag_counters counter = alloc_tag_read(tag); >>>> s64 bytes = counter.bytes; >>>> >>>> - seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls); >>>> - codetag_to_text(out, ct); >>>> - seq_buf_putc(out, ' '); >>>> - seq_buf_putc(out, '\n'); >>>> + if (bytes || counter.calls) { >>>> + seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls); >>>> + codetag_to_text(out, ct); >>>> + seq_buf_putc(out, ' '); >>>> + seq_buf_putc(out, '\n'); >>>> + } >>>> } >>>> >>>> static int allocinfo_show(struct seq_file *m, void *arg) >>>> -- >>>> 2.25.1 >>>> >> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-12-13 3:51 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-12-11 8:56 [PATCH] mm/code_tag: Skip displaying the code_tag if it is not called Zhenhua Huang 2024-12-12 7:16 ` Suren Baghdasaryan 2024-12-12 7:50 ` Zhenhua Huang 2024-12-13 2:44 ` Suren Baghdasaryan 2024-12-13 3:51 ` Zhenhua Huang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox