Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1345808 > unrolled thread

[PATCH 0/18] change mmap_sem taken for write killable

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/18] change mmap_sem taken for write killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 14:30 +0100
    [PATCH 13/18] exec: make exec path waiting for mmap_sem killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 14:30 +0100
      Re: [PATCH 13/18] exec: make exec path waiting for mmap_sem killable Oleg Nesterov <oleg@redhat.com> - 2016-02-29 18:30 +0100
        Re: [PATCH 13/18] exec: make exec path waiting for mmap_sem killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 18:50 +0100
          Re: [PATCH 13/18] exec: make exec path waiting for mmap_sem killable Oleg Nesterov <oleg@redhat.com> - 2016-02-29 19:20 +0100
    [PATCH 15/18] uprobes: wait for mmap_sem for write killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 14:30 +0100
      Re: [PATCH 15/18] uprobes: wait for mmap_sem for write killable Oleg Nesterov <oleg@redhat.com> - 2016-02-29 17:00 +0100
        Re: [PATCH 15/18] uprobes: wait for mmap_sem for write killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 17:30 +0100
          Re: [PATCH 15/18] uprobes: wait for mmap_sem for write killable Oleg Nesterov <oleg@redhat.com> - 2016-02-29 18:20 +0100
      [PATCH] uprobes: wait for mmap_sem for write killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 18:50 +0100
        Re: [PATCH] uprobes: wait for mmap_sem for write killable kbuild test robot <lkp@intel.com> - 2016-02-29 19:00 +0100
        Re: [PATCH] uprobes: wait for mmap_sem for write killable kbuild test robot <lkp@intel.com> - 2016-02-29 19:00 +0100
        Re: [PATCH] uprobes: wait for mmap_sem for write killable Oleg Nesterov <oleg@redhat.com> - 2016-02-29 19:20 +0100
          Re: [PATCH] uprobes: wait for mmap_sem for write killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 19:30 +0100
            Re: [PATCH] uprobes: wait for mmap_sem for write killable Oleg Nesterov <oleg@redhat.com> - 2016-02-29 19:40 +0100
    [PATCH 08/18] mm, fork: make dup_mmap wait for mmap_sem for write killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 14:30 +0100
      Re: [PATCH 08/18] mm, fork: make dup_mmap wait for mmap_sem for  write killable Oleg Nesterov <oleg@redhat.com> - 2016-02-29 19:00 +0100
      [PATCH] mm, fork: make dup_mmap wait for mmap_sem for write killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 19:10 +0100
        Re: [PATCH] mm, fork: make dup_mmap wait for mmap_sem for write  killable kbuild test robot <lkp@intel.com> - 2016-02-29 21:20 +0100
    [PATCH 10/18] vdso: make arch_setup_additional_pages wait for mmap_sem for write killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 14:30 +0100
      Re: [PATCH 10/18] vdso: make arch_setup_additional_pages wait for  mmap_sem for write killable Andy Lutomirski <luto@amacapital.net> - 2016-02-29 16:50 +0100
    [PATCH 06/18] mm: make vm_brk killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 14:40 +0100
    [PATCH 18/18] drm/amdgpu: make amdgpu_mn_get wait for mmap_sem killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 14:40 +0100
    [PATCH 05/18] mm, elf: handle vm_brk error Michal Hocko <mhocko@kernel.org> - 2016-02-29 14:40 +0100
    [PATCH 16/18] drm/i915: make i915_gem_mmap_ioctl wait for mmap_sem killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 14:40 +0100
    [PATCH 12/18] aio: make aio_setup_ring killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 14:40 +0100
      Re: [PATCH 12/18] aio: make aio_setup_ring killable Jeff Moyer <jmoyer@redhat.com> - 2016-02-29 17:20 +0100
    [PATCH 01/18] mm: Make mmap_sem for write waits killable for mm syscalls Michal Hocko <mhocko@kernel.org> - 2016-02-29 14:40 +0100
      Re: [PATCH 01/18] mm: Make mmap_sem for write waits killable for mm  syscalls kbuild test robot <lkp@intel.com> - 2016-02-29 14:50 +0100
    [PATCH 09/18] ipc, shm: make shmem attach/detach wait for mmap_sem killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 14:40 +0100
    [PATCH 02/18] mm: make vm_mmap killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 14:40 +0100
    Re: [PATCH 0/18] change mmap_sem taken for write killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 14:40 +0100
    [PATCH 14/18] prctl: make PR_SET_THP_DISABLE wait for mmap_sem killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 14:40 +0100
    [PATCH 17/18] drm/radeon: make radeon_mn_get wait for mmap_sem killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 14:40 +0100
      Re: [PATCH 17/18] drm/radeon: make radeon_mn_get wait for mmap_sem  killable Christian König <christian.koenig@amd.com> - 2016-02-29 15:20 +0100
    Re: [PATCH 0/18] change mmap_sem taken for write killable "Kirill A. Shutemov" <kirill@shutemov.name> - 2016-02-29 15:10 +0100
      Re: [PATCH 0/18] change mmap_sem taken for write killable Michal Hocko <mhocko@kernel.org> - 2016-02-29 15:20 +0100
        Re: [PATCH 0/18] change mmap_sem taken for write killable "Kirill A. Shutemov" <kirill@shutemov.name> - 2016-02-29 16:10 +0100

