From: Charan Teja Kalla <[email protected]>
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/[email protected]/
Signed-off-by: Charan Teja Kalla <[email protected]>
Signed-off-by: Pratyush Brahma <[email protected]>
---
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/[email protected]/
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
On Tue, 16 Aug 2022 11:30:17 +0530 Pratyush Brahma <[email protected]> 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.
On 17-08-2022 06:05, Andrew Morton wrote:
> On Tue, 16 Aug 2022 11:30:17 +0530 Pratyush Brahma <[email protected]> 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
On Tue, 16 Aug 2022 11:30:17 +0530 Pratyush Brahma <[email protected]> wrote:
> From: Charan Teja Kalla <[email protected]>
>
> 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?
On 17-08-2022 21:32, Andrew Morton wrote:
> On Tue, 16 Aug 2022 11:30:17 +0530 Pratyush Brahma <[email protected]> wrote:
>
>> From: Charan Teja Kalla <[email protected]>
>>
>> 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.
>
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 <[email protected]> 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