On Tue, Sep 19, 2023 at 04:33:35PM -0700, Suraj Jitindar Singh wrote:
> From: Zhihao Cheng <chengzhihao1@xxxxxxxxxx>
>
> commit d919a1e79bac890421537cf02ae773007bf55e6b upstream.
>
> Commit 7bc3e6e55acf06 ("proc: Use a list of inodes to flush from proc")
> moved proc_flush_task() behind __exit_signal(). Then, process systemd can
> take long period high cpu usage during releasing task in following
> concurrent processes:
>
> systemd ps
> kernel_waitid stat(/proc/tgid)
> do_wait filename_lookup
> wait_consider_task lookup_fast
> release_task
> __exit_signal
> __unhash_process
> detach_pid
> __change_pid // remove task->pid_links
> d_revalidate -> pid_revalidate // 0
> d_invalidate(/proc/tgid)
> shrink_dcache_parent(/proc/tgid)
> d_walk(/proc/tgid)
> spin_lock_nested(/proc/tgid/fd)
> // iterating opened fd
> proc_flush_pid |
> d_invalidate (/proc/tgid/fd) |
> shrink_dcache_parent(/proc/tgid/fd) |
> shrink_dentry_list(subdirs) ↓
> shrink_lock_dentry(/proc/tgid/fd) --> race on dentry lock
>
> Function d_invalidate() will remove dentry from hash firstly, but why does
> proc_flush_pid() process dentry '/proc/tgid/fd' before dentry
> '/proc/tgid'? That's because proc_pid_make_inode() adds proc inode in
> reverse order by invoking hlist_add_head_rcu(). But proc should not add
> any inodes under '/proc/tgid' except '/proc/tgid/task/pid', fix it by
> adding inode into 'pid->inodes' only if the inode is /proc/tgid or
> /proc/tgid/task/pid.
>
> Performance regression:
> Create 200 tasks, each task open one file for 50,000 times. Kill all
> tasks when opened files exceed 10,000,000 (cat /proc/sys/fs/file-nr).
>
> Before fix:
> $ time killall -wq aa
> real 4m40.946s # During this period, we can see 'ps' and 'systemd'
> taking high cpu usage.
>
> After fix:
> $ time killall -wq aa
> real 1m20.732s # During this period, we can see 'systemd' taking
> high cpu usage.
>
> Link: https://lkml.kernel.org/r/20220713130029.4133533-1-chengzhihao1@xxxxxxxxxx
> Fixes: 7bc3e6e55acf06 ("proc: Use a list of inodes to flush from proc")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216054
> Signed-off-by: Zhihao Cheng <chengzhihao1@xxxxxxxxxx>
> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
> Suggested-by: Brian Foster <bfoster@xxxxxxxxxx>
> Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> Cc: Alexey Dobriyan <adobriyan@xxxxxxxxx>
> Cc: Eric Biederman <ebiederm@xxxxxxxxxxxx>
> Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
> Cc: Baoquan He <bhe@xxxxxxxxxx>
> Cc: Kalesh Singh <kaleshsingh@xxxxxxxxxx>
> Cc: Yu Kuai <yukuai3@xxxxxxxxxx>
> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> [ bp: Context adjustments ]
> Signed-off-by: Suraj Jitindar Singh <surajjs@xxxxxxxxxx>
both now queued up, thanks.
greg k-h