Page 1 of 2  [1] 2  Next page →

#1345808 — [PATCH 0/18] change mmap_sem taken for write killable

Hi,
this is a follow up work for oom_reaper [1]. As the async OOM killing
depends on oom_sem for read we would really appreciate if a holder for
write stood in the way. This patchset is changing many of down_write
calls to be killable to help those cases when the writer is blocked and
waiting for readers to release the lock and so help __oom_reap_task to
process the oom victim.

Most of the patches are really trivial because the lock is help from a
shallow syscall paths where we can return EINTR trivially. Others seem
to be easy as well as the callers are already handling fatal errors and
bail and return to userspace which should be sufficient to handle the
failure gracefully. I am not familiar with all those code paths so a
deeper review is really appreciated.

As this work is touching more areas which are not directly connected I
have tried to keep the CC list as small as possible and people who I
believed would be familiar are CCed only to the specific patches (all
should have received the cover though).

This patchset is based on linux-next and it depends on down_write_killable
for rw_semaphores posted recently [2].

I haven't covered all the mmap_write(mm->mmap_sem) instances here

$ git grep "down_write(.*\<mmap_sem\>)" next/master | wc -l
102
$ git grep "down_write(.*\<mmap_sem\>)" | wc -l
66

I have tried to cover those which should be relatively easy to review in
this series because this alone should be a nice improvement. Other places
can be changed on top.

Any feedback is highly appreciated.

---
[1] http://lkml.kernel.org/r/1452094975-551-1-git-send-email-mhocko@kernel.org
[2] http://lkml.kernel.org/r/1456750705-7141-1-git-send-email-mhocko@kernel.org

[toc] | [next] | [standalone]

#1345809 — [PATCH 13/18] exec: make exec path waiting for mmap_sem killable

From: Michal Hocko <mhocko@suse.com>

setup_arg_pages requires mmap_sem for write. If the waiting task
gets killed by the oom killer it would block oom_reaper from
asynchronous address space reclaim and reduce the chances of timely
OOM resolving. Wait for the lock in the killable mode and return with
EINTR if the task got killed while waiting. All the callers are already
handling error path and the fatal signal doesn't need any additional
treatment.

