Skip to content

Commit

Permalink
ANDROID: uid_sys_stats: Use a single work for deferred updates
Browse files Browse the repository at this point in the history
uid_sys_stats tries to acquire a lock when any task exits to do some
bookkeeping in common data structure. If the lock is contended, it
allocates and schedules a work to do the work later to avoid task exit
latency.

In a stress test which creates many tasks exiting, the workqueue can be
overwhelmed by the number of works being scheduled and allocates more
worker threads to handle queue. The growth of the number of threads is
effectively unbounded and can exhaust the process table. This causes
denial of service to userspace trying to fork().

Instead of allocating a new work each, create a linked list of the
update stats deferred work and have a single work to drain the linked
list. The linked list is implemented using an atomic_long_t.

Bug: 294468796
Fixes: 5586278c0fe6 ("ANDROID: uid_sys_stats: defer process_notifier work if uid_lock is contended")
Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
(cherry picked from https://android-review.googlesource.com/q/commit:8e86825eecfaaa582ab51a0924b469d2d2adc743)
Merged-In: I15f20f4f69ea66a452bdf815c4ef3a0da3edfd36
Change-Id: I15f20f4f69ea66a452bdf815c4ef3a0da3edfd36
  • Loading branch information
eberman-quic authored and Treehugger Robot committed Sep 5, 2023
1 parent 622c141 commit f37ba43
Showing 1 changed file with 28 additions and 20 deletions.
48 changes: 28 additions & 20 deletions drivers/misc/uid_sys_stats.c
Original file line number Diff line number Diff line change
Expand Up @@ -629,46 +629,53 @@ static const struct proc_ops uid_procstat_fops = {
};

struct update_stats_work {
struct work_struct work;
uid_t uid;
#ifdef CONFIG_UID_SYS_STATS_DEBUG
struct task_struct *task;
#endif
struct task_io_accounting ioac;
u64 utime;
u64 stime;
struct update_stats_work *next;
};

static atomic_long_t work_usw;

static void update_stats_workfn(struct work_struct *work)
{
struct update_stats_work *usw =
container_of(work, struct update_stats_work, work);
struct update_stats_work *usw;
struct uid_entry *uid_entry;
struct task_entry *task_entry __maybe_unused;

rt_mutex_lock(&uid_lock);
uid_entry = find_uid_entry(usw->uid);
if (!uid_entry)
goto exit;
while ((usw = (struct update_stats_work *)atomic_long_read(&work_usw))) {
if (atomic_long_cmpxchg(&work_usw, (long)usw, (long)(usw->next)) != (long)usw)
continue;

uid_entry = find_uid_entry(usw->uid);
if (!uid_entry)
goto next;

uid_entry->utime += usw->utime;
uid_entry->stime += usw->stime;
uid_entry->utime += usw->utime;
uid_entry->stime += usw->stime;

#ifdef CONFIG_UID_SYS_STATS_DEBUG
task_entry = find_task_entry(uid_entry, usw->task);
if (!task_entry)
goto exit;
add_uid_tasks_io_stats(task_entry, &usw->ioac,
UID_STATE_DEAD_TASKS);
task_entry = find_task_entry(uid_entry, usw->task);
if (!task_entry)
goto next;
add_uid_tasks_io_stats(task_entry, &usw->ioac,
UID_STATE_DEAD_TASKS);
#endif
__add_uid_io_stats(uid_entry, &usw->ioac, UID_STATE_DEAD_TASKS);
exit:
rt_mutex_unlock(&uid_lock);
__add_uid_io_stats(uid_entry, &usw->ioac, UID_STATE_DEAD_TASKS);
next:
#ifdef CONFIG_UID_SYS_STATS_DEBUG
put_task_struct(usw->task);
put_task_struct(usw->task);
#endif
kfree(usw);
kfree(usw);
}
rt_mutex_unlock(&uid_lock);
}
static DECLARE_WORK(update_stats_work, update_stats_workfn);

static int process_notifier(struct notifier_block *self,
unsigned long cmd, void *v)
Expand All @@ -687,7 +694,6 @@ static int process_notifier(struct notifier_block *self,

usw = kmalloc(sizeof(struct update_stats_work), GFP_KERNEL);
if (usw) {
INIT_WORK(&usw->work, update_stats_workfn);
usw->uid = uid;
#ifdef CONFIG_UID_SYS_STATS_DEBUG
usw->task = get_task_struct(task);
Expand All @@ -698,7 +704,9 @@ static int process_notifier(struct notifier_block *self,
*/
usw->ioac = task->ioac;
task_cputime_adjusted(task, &usw->utime, &usw->stime);
schedule_work(&usw->work);
usw->next = (struct update_stats_work *)atomic_long_xchg(&work_usw,
(long)usw);
schedule_work(&update_stats_work);
}
return NOTIFY_OK;
}
Expand Down

0 comments on commit f37ba43

Please sign in to comment.