| Message ID | 155790967258.1319.11531787078240675602.stgit@buzz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/5] proc: use down_read_killable for /proc/pid/maps | expand |
On Wed, May 15, 2019 at 11:41:12AM +0300, Konstantin Khlebnikov wrote: > Do not stuck forever if something wrong. > This function also used for /proc/pid/smaps. > > Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> All patches in series look ok to me (actually I thought if there is a scenario where might_sleep may trigger a warning, and didn't find one, so should be safe). Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>
On 15.05.2019 11:41, Konstantin Khlebnikov wrote: > Do not stuck forever if something wrong. > This function also used for /proc/pid/smaps. > > Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> For the series: Reviewed-by: Kirill Tkhai <ktkhai@virtuozzo.com> > --- > fs/proc/task_mmu.c | 6 +++++- > fs/proc/task_nommu.c | 6 +++++- > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c > index 01d4eb0e6bd1..2bf210229daf 100644 > --- a/fs/proc/task_mmu.c > +++ b/fs/proc/task_mmu.c > @@ -166,7 +166,11 @@ static void *m_start(struct seq_file *m, loff_t *ppos) > if (!mm || !mmget_not_zero(mm)) > return NULL; > > - down_read(&mm->mmap_sem); > + if (down_read_killable(&mm->mmap_sem)) { > + mmput(mm); > + return ERR_PTR(-EINTR); > + } > + > hold_task_mempolicy(priv); > priv->tail_vma = get_gate_vma(mm); > > diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c > index 36bf0f2e102e..7907e6419e57 100644 > --- a/fs/proc/task_nommu.c > +++ b/fs/proc/task_nommu.c > @@ -211,7 +211,11 @@ static void *m_start(struct seq_file *m, loff_t *pos) > if (!mm || !mmget_not_zero(mm)) > return NULL; > > - down_read(&mm->mmap_sem); > + if (down_read_killable(&mm->mmap_sem)) { > + mmput(mm); > + return ERR_PTR(-EINTR); > + } > + > /* start from the Nth VMA */ > for (p = rb_first(&mm->mm_rb); p; p = rb_next(p)) > if (n-- == 0)
On Wed 15-05-19 11:41:12, Konstantin Khlebnikov wrote: > Do not stuck forever if something wrong. > This function also used for /proc/pid/smaps. I do agree that the killable variant is better but I do not understand the changelog. What would keep the lock blocked for ever? I do not think we have writer lock held for unbound amount of time anywhere, do we? > Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> Other than that Acked-by: Michal Hocko <mhocko@suse.com> > --- > fs/proc/task_mmu.c | 6 +++++- > fs/proc/task_nommu.c | 6 +++++- > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c > index 01d4eb0e6bd1..2bf210229daf 100644 > --- a/fs/proc/task_mmu.c > +++ b/fs/proc/task_mmu.c > @@ -166,7 +166,11 @@ static void *m_start(struct seq_file *m, loff_t *ppos) > if (!mm || !mmget_not_zero(mm)) > return NULL; > > - down_read(&mm->mmap_sem); > + if (down_read_killable(&mm->mmap_sem)) { > + mmput(mm); > + return ERR_PTR(-EINTR); > + } > + > hold_task_mempolicy(priv); > priv->tail_vma = get_gate_vma(mm); > > diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c > index 36bf0f2e102e..7907e6419e57 100644 > --- a/fs/proc/task_nommu.c > +++ b/fs/proc/task_nommu.c > @@ -211,7 +211,11 @@ static void *m_start(struct seq_file *m, loff_t *pos) > if (!mm || !mmget_not_zero(mm)) > return NULL; > > - down_read(&mm->mmap_sem); > + if (down_read_killable(&mm->mmap_sem)) { > + mmput(mm); > + return ERR_PTR(-EINTR); > + } > + > /* start from the Nth VMA */ > for (p = rb_first(&mm->mm_rb); p; p = rb_next(p)) > if (n-- == 0)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c index 01d4eb0e6bd1..2bf210229daf 100644 --- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c @@ -166,7 +166,11 @@ static void *m_start(struct seq_file *m, loff_t *ppos) if (!mm || !mmget_not_zero(mm)) return NULL; - down_read(&mm->mmap_sem); + if (down_read_killable(&mm->mmap_sem)) { + mmput(mm); + return ERR_PTR(-EINTR); + } + hold_task_mempolicy(priv); priv->tail_vma = get_gate_vma(mm); diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c index 36bf0f2e102e..7907e6419e57 100644 --- a/fs/proc/task_nommu.c +++ b/fs/proc/task_nommu.c @@ -211,7 +211,11 @@ static void *m_start(struct seq_file *m, loff_t *pos) if (!mm || !mmget_not_zero(mm)) return NULL; - down_read(&mm->mmap_sem); + if (down_read_killable(&mm->mmap_sem)) { + mmput(mm); + return ERR_PTR(-EINTR); + } + /* start from the Nth VMA */ for (p = rb_first(&mm->mm_rb); p; p = rb_next(p)) if (n-- == 0)
Do not stuck forever if something wrong. This function also used for /proc/pid/smaps. Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> --- fs/proc/task_mmu.c | 6 +++++- fs/proc/task_nommu.c | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-)