The same applies to __bprm_mm_init.

Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 fs/exec.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index c4010b8207a1..29f2f22ae067 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -267,7 +267,10 @@ static int __bprm_mm_init(struct linux_binprm *bprm)
 	if (!vma)
 		return -ENOMEM;
 
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem)) {
+		err = -EINTR;
+		goto err_free;
+	}
 	vma->vm_mm = mm;
 
 	/*
@@ -294,6 +297,7 @@ static int __bprm_mm_init(struct linux_binprm *bprm)
 	return 0;
 err:
 	up_write(&mm->mmap_sem);
+err_free:
 	bprm->vma = NULL;
 	kmem_cache_free(vm_area_cachep, vma);
 	return err;
@@ -700,7 +704,9 @@ int setup_arg_pages(struct linux_binprm *bprm,
 		bprm->loader -= stack_shift;
 	bprm->exec -= stack_shift;
 
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
+
 	vm_flags = VM_STACK_FLAGS;
 
 	/*
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1346018 — Re: [PATCH 13/18] exec: make exec path waiting for mmap_sem killable

On 02/29, Michal Hocko wrote:
>
> @@ -267,7 +267,10 @@ static int __bprm_mm_init(struct linux_binprm *bprm)
>  	if (!vma)
>  		return -ENOMEM;
>  
> -	down_write(&mm->mmap_sem);
> +	if (down_write_killable(&mm->mmap_sem)) {
> +		err = -EINTR;
> +		goto err_free;
> +	}
>  	vma->vm_mm = mm;

I won't argue, but this looks unnecessary. Nobody else can see this new mm,
down_write() can't block.

In fact I think we can just remove down_write/up_write here. Except perhaps
there is lockdep_assert_held() somewhere in these paths.

Oleg.

[toc] | [prev] | [next] | [standalone]

#1346040 — Re: [PATCH 13/18] exec: make exec path waiting for mmap_sem killable

On Mon 29-02-16 18:23:34, Oleg Nesterov wrote:
> On 02/29, Michal Hocko wrote:
> >
> > @@ -267,7 +267,10 @@ static int __bprm_mm_init(struct linux_binprm *bprm)
> >  	if (!vma)
> >  		return -ENOMEM;
> >  
> > -	down_write(&mm->mmap_sem);
> > +	if (down_write_killable(&mm->mmap_sem)) {
> > +		err = -EINTR;
> > +		goto err_free;
> > +	}
> >  	vma->vm_mm = mm;
> 
> I won't argue, but this looks unnecessary. Nobody else can see this new mm,
> down_write() can't block.
> 
> In fact I think we can just remove down_write/up_write here. Except perhaps
> there is lockdep_assert_held() somewhere in these paths.

This is what I had initially but then I've noticed that mm_alloc() does
mm_init(current)->init_new_context(current) so the outside can see this
mm AFAICS. Now I guess this shouldn't matter in the real life but the
code doesn't seem much harder to follow, the callers are already
handling all error paths so I guess it would be better to simply move on
this. Or am I misunderstanding the code or missing something?

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1346065 — Re: [PATCH 13/18] exec: make exec path waiting for mmap_sem killable

On 02/29, Michal Hocko wrote:
>
> On Mon 29-02-16 18:23:34, Oleg Nesterov wrote:
> > On 02/29, Michal Hocko wrote:
> > >
> > > @@ -267,7 +267,10 @@ static int __bprm_mm_init(struct linux_binprm *bprm)
> > >  	if (!vma)
> > >  		return -ENOMEM;
> > >
> > > -	down_write(&mm->mmap_sem);
> > > +	if (down_write_killable(&mm->mmap_sem)) {
> > > +		err = -EINTR;
> > > +		goto err_free;
> > > +	}
> > >  	vma->vm_mm = mm;
> >
> > I won't argue, but this looks unnecessary. Nobody else can see this new mm,
> > down_write() can't block.
> >
> > In fact I think we can just remove down_write/up_write here. Except perhaps
> > there is lockdep_assert_held() somewhere in these paths.
>
> This is what I had initially but then I've noticed that mm_alloc() does
> mm_init(current)->init_new_context(current)

yes, and init_new_context() is arch dependant...

> code doesn't seem much harder to follow, the callers are already
> handling all error paths so I guess it would be better to simply move on
> this.

Yes, agreed, please forget.

Oleg.

[toc] | [prev] | [next] | [standalone]

#1345810 — [PATCH 15/18] uprobes: wait for mmap_sem for write killable

From: Michal Hocko <mhocko@suse.com>

xol_add_vma needs mmap_sem for write. If the waiting task gets killed by
the oom killer it would block oom_reaper from asynchronous address space
reclaim and reduce the chances of timely OOM resolving. Wait for the
lock in the killable mode and return with EINTR if the task got killed
while waiting.

Cc: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 kernel/events/uprobes.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 8eef5f55d3f0..a79315d0f711 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1130,7 +1130,9 @@ static int xol_add_vma(struct mm_struct *mm, struct xol_area *area)
 	struct vm_area_struct *vma;
 	int ret;
 
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
+
 	if (mm->uprobes_state.xol_area) {
 		ret = -EALREADY;
 		goto fail;
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1345935 — Re: [PATCH 15/18] uprobes: wait for mmap_sem for write killable

On 02/29, Michal Hocko wrote:
>
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -1130,7 +1130,9 @@ static int xol_add_vma(struct mm_struct *mm, struct xol_area *area)
>  	struct vm_area_struct *vma;
>  	int ret;
>
> -	down_write(&mm->mmap_sem);
> +	if (down_write_killable(&mm->mmap_sem))
> +		return -EINTR;
> +

Yes, but then dup_xol_work() should probably check fatal_signal_pending() to
suppress uprobe_warn(), the warning looks like a kernel problem.

Oleg.

[toc] | [prev] | [next] | [standalone]

#1345968 — Re: [PATCH 15/18] uprobes: wait for mmap_sem for write killable

On Mon 29-02-16 16:57:13, Oleg Nesterov wrote:
> On 02/29, Michal Hocko wrote:
> >
> > --- a/kernel/events/uprobes.c
> > +++ b/kernel/events/uprobes.c
> > @@ -1130,7 +1130,9 @@ static int xol_add_vma(struct mm_struct *mm, struct xol_area *area)
> >  	struct vm_area_struct *vma;
> >  	int ret;
> >
> > -	down_write(&mm->mmap_sem);
> > +	if (down_write_killable(&mm->mmap_sem))
> > +		return -EINTR;
> > +
> 
> Yes, but then dup_xol_work() should probably check fatal_signal_pending() to
> suppress uprobe_warn(), the warning looks like a kernel problem.

Ahh, I see. I didn't understand what is the purpose of the warning. Does
the following work for you?
---
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index a79315d0f711..fb4a6bcc88ce 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1470,7 +1470,8 @@ static void dup_xol_work(struct callback_head *work)
 	if (current->flags & PF_EXITING)
 		return;
 
-	if (!__create_xol_area(current->utask->dup_xol_addr))
+	if (!__create_xol_area(current->utask->dup_xol_addr) &&
+			!fatal_signal_pending(current)
 		uprobe_warn(current, "dup xol area");
 }
 
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1346011 — Re: [PATCH 15/18] uprobes: wait for mmap_sem for write killable

On 02/29, Michal Hocko wrote:
>
> Ahh, I see. I didn't understand what is the purpose of the warning. Does
> the following work for you?
> ---
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index a79315d0f711..fb4a6bcc88ce 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -1470,7 +1470,8 @@ static void dup_xol_work(struct callback_head *work)
>  	if (current->flags & PF_EXITING)
>  		return;
>
> -	if (!__create_xol_area(current->utask->dup_xol_addr))
> +	if (!__create_xol_area(current->utask->dup_xol_addr) &&
> +			!fatal_signal_pending(current)
>  		uprobe_warn(current, "dup xol area");
>  }

Yes, I think this is fine.

Probably deserves a cleanup, but we can do it later.

Oleg.

[toc] | [prev] | [next] | [standalone]

#1346029 — [PATCH] uprobes: wait for mmap_sem for write killable

From: Michal Hocko <mhocko@suse.com>

xol_add_vma needs mmap_sem for write. If the waiting task gets killed by
the oom killer it would block oom_reaper from asynchronous address space
reclaim and reduce the chances of timely OOM resolving. Wait for the
lock in the killable mode and return with EINTR if the task got killed
while waiting.

Do not warn in dup_xol_work if __create_xol_area failed due to fatal
signal pending because this is usually considered a kernel issue.

Cc: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 kernel/events/uprobes.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 8eef5f55d3f0..fb4a6bcc88ce 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1130,7 +1130,9 @@ static int xol_add_vma(struct mm_struct *mm, struct xol_area *area)
 	struct vm_area_struct *vma;
 	int ret;
 
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
+
 	if (mm->uprobes_state.xol_area) {
 		ret = -EALREADY;
 		goto fail;
@@ -1468,7 +1470,8 @@ static void dup_xol_work(struct callback_head *work)
 	if (current->flags & PF_EXITING)
 		return;
 
-	if (!__create_xol_area(current->utask->dup_xol_addr))
+	if (!__create_xol_area(current->utask->dup_xol_addr) &&
+			!fatal_signal_pending(current)
 		uprobe_warn(current, "dup xol area");
 }
 
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1346050 — Re: [PATCH] uprobes: wait for mmap_sem for write killable

[Multipart message — attachments visible in raw view] — view raw

Hi Michal,

[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.5-rc6 next-20160229]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Michal-Hocko/uprobes-wait-for-mmap_sem-for-write-killable/20160301-014513
config: i386-randconfig-x003-201609 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from include/linux/linkage.h:4:0,
                    from include/linux/kernel.h:6,
                    from kernel/events/uprobes.c:25:
   kernel/events/uprobes.c: In function 'xol_add_vma':
   kernel/events/uprobes.c:1133:6: error: implicit declaration of function 'down_write_killable' [-Werror=implicit-function-declaration]
     if (down_write_killable(&mm->mmap_sem))
         ^
   include/linux/compiler.h:147:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^
   kernel/events/uprobes.c:1133:2: note: in expansion of macro 'if'
     if (down_write_killable(&mm->mmap_sem))
     ^
   kernel/events/uprobes.c: In function 'dup_xol_work':
>> kernel/events/uprobes.c:2029:0: error: unterminated argument list invoking macro "if"
    __initcall(init_uprobes);
    ^
>> kernel/events/uprobes.c:1473:2: error: expected '(' at end of input
     if (!__create_xol_area(current->utask->dup_xol_addr) &&
     ^
>> kernel/events/uprobes.c:1473:2: error: expected declaration or statement at end of input
   kernel/events/uprobes.c: At top level:
   kernel/events/uprobes.c:961:12: warning: 'unapply_uprobe' defined but not used [-Wunused-function]
    static int unapply_uprobe(struct uprobe *uprobe, struct mm_struct *mm)
               ^
   kernel/events/uprobes.c:1293:22: warning: 'xol_get_insn_slot' defined but not used [-Wunused-function]
    static unsigned long xol_get_insn_slot(struct uprobe *uprobe)
                         ^
   kernel/events/uprobes.c:1427:28: warning: 'get_utask' defined but not used [-Wunused-function]
    static struct uprobe_task *get_utask(void)
                               ^
   kernel/events/uprobes.c:1434:12: warning: 'dup_utask' defined but not used [-Wunused-function]
    static int dup_utask(struct task_struct *t, struct uprobe_task *o_utask)
               ^
   kernel/events/uprobes.c:1462:13: warning: 'uprobe_warn' defined but not used [-Wunused-function]
    static void uprobe_warn(struct task_struct *t, const char *msg)
                ^
   kernel/events/uprobes.c:1468:13: warning: 'dup_xol_work' defined but not used [-Wunused-function]
    static void dup_xol_work(struct callback_head *work)
                ^
   cc1: some warnings being treated as errors

vim +/if +2029 kernel/events/uprobes.c

0326f5a9 kernel/events/uprobes.c Srikar Dronamraju 2012-03-13  2023  
32cdba1e kernel/events/uprobes.c Oleg Nesterov     2012-11-14  2024  	if (percpu_init_rwsem(&dup_mmap_sem))
32cdba1e kernel/events/uprobes.c Oleg Nesterov     2012-11-14  2025  		return -ENOMEM;
32cdba1e kernel/events/uprobes.c Oleg Nesterov     2012-11-14  2026  
0326f5a9 kernel/events/uprobes.c Srikar Dronamraju 2012-03-13  2027  	return register_die_notifier(&uprobe_exception_nb);
2b144498 kernel/uprobes.c        Srikar Dronamraju 2012-02-09  2028  }
736e89d9 kernel/events/uprobes.c Oleg Nesterov     2013-10-31 @2029  __initcall(init_uprobes);

:::::: The code at line 2029 was first introduced by commit
:::::: 736e89d9f782a7dd9a38ecda13b2db916fa72f33 uprobes: Kill module_init() and module_exit()

:::::: TO: Oleg Nesterov <oleg@redhat.com>
:::::: CC: Oleg Nesterov <oleg@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1346051 — Re: [PATCH] uprobes: wait for mmap_sem for write killable

[Multipart message — attachments visible in raw view] — view raw

Hi Michal,

[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.5-rc6 next-20160229]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Michal-Hocko/uprobes-wait-for-mmap_sem-for-write-killable/20160301-014513
config: x86_64-randconfig-x019-201609 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   kernel/events/uprobes.c: In function 'xol_add_vma':
   kernel/events/uprobes.c:1133:6: error: implicit declaration of function 'down_write_killable' [-Werror=implicit-function-declaration]
     if (down_write_killable(&mm->mmap_sem))
         ^
   kernel/events/uprobes.c: In function 'dup_xol_work':
>> kernel/events/uprobes.c:1475:3: error: expected ')' before 'uprobe_warn'
      uprobe_warn(current, "dup xol area");
      ^
>> kernel/events/uprobes.c:1476:1: error: expected expression before '}' token
    }
    ^
   cc1: some warnings being treated as errors

vim +1475 kernel/events/uprobes.c

aa59c53f Oleg Nesterov 2013-10-13  1469  {
aa59c53f Oleg Nesterov 2013-10-13  1470  	if (current->flags & PF_EXITING)
aa59c53f Oleg Nesterov 2013-10-13  1471  		return;
aa59c53f Oleg Nesterov 2013-10-13  1472  
6b584cb3 Michal Hocko  2016-02-29  1473  	if (!__create_xol_area(current->utask->dup_xol_addr) &&
6b584cb3 Michal Hocko  2016-02-29  1474  			!fatal_signal_pending(current)
aa59c53f Oleg Nesterov 2013-10-13 @1475  		uprobe_warn(current, "dup xol area");
aa59c53f Oleg Nesterov 2013-10-13 @1476  }
aa59c53f Oleg Nesterov 2013-10-13  1477  
e78aebfd Anton Arapov  2013-04-03  1478  /*
b68e0749 Oleg Nesterov 2013-10-13  1479   * Called in context of a new clone/fork from copy_process.

:::::: The code at line 1475 was first introduced by commit
:::::: aa59c53fd4599c91ccf9629af0c2777b89929076 uprobes: Change uprobe_copy_process() to dup xol_area

:::::: TO: Oleg Nesterov <oleg@redhat.com>
:::::: CC: Oleg Nesterov <oleg@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1346066 — Re: [PATCH] uprobes: wait for mmap_sem for write killable

On 02/29, Michal Hocko wrote:
>
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -1130,7 +1130,9 @@ static int xol_add_vma(struct mm_struct *mm, struct xol_area *area)
>  	struct vm_area_struct *vma;
>  	int ret;
>  
> -	down_write(&mm->mmap_sem);
> +	if (down_write_killable(&mm->mmap_sem))
> +		return -EINTR;
> +
>  	if (mm->uprobes_state.xol_area) {
>  		ret = -EALREADY;
>  		goto fail;
> @@ -1468,7 +1470,8 @@ static void dup_xol_work(struct callback_head *work)
>  	if (current->flags & PF_EXITING)
>  		return;
>  
> -	if (!__create_xol_area(current->utask->dup_xol_addr))
> +	if (!__create_xol_area(current->utask->dup_xol_addr) &&
> +			!fatal_signal_pending(current)
>  		uprobe_warn(current, "dup xol area");
>  }

Looks good, thanks.

Oleg.

[toc] | [prev] | [next] | [standalone]

#1346073 — Re: [PATCH] uprobes: wait for mmap_sem for write killable

On Mon 29-02-16 19:11:06, Oleg Nesterov wrote:
> On 02/29, Michal Hocko wrote:
> >
> > --- a/kernel/events/uprobes.c
> > +++ b/kernel/events/uprobes.c
> > @@ -1130,7 +1130,9 @@ static int xol_add_vma(struct mm_struct *mm, struct xol_area *area)
> >  	struct vm_area_struct *vma;
> >  	int ret;
> >  
> > -	down_write(&mm->mmap_sem);
> > +	if (down_write_killable(&mm->mmap_sem))
> > +		return -EINTR;
> > +
> >  	if (mm->uprobes_state.xol_area) {
> >  		ret = -EALREADY;
> >  		goto fail;
> > @@ -1468,7 +1470,8 @@ static void dup_xol_work(struct callback_head *work)
> >  	if (current->flags & PF_EXITING)
> >  		return;
> >  
> > -	if (!__create_xol_area(current->utask->dup_xol_addr))
> > +	if (!__create_xol_area(current->utask->dup_xol_addr) &&
> > +			!fatal_signal_pending(current)
> >  		uprobe_warn(current, "dup xol area");
> >  }
> 
> Looks good, thanks.

Can I consider this your Acked-by?

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1346085 — Re: [PATCH] uprobes: wait for mmap_sem for write killable

On 02/29, Michal Hocko wrote:
>
> On Mon 29-02-16 19:11:06, Oleg Nesterov wrote:
> > On 02/29, Michal Hocko wrote:
> > >
> > > --- a/kernel/events/uprobes.c
> > > +++ b/kernel/events/uprobes.c
> > > @@ -1130,7 +1130,9 @@ static int xol_add_vma(struct mm_struct *mm, struct xol_area *area)
> > >  	struct vm_area_struct *vma;
> > >  	int ret;
> > >
> > > -	down_write(&mm->mmap_sem);
> > > +	if (down_write_killable(&mm->mmap_sem))
> > > +		return -EINTR;
> > > +
> > >  	if (mm->uprobes_state.xol_area) {
> > >  		ret = -EALREADY;
> > >  		goto fail;
> > > @@ -1468,7 +1470,8 @@ static void dup_xol_work(struct callback_head *work)
> > >  	if (current->flags & PF_EXITING)
> > >  		return;
> > >
> > > -	if (!__create_xol_area(current->utask->dup_xol_addr))
> > > +	if (!__create_xol_area(current->utask->dup_xol_addr) &&
> > > +			!fatal_signal_pending(current)
> > >  		uprobe_warn(current, "dup xol area");
> > >  }
> >
> > Looks good, thanks.
>
> Can I consider this your Acked-by?

Yes, feel free to add. I forgot to add it.

Oleg.

[toc] | [prev] | [next] | [standalone]

#1345813 — [PATCH 08/18] mm, fork: make dup_mmap wait for mmap_sem for write killable

From: Michal Hocko <mhocko@suse.com>

dup_mmap needs to lock current's mm mmap_sem for write. If the waiting
task gets killed by the oom killer it would block oom_reaper from
asynchronous address space reclaim and reduce the chances of timely OOM
resolving. Wait for the lock in the killable mode and return with EINTR
if the task got killed while waiting.

Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Konstantin Khlebnikov <koct9i@gmail.com>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 kernel/fork.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index d277e83ed3e0..e064bc8453dc 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -413,7 +413,10 @@ static int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
 	unsigned long charge;
 
 	uprobe_start_dup_mmap();
-	down_write(&oldmm->mmap_sem);
+	if (down_write_killable(&oldmm->mmap_sem)) {
+		uprobe_end_dup_mmap();
+		return -EINTR;
+	}
 	flush_cache_dup_mm(oldmm);
 	uprobe_dup_mmap(oldmm, mm);
 	/*
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1346046 — Re: [PATCH 08/18] mm, fork: make dup_mmap wait for mmap_sem for write killable

On 02/29, Michal Hocko wrote:
>
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -413,7 +413,10 @@ static int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
>  	unsigned long charge;
>  
>  	uprobe_start_dup_mmap();
> -	down_write(&oldmm->mmap_sem);
> +	if (down_write_killable(&oldmm->mmap_sem)) {
> +		uprobe_end_dup_mmap();
> +		return -EINTR;
> +	}

This is really cosmetic and subjective, I won't insist if you prefer it this way.

But perhaps it makes sense to add another "fail" label above uprobe_end_dup_mmap()
we already have... IMO it is always better to avoid duplicating when it comes to
"unlock".

Oleg.

[toc] | [prev] | [next] | [standalone]

#1346060 — [PATCH] mm, fork: make dup_mmap wait for mmap_sem for write killable

From: Michal Hocko <mhocko@suse.com>

dup_mmap needs to lock current's mm mmap_sem for write. If the waiting
task gets killed by the oom killer it would block oom_reaper from
asynchronous address space reclaim and reduce the chances of timely OOM
resolving. Wait for the lock in the killable mode and return with EINTR
if the task got killed while waiting.

Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Konstantin Khlebnikov <koct9i@gmail.com>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 kernel/fork.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index d277e83ed3e0..139968026b76 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -413,7 +413,10 @@ static int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
 	unsigned long charge;
 
 	uprobe_start_dup_mmap();
-	down_write(&oldmm->mmap_sem);
+	if (down_write_killable(&oldmm->mmap_sem)) {
+		retval = -EINTR;
+		goto fail_uprobe_end;
+	}
 	flush_cache_dup_mm(oldmm);
 	uprobe_dup_mmap(oldmm, mm);
 	/*
@@ -525,6 +528,7 @@ static int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
 	up_write(&mm->mmap_sem);
 	flush_tlb_mm(oldmm);
 	up_write(&oldmm->mmap_sem);
+fail_uprobe_end:
 	uprobe_end_dup_mmap();
 	return retval;
 fail_nomem_anon_vma_fork:
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1346138 — Re: [PATCH] mm, fork: make dup_mmap wait for mmap_sem for write killable

[Multipart message — attachments visible in raw view] — view raw

Hi Michal,

[auto build test ERROR on v4.5-rc6]
[also build test ERROR on next-20160229]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Michal-Hocko/mm-fork-make-dup_mmap-wait-for-mmap_sem-for-write-killable/20160301-021107
config: x86_64-lkp (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   kernel/fork.c: In function 'dup_mmap':
>> kernel/fork.c:405:2: error: implicit declaration of function 'down_write_killable' [-Werror=implicit-function-declaration]
     if (down_write_killable(&oldmm->mmap_sem)) {
     ^
   cc1: some warnings being treated as errors

vim +/down_write_killable +405 kernel/fork.c

   399		struct vm_area_struct *mpnt, *tmp, *prev, **pprev;
   400		struct rb_node **rb_link, *rb_parent;
   401		int retval;
   402		unsigned long charge;
   403	
   404		uprobe_start_dup_mmap();
 > 405		if (down_write_killable(&oldmm->mmap_sem)) {
   406			retval = -EINTR;
   407			goto fail_uprobe_end;
   408		}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1345814 — [PATCH 10/18] vdso: make arch_setup_additional_pages wait for mmap_sem for write killable

From: Michal Hocko <mhocko@suse.com>

most architectures are relying on mmap_sem for write in their
arch_setup_additional_pages. If the waiting task gets killed by the oom
killer it would block oom_reaper from asynchronous address space reclaim
and reduce the chances of timely OOM resolving. Wait for the lock in
the killable mode and return with EINTR if the task got killed while
waiting.

Cc: linux-arch@vger.kernel.org
Cc: Andy Lutomirski <luto@amacapital.net>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 arch/arm/kernel/process.c          | 3 ++-
 arch/arm64/kernel/vdso.c           | 6 ++++--
 arch/hexagon/kernel/vdso.c         | 3 ++-
 arch/mips/kernel/vdso.c            | 3 ++-
 arch/powerpc/kernel/vdso.c         | 3 ++-
 arch/s390/kernel/vdso.c            | 3 ++-
 arch/sh/kernel/vsyscall/vsyscall.c | 4 +++-
 arch/x86/entry/vdso/vma.c          | 3 ++-
 arch/x86/um/vdso/vma.c             | 3 ++-
 9 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 4adfb46e3ee9..94cccae090fa 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -420,7 +420,8 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
 	npages = 1; /* for sigpage */
 	npages += vdso_total_pages;
 
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
 	hint = sigpage_addr(mm, npages);
 	addr = get_unmapped_area(NULL, hint, npages << PAGE_SHIFT, 0, 0);
 	if (IS_ERR_VALUE(addr)) {
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index 97bc68f4c689..d7423dfb4a4a 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -95,7 +95,8 @@ int aarch32_setup_vectors_page(struct linux_binprm *bprm, int uses_interp)
 	};
 	void *ret;
 
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
 	current->mm->context.vdso = (void *)addr;
 
 	/* Map vectors page at the high address. */
