Messages in this thread Patch in this message | | | Date | Wed, 8 Jan 2020 12:23:11 -0800 | From | Fenghua Yu <> | Subject | Re: [bug report] resctrl high memory comsumption |
| |
On Wed, Jan 08, 2020 at 09:07:41AM -0800, Shakeel Butt wrote: > Hi, > > Recently we had a bug in the system software writing the same pids to > the tasks file of resctrl group multiple times. The resctrl code > allocates "struct task_move_callback" for each such write and call > task_work_add() for that task to handle it on return to user-space > without checking if such request already exist for that particular > task. The issue arises for long sleeping tasks which has thousands for > such request queued to be handled. On our production, we notice > thousands of tasks having thousands of such requests and taking GiBs > of memory for "struct task_move_callback". I am not very familiar with > the code to judge if task_work_cancel() is the right approach or just > checking closid/rmid before doing task_work_add(). >
Thank you for reporting the issue, Shakeel!
Could you please check if the following patch fixes the issue? From 3c23c39b6a44fdfbbbe0083d074dcc114d7d7f1c Mon Sep 17 00:00:00 2001 From: Fenghua Yu <fenghua.yu@intel.com> Date: Wed, 8 Jan 2020 19:53:33 +0000 Subject: [RFC PATCH] x86/resctrl: Fix redundant task movements
Currently a task can be moved to a rdtgroup multiple times. But, this can cause multiple task works are added, waste memory and degrade performance.
To fix the issue, only move the task to a rdtgroup when the task is not in the rdgroup. Don't try to move the task to the rdtgroup again when the task is already in the rdtgroup.
Reported-by: Shakeel Butt <shakeelb@google.com> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com> --- arch/x86/kernel/cpu/resctrl/rdtgroup.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c index 2e3b06d6bbc6..75300c4a5969 100644 --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c @@ -546,6 +546,17 @@ static int __rdtgroup_move_task(struct task_struct *tsk, struct task_move_callback *callback; int ret; + /* If the task is already in rdtgrp, don't move the task. */ + if ((rdtgrp->type == RDTCTRL_GROUP && tsk->closid == rdtgrp->closid && + tsk->rmid == rdtgrp->mon.rmid) || + (rdtgrp->type == RDTMON_GROUP && + rdtgrp->mon.parent->closid == tsk->closid && + tsk->rmid == rdtgrp->mon.rmid)) { + rdt_last_cmd_puts("Task is already in the rdgroup\n"); + + return -EINVAL; + } + callback = kzalloc(sizeof(*callback), GFP_KERNEL); if (!callback) return -ENOMEM; -- 2.19.1
| |