Stephen Brennan <stephen.s.brennan@xxxxxxxxxx> writes:
> The pid_revalidate() function requires dropping from RCU into REF lookup
> mode. When many threads are resolving paths within /proc in parallel,
> this can result in heavy spinlock contention as each thread tries to
> grab a reference to the /proc dentry lock (and drop it shortly
> thereafter).
I am feeling dense at the moment. Which lock specifically are you
referring to? The only locks I can thinking of are sleeping locks,
not spinlocks.
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index ebea9501afb8..833d55a59e20 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -1830,19 +1846,22 @@ static int pid_revalidate(struct dentry *dentry, unsigned int flags)
> {
> struct inode *inode;
> struct task_struct *task;
> + int rv = 0;
>
> - if (flags & LOOKUP_RCU)
> - return -ECHILD;
> -
> - inode = d_inode(dentry);
> - task = get_proc_task(inode);
> -
> - if (task) {
> - pid_update_inode(task, inode);
> - put_task_struct(task);
> - return 1;
> + if (flags & LOOKUP_RCU) {
Why do we need to test flags here at all?
Why can't the code simply take an rcu_read_lock unconditionally and just
pass flags into do_pid_update_inode?
> + inode = d_inode_rcu(dentry);
> + task = pid_task(proc_pid(inode), PIDTYPE_PID);
> + if (task)
> + rv = do_pid_update_inode(task, inode, flags);
> + } else {
> + inode = d_inode(dentry);
> + task = get_proc_task(inode);
> + if (task) {
> + rv = do_pid_update_inode(task, inode, flags);
> + put_task_struct(task);
> + }
> }
> - return 0;
> + return rv;
> }
>
> static inline bool proc_inode_is_dead(struct inode *inode)
Eric
