On Sun, Jun 09, 2019 at 01:09:00PM +0300, Konstantin Khlebnikov wrote: > Do not stuck forever if something wrong. > Killable lock allows to cleanup stuck tasks and simplifies investigation.
This patch breaks the CRIU project, because stat() returns EINTR instead of ENOENT: [root@fc24 criu]# stat /proc/self/map_files/0-0 stat: cannot stat '/proc/self/map_files/0-0': Interrupted system call Here is one inline comment with the fix for this issue. > > It seems ->d_revalidate() could return any error (except ECHILD) to > abort validation and pass error as result of lookup sequence. > > Signed-off-by: Konstantin Khlebnikov <[email protected]> > Reviewed-by: Roman Gushchin <[email protected]> > Reviewed-by: Cyrill Gorcunov <[email protected]> > Reviewed-by: Kirill Tkhai <[email protected]> It was nice to see all four of you in one place :). > Acked-by: Michal Hocko <[email protected]> > --- > fs/proc/base.c | 27 +++++++++++++++++++++------ > 1 file changed, 21 insertions(+), 6 deletions(-) > > diff --git a/fs/proc/base.c b/fs/proc/base.c > index 9c8ca6cd3ce4..515ab29c2adf 100644 > --- a/fs/proc/base.c > +++ b/fs/proc/base.c > @@ -1962,9 +1962,12 @@ static int map_files_d_revalidate(struct dentry > *dentry, unsigned int flags) > goto out; > > if (!dname_to_vma_addr(dentry, &vm_start, &vm_end)) { > - down_read(&mm->mmap_sem); > - exact_vma_exists = !!find_exact_vma(mm, vm_start, vm_end); > - up_read(&mm->mmap_sem); > + status = down_read_killable(&mm->mmap_sem); > + if (!status) { > + exact_vma_exists = !!find_exact_vma(mm, vm_start, > + vm_end); > + up_read(&mm->mmap_sem); > + } > } > > mmput(mm); > @@ -2010,8 +2013,11 @@ static int map_files_get_link(struct dentry *dentry, > struct path *path) > if (rc) > goto out_mmput; > > + rc = down_read_killable(&mm->mmap_sem); > + if (rc) > + goto out_mmput; > + > rc = -ENOENT; > - down_read(&mm->mmap_sem); > vma = find_exact_vma(mm, vm_start, vm_end); > if (vma && vma->vm_file) { > *path = vma->vm_file->f_path; > @@ -2107,7 +2113,10 @@ static struct dentry *proc_map_files_lookup(struct > inode *dir, > if (!mm) > goto out_put_task; > > - down_read(&mm->mmap_sem); > + result = ERR_PTR(-EINTR); > + if (down_read_killable(&mm->mmap_sem)) > + goto out_put_mm; > + result = ERR_PTR(-ENOENT); > vma = find_exact_vma(mm, vm_start, vm_end); > if (!vma) > goto out_no_vma; > @@ -2118,6 +2127,7 @@ static struct dentry *proc_map_files_lookup(struct > inode *dir, > > out_no_vma: > up_read(&mm->mmap_sem); > +out_put_mm: > mmput(mm); > out_put_task: > put_task_struct(task); > @@ -2160,7 +2170,12 @@ proc_map_files_readdir(struct file *file, struct > dir_context *ctx) > mm = get_task_mm(task); > if (!mm) > goto out_put_task; > - down_read(&mm->mmap_sem); > + > + ret = down_read_killable(&mm->mmap_sem); > + if (ret) { > + mmput(mm); > + goto out_put_task; > + } > > nr_files = 0; >