@@ -163,7 +164,8 @@ int arch_setup_additional_pages(struct linux_binprm *bprm,
 	/* Be sure to map the data page */
 	vdso_mapping_len = vdso_text_len + PAGE_SIZE;
 
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
 	vdso_base = get_unmapped_area(NULL, 0, vdso_mapping_len, 0, 0);
 	if (IS_ERR_VALUE(vdso_base)) {
 		ret = ERR_PTR(vdso_base);
diff --git a/arch/hexagon/kernel/vdso.c b/arch/hexagon/kernel/vdso.c
index 0bf5a87e4d0a..3ea968415539 100644
--- a/arch/hexagon/kernel/vdso.c
+++ b/arch/hexagon/kernel/vdso.c
@@ -65,7 +65,8 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
 	unsigned long vdso_base;
 	struct mm_struct *mm = current->mm;
 
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
 
 	/* Try to get it loaded right near ld.so/glibc. */
 	vdso_base = STACK_TOP;
diff --git a/arch/mips/kernel/vdso.c b/arch/mips/kernel/vdso.c
index 975e99759bab..54e1663ce639 100644
--- a/arch/mips/kernel/vdso.c
+++ b/arch/mips/kernel/vdso.c
@@ -104,7 +104,8 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
 	struct resource gic_res;
 	int ret;
 
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
 
 	/*
 	 * Determine total area size. This includes the VDSO data itself, the
diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index def1b8b5e6c1..6767605ea8da 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -195,7 +195,8 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
 	 * and end up putting it elsewhere.
 	 * Add enough to the size so that the result can be aligned.
 	 */
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
 	vdso_base = get_unmapped_area(NULL, vdso_base,
 				      (vdso_pages << PAGE_SHIFT) +
 				      ((VDSO_ALIGNMENT - 1) & PAGE_MASK),
diff --git a/arch/s390/kernel/vdso.c b/arch/s390/kernel/vdso.c
index 94495cac8be3..5904abf6b1ae 100644
--- a/arch/s390/kernel/vdso.c
+++ b/arch/s390/kernel/vdso.c
@@ -216,7 +216,8 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
 	 * it at vdso_base which is the "natural" base for it, but we might
 	 * fail and end up putting it elsewhere.
 	 */
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
 	vdso_base = get_unmapped_area(NULL, 0, vdso_pages << PAGE_SHIFT, 0, 0);
 	if (IS_ERR_VALUE(vdso_base)) {
 		rc = vdso_base;
diff --git a/arch/sh/kernel/vsyscall/vsyscall.c b/arch/sh/kernel/vsyscall/vsyscall.c
index ea2aa1393b87..cc0cc5b4ff18 100644
--- a/arch/sh/kernel/vsyscall/vsyscall.c
+++ b/arch/sh/kernel/vsyscall/vsyscall.c
@@ -64,7 +64,9 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
 	unsigned long addr;
 	int ret;
 
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
+
 	addr = get_unmapped_area(NULL, 0, PAGE_SIZE, 0, 0);
 	if (IS_ERR_VALUE(addr)) {
 		ret = addr;
diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c
index 10f704584922..69d861f67c47 100644
--- a/arch/x86/entry/vdso/vma.c
+++ b/arch/x86/entry/vdso/vma.c
@@ -174,7 +174,8 @@ static int map_vdso(const struct vdso_image *image, bool calculate_addr)
 		addr = 0;
 	}
 
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
 
 	addr = get_unmapped_area(NULL, addr,
 				 image->size - image->sym_vvar_start, 0, 0);
diff --git a/arch/x86/um/vdso/vma.c b/arch/x86/um/vdso/vma.c
index 237c6831e095..6be22f991b59 100644
--- a/arch/x86/um/vdso/vma.c
+++ b/arch/x86/um/vdso/vma.c
@@ -61,7 +61,8 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
 	if (!vdso_enabled)
 		return 0;
 
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
 
 	err = install_special_mapping(mm, um_vdso_addr, PAGE_SIZE,
 		VM_READ|VM_EXEC|
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web