* [PATCH v2] mm: oom_kill: add trace logs in process_mrelease() system call
@ 2022-08-16 6:00 Pratyush Brahma
2022-08-17 0:35 ` Andrew Morton
2022-08-17 16:02 ` Andrew Morton
0 siblings, 2 replies; 6+ messages in thread
From: Pratyush Brahma @ 2022-08-16 6:00 UTC (permalink / raw)
To: akpm; +Cc: linux-mm, linux-kernel, quic_charante, Pratyush Brahma
From: Charan Teja Kalla <quic_charante@quicinc.com>
The process_mrelease() system call[1] is used to release the memory of
a dying process from the context of the caller, which is similar to and
uses the functions of the oom reaper logic. There exists trace logs for
a process when reaped by the oom reaper. Just extend the same to when
done by the process_mrelease() system call.
[1]
https://lore.kernel.org/linux-mm/20210809185259.405936-1-surenb@google.com/
Signed-off-by: Charan Teja Kalla <quic_charante@quicinc.com>
Signed-off-by: Pratyush Brahma <quic_pbrahma@quicinc.com>
---
Changes in v2:
- Added trace_skip_task_reaping() to cover more cases where we skip
reaping.
- Print debug information in pr_debug instead of pr_info
- The original author email domain has changed. Update the new email
address.
[v1]
https://patchwork.kernel.org/project/linux-mm/patch/1629106756-20874-1-git-send-email-charante@codeaurora.org/
mm/oom_kill.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 3c6cf9e3cd66..51ad5f0b612e 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -995,7 +995,6 @@ static void __oom_kill_process(struct task_struct *victim, const char *message)
mmdrop(mm);
put_task_struct(victim);
}
-#undef K
/*
* Kill provided task unless it's secured by setting
@@ -1241,17 +1240,33 @@ SYSCALL_DEFINE2(process_mrelease, int, pidfd, unsigned int, flags)
goto drop_mm;
if (mmap_read_lock_killable(mm)) {
+ trace_skip_task_reaping(task->pid);
ret = -EINTR;
- goto drop_mm;
+ goto read_unlock;
}
/*
* Check MMF_OOM_SKIP again under mmap_read_lock protection to ensure
* possible change in exit_mmap is seen
*/
- if (!test_bit(MMF_OOM_SKIP, &mm->flags) && !__oom_reap_task_mm(mm))
+ if (test_bit(MMF_OOM_SKIP, &mm->flags)) {
+ trace_skip_task_reaping(task->pid);
+ goto read_unlock;
+ }
+
+ trace_start_task_reaping(task->pid);
+
+ if (!__oom_reap_task_mm(mm))
ret = -EAGAIN;
- mmap_read_unlock(mm);
+ pr_debug("process_mrelease: reaped process %d (%s), now anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lukB\n",
+ task_pid_nr(task), task->comm,
+ K(get_mm_counter(mm, MM_ANONPAGES)),
+ K(get_mm_counter(mm, MM_FILEPAGES)),
+ K(get_mm_counter(mm, MM_SHMEMPAGES)));
+ trace_finish_task_reaping(task->pid);
+
+read_unlock:
+ mmap_read_unlock(mm);
drop_mm:
mmdrop(mm);
put_task:
@@ -1261,3 +1276,4 @@ SYSCALL_DEFINE2(process_mrelease, int, pidfd, unsigned int, flags)
return -ENOSYS;
#endif /* CONFIG_MMU */
}
+#undef K
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] mm: oom_kill: add trace logs in process_mrelease() system call
2022-08-16 6:00 [PATCH v2] mm: oom_kill: add trace logs in process_mrelease() system call Pratyush Brahma
@ 2022-08-17 0:35 ` Andrew Morton
2022-08-17 6:47 ` Pratyush Brahma
2022-08-17 16:02 ` Andrew Morton
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2022-08-17 0:35 UTC (permalink / raw)
To: Pratyush Brahma; +Cc: linux-mm, linux-kernel, quic_charante, Pratyush Brahma
On Tue, 16 Aug 2022 11:30:17 +0530 Pratyush Brahma <pbrahma@qti.qualcomm.com> wrote:
> The process_mrelease() system call[1] is used to release the memory of
> a dying process from the context of the caller, which is similar to and
> uses the functions of the oom reaper logic. There exists trace logs for
> a process when reaped by the oom reaper. Just extend the same to when
> done by the process_mrelease() system call.
Why? Please describe in full detail the end-user value of this change.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm: oom_kill: add trace logs in process_mrelease() system call
2022-08-17 0:35 ` Andrew Morton
@ 2022-08-17 6:47 ` Pratyush Brahma
2022-09-19 14:43 ` Michal Hocko
0 siblings, 1 reply; 6+ messages in thread
From: Pratyush Brahma @ 2022-08-17 6:47 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-mm, linux-kernel, quic_charante, quic_pkondeti
On 17-08-2022 06:05, Andrew Morton wrote:
> On Tue, 16 Aug 2022 11:30:17 +0530 Pratyush Brahma <pbrahma@qti.qualcomm.com> wrote:
>
>> The process_mrelease() system call[1] is used to release the memory of
>> a dying process from the context of the caller, which is similar to and
>> uses the functions of the oom reaper logic. There exists trace logs for
>> a process when reaped by the oom reaper. Just extend the same to when
>> done by the process_mrelease() system call.
> Why? Please describe in full detail the end-user value of this change.
This patch provides information on how much memory is freed from a process
which is being reaped. Adding trace events in the process_mrelease() path when
process is being reaped would enable more holistic debug as it happens in
oom_reap_task_mm() currently.
This extends the debug functionality for the events as described in [1] to
the process_mrelease() system call. Now the coverage of trace events is complete.
[1]
https://lore.kernel.org/all/20170530185231.GA13412@castle/T/#u
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm: oom_kill: add trace logs in process_mrelease() system call
2022-08-17 6:47 ` Pratyush Brahma
@ 2022-09-19 14:43 ` Michal Hocko
0 siblings, 0 replies; 6+ messages in thread
From: Michal Hocko @ 2022-09-19 14:43 UTC (permalink / raw)
To: Pratyush Brahma
Cc: Andrew Morton, linux-mm, linux-kernel, quic_charante, quic_pkondeti
On Wed 17-08-22 12:17:22, Pratyush Brahma wrote:
>
> On 17-08-2022 06:05, Andrew Morton wrote:
> > On Tue, 16 Aug 2022 11:30:17 +0530 Pratyush Brahma <pbrahma@qti.qualcomm.com> wrote:
> >
> > > The process_mrelease() system call[1] is used to release the memory of
> > > a dying process from the context of the caller, which is similar to and
> > > uses the functions of the oom reaper logic. There exists trace logs for
> > > a process when reaped by the oom reaper. Just extend the same to when
> > > done by the process_mrelease() system call.
> > Why? Please describe in full detail the end-user value of this change.
>
> This patch provides information on how much memory is freed from a process
> which is being reaped. Adding trace events in the process_mrelease() path when
> process is being reaped would enable more holistic debug as it happens in
> oom_reap_task_mm() currently.
>
> This extends the debug functionality for the events as described in [1] to
> the process_mrelease() system call. Now the coverage of trace events is complete.
Yes this is all nice but why it is needed to extend process_mrelease to
be in par with the oom path? OOM path happens out of direct control of
while this is under direct control of the caller. So why do we need more
debugging data from the kernel? The information dumped by the tracing
can be queried already by other means and much more extensibly.
> [1]
> https://lore.kernel.org/all/20170530185231.GA13412@castle/T/#u
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm: oom_kill: add trace logs in process_mrelease() system call
2022-08-16 6:00 [PATCH v2] mm: oom_kill: add trace logs in process_mrelease() system call Pratyush Brahma
2022-08-17 0:35 ` Andrew Morton
@ 2022-08-17 16:02 ` Andrew Morton
2022-08-18 4:45 ` Pratyush Brahma
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2022-08-17 16:02 UTC (permalink / raw)
To: Pratyush Brahma; +Cc: linux-mm, linux-kernel, quic_charante, Pratyush Brahma
On Tue, 16 Aug 2022 11:30:17 +0530 Pratyush Brahma <pbrahma@qti.qualcomm.com> wrote:
> From: Charan Teja Kalla <quic_charante@quicinc.com>
>
> The process_mrelease() system call[1] is used to release the memory of
> a dying process from the context of the caller, which is similar to and
> uses the functions of the oom reaper logic. There exists trace logs for
> a process when reaped by the oom reaper. Just extend the same to when
> done by the process_mrelease() system call.
>
> ...
>
> + pr_debug("process_mrelease: reaped process %d (%s), now anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lukB\n",
> + task_pid_nr(task), task->comm,
> + K(get_mm_counter(mm, MM_ANONPAGES)),
> + K(get_mm_counter(mm, MM_FILEPAGES)),
> + K(get_mm_counter(mm, MM_SHMEMPAGES)));
This addition wasn't changelogged. It's the only pr_debug in
oom_kill.c. Please explain?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] mm: oom_kill: add trace logs in process_mrelease() system call
2022-08-17 16:02 ` Andrew Morton
@ 2022-08-18 4:45 ` Pratyush Brahma
0 siblings, 0 replies; 6+ messages in thread
From: Pratyush Brahma @ 2022-08-18 4:45 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-mm, linux-kernel, quic_charante, quic_pkondeti
On 17-08-2022 21:32, Andrew Morton wrote:
> On Tue, 16 Aug 2022 11:30:17 +0530 Pratyush Brahma <pbrahma@qti.qualcomm.com> wrote:
>
>> From: Charan Teja Kalla <quic_charante@quicinc.com>
>>
>> The process_mrelease() system call[1] is used to release the memory of
>> a dying process from the context of the caller, which is similar to and
>> uses the functions of the oom reaper logic. There exists trace logs for
>> a process when reaped by the oom reaper. Just extend the same to when
>> done by the process_mrelease() system call.
>>
>> ...
>>
>> + pr_debug("process_mrelease: reaped process %d (%s), now anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lukB\n",
>> + task_pid_nr(task), task->comm,
>> + K(get_mm_counter(mm, MM_ANONPAGES)),
>> + K(get_mm_counter(mm, MM_FILEPAGES)),
>> + K(get_mm_counter(mm, MM_SHMEMPAGES)));
> This addition wasn't changelogged. It's the only pr_debug in
> oom_kill.c. Please explain?
The equivalent pr_info() message as in oom_reap_task_mm() is made
pr_debug() here as per review comments in patch-set v1.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-19 14:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-16 6:00 [PATCH v2] mm: oom_kill: add trace logs in process_mrelease() system call Pratyush Brahma
2022-08-17 0:35 ` Andrew Morton
2022-08-17 6:47 ` Pratyush Brahma
2022-09-19 14:43 ` Michal Hocko
2022-08-17 16:02 ` Andrew Morton
2022-08-18 4:45 ` Pratyush Brahma
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).