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 2 of 2 — ← Prev page 1 [2]

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

On Mon, Feb 29, 2016 at 5:26 AM, Michal Hocko <mhocko@kernel.org> wrote:
> 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.

Acked-by: Andy Lutomirski <luto@kernel.org> # for the x86 vdso

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

#1345817 — [PATCH 06/18] mm: make vm_brk killable

From: Michal Hocko <mhocko@suse.com>

Now that all the callers handle vm_brk failure we can change it
wait for mmap_sem killable to help oom_reaper to not get blocked
just because vm_brk gets blocked behind mmap_sem readers.

Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/mm.h | 2 +-
 mm/mmap.c          | 9 +++------
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 4ee6a3561540..dccaea8682f2 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2077,7 +2077,7 @@ static inline void mm_populate(unsigned long addr, unsigned long len) {}
 #endif
 
 /* These take the mm semaphore themselves */
-extern unsigned long vm_brk(unsigned long, unsigned long);
+extern unsigned long __must_check vm_brk(unsigned long, unsigned long);
 extern int vm_munmap(unsigned long, size_t);
 extern unsigned long __must_check vm_mmap(struct file *, unsigned long,
         unsigned long, unsigned long,
diff --git a/mm/mmap.c b/mm/mmap.c
index 5d33c841e3a2..3f264fb14118 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2718,12 +2718,9 @@ unsigned long vm_brk(unsigned long addr, unsigned long len)
 	unsigned long ret;
 	bool populate;
 
-	/*
-	 * XXX not all users are chcecking the return value, convert
-	 * to down_write_killable after they are able to cope with
-	 * error
-	 */
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
+
 	ret = do_brk(addr, len);
 	populate = ((mm->def_flags & VM_LOCKED) != 0);
 	up_write(&mm->mmap_sem);
-- 
2.7.0

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

#1345818 — [PATCH 18/18] drm/amdgpu: make amdgpu_mn_get wait for mmap_sem killable

From: Michal Hocko <mhocko@suse.com>

amdgpu_mn_get which is called during ioct path relies on 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: David Airlie <airlied@linux.ie>
Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index d7ec9bd6755f..6f44f1c23be3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -181,7 +181,10 @@ static struct amdgpu_mn *amdgpu_mn_get(struct amdgpu_device *adev)
 	int r;
 
 	mutex_lock(&adev->mn_lock);
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem)) {
+		mutex_unlock(&adev->mn_lock);
+		return -EINTR;
+	}
 
 	hash_for_each_possible(adev->mn_hash, rmn, node, (unsigned long)mm)
 		if (rmn->mm == mm)
-- 
2.7.0

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

#1345820 — [PATCH 05/18] mm, elf: handle vm_brk error

From: Michal Hocko <mhocko@suse.com>

load_elf_library doesn't handle vm_brk failure although nothing really
indicates it cannot do that because the function is allowed to fail
due to vm_mmap failures already. This might be not a problem now
but later patch will make vm_brk killable (resp. mmap_sem for write
waiting will become killable) and so the failure will be more probable.

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

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 7d914c67a9d0..2e45ae57ea88 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1176,8 +1176,11 @@ static int load_elf_library(struct file *file)
 	len = ELF_PAGESTART(eppnt->p_filesz + eppnt->p_vaddr +
 			    ELF_MIN_ALIGN - 1);
 	bss = eppnt->p_memsz + eppnt->p_vaddr;
-	if (bss > len)
-		vm_brk(len, bss - len);
+	if (bss > len) {
+		error = vm_brk(len, bss - len);
+		if (BAD_ADDR(error))
+			goto out_free_ph;
+	}
 	error = 0;
 
 out_free_ph:
-- 
2.7.0

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

#1345821 — [PATCH 16/18] drm/i915: make i915_gem_mmap_ioctl wait for mmap_sem killable

From: Michal Hocko <mhocko@suse.com>

i915_gem_mmap_ioctl relies on 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: Daniel Vetter <daniel.vetter@intel.com>
Cc: David Airlie <airlied@linux.ie>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f68f34606f2f..a50136cbd332 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1754,7 +1754,10 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
 		struct mm_struct *mm = current->mm;
 		struct vm_area_struct *vma;
 
-		down_write(&mm->mmap_sem);
+		if (down_write_killable(&mm->mmap_sem)) {
+			drm_gem_object_unreference_unlocked(obj);
+			return -EINTR;
+		}
 		vma = find_vma(mm, addr);
 		if (vma)
 			vma->vm_page_prot =
-- 
2.7.0

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

#1345822 — [PATCH 12/18] aio: make aio_setup_ring killable

From: Michal Hocko <mhocko@suse.com>

aio_setup_ring waits for mmap_sem in writable mode. 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. This will also expedite
the return to the userspace and do_exit.

Cc: Benjamin LaHaise <bcrl@kvack.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 fs/aio.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/aio.c b/fs/aio.c
index 56bcdf4105f4..1c2e7e2c1b2b 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -520,7 +520,12 @@ static int aio_setup_ring(struct kioctx *ctx)
 	ctx->mmap_size = nr_pages * PAGE_SIZE;
 	pr_debug("attempting mmap of %lu bytes\n", ctx->mmap_size);
 
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem)) {
+		ctx->mmap_size = 0;
+		aio_free_ring(ctx);
+		return -EINTR;
+	}
+
 	ctx->mmap_base = do_mmap_pgoff(ctx->aio_ring_file, 0, ctx->mmap_size,
 				       PROT_READ | PROT_WRITE,
 				       MAP_SHARED, 0, &unused);
-- 
2.7.0

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

#1345948 — Re: [PATCH 12/18] aio: make aio_setup_ring killable

Michal Hocko <mhocko@kernel.org> writes:

> From: Michal Hocko <mhocko@suse.com>
>
> aio_setup_ring waits for mmap_sem in writable mode. 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. This will also expedite
> the return to the userspace and do_exit.
>
> Cc: Benjamin LaHaise <bcrl@kvack.org>
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
>  fs/aio.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 56bcdf4105f4..1c2e7e2c1b2b 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -520,7 +520,12 @@ static int aio_setup_ring(struct kioctx *ctx)
>  	ctx->mmap_size = nr_pages * PAGE_SIZE;
>  	pr_debug("attempting mmap of %lu bytes\n", ctx->mmap_size);
>  
> -	down_write(&mm->mmap_sem);
> +	if (down_write_killable(&mm->mmap_sem)) {
> +		ctx->mmap_size = 0;
> +		aio_free_ring(ctx);
> +		return -EINTR;
> +	}
> +
>  	ctx->mmap_base = do_mmap_pgoff(ctx->aio_ring_file, 0, ctx->mmap_size,
>  				       PROT_READ | PROT_WRITE,
>  				       MAP_SHARED, 0, &unused);

Reviewed-by: Jeff Moyer <jmoyer@redhat.com>

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

#1345823 — [PATCH 01/18] mm: Make mmap_sem for write waits killable for mm syscalls

From: Michal Hocko <mhocko@suse.com>

This is the first step in making mmap_sem write holders killable. It
focuses on the trivial ones which are taking the lock early after
entering the syscall and they are not changing state before.

Therefore it is very easy to change them to use down_write_killable
and immediately return with -EINTR. This will allow the waiter to
pass away without blocking the mmap_sem which might be required to
make a forward progress. E.g. the oom reaper will need the lock for
reading to dismantle the OOM victim address space.

The only tricky function in this patch is vm_mmap_pgoff which has many
call sites via vm_mmap. To reduce the risk keep vm_mmap with the
original non-killable semantic for now.

vm_munmap callers do not bother checking the return value so open code
it into the munmap syscall path for now for simplicity.

Cc: Mel Gorman <mgorman@suse.de>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Konstantin Khlebnikov <koct9i@gmail.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 mm/internal.h |  5 +++--
 mm/madvise.c  |  8 +++++---
 mm/mlock.c    | 16 ++++++++++------
 mm/mmap.c     | 27 +++++++++++++++++++++++----
 mm/mprotect.c |  3 ++-
 mm/mremap.c   |  3 ++-
 mm/nommu.c    |  2 +-
 mm/util.c     | 12 +++++++++---
 8 files changed, 55 insertions(+), 21 deletions(-)

diff --git a/mm/internal.h b/mm/internal.h
index 72bbce3efc36..26576cb3247e 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -444,9 +444,10 @@ extern u64 hwpoison_filter_flags_value;
 extern u64 hwpoison_filter_memcg;
 extern u32 hwpoison_filter_enable;
 
-extern unsigned long vm_mmap_pgoff(struct file *, unsigned long,
+extern unsigned long  __must_check vm_mmap_pgoff(struct file *, unsigned long,
         unsigned long, unsigned long,
-        unsigned long, unsigned long);
+        unsigned long, unsigned long,
+        bool);
 
 extern void set_pageblock_order(void);
 unsigned long reclaim_clean_pages_from_list(struct zone *zone,
diff --git a/mm/madvise.c b/mm/madvise.c
index a50ac1885d09..b1809a409a77 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -764,10 +764,12 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
 		return error;
 
 	write = madvise_need_mmap_write(behavior);
-	if (write)
-		down_write(&current->mm->mmap_sem);
-	else
+	if (write) {
+		if (down_write_killable(&current->mm->mmap_sem))
+			return -EINTR;
+	} else {
 		down_read(&current->mm->mmap_sem);
+	}
 
 	blk_start_plug(&plug);
 
diff --git a/mm/mlock.c b/mm/mlock.c
index 96f001041928..ef8dc9f395c4 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -617,7 +617,7 @@ static int apply_vma_lock_flags(unsigned long start, size_t len,
 	return error;
 }
 
-static int do_mlock(unsigned long start, size_t len, vm_flags_t flags)
+static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t flags)
 {
 	unsigned long locked;
 	unsigned long lock_limit;
@@ -635,7 +635,8 @@ static int do_mlock(unsigned long start, size_t len, vm_flags_t flags)
 	lock_limit >>= PAGE_SHIFT;
 	locked = len >> PAGE_SHIFT;
 
-	down_write(&current->mm->mmap_sem);
+	if (down_write_killable(&current->mm->mmap_sem))
+		return -EINTR;
 
 	locked += current->mm->locked_vm;
 
@@ -678,7 +679,8 @@ SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, len)
 	len = PAGE_ALIGN(len + (offset_in_page(start)));
 	start &= PAGE_MASK;
 
-	down_write(&current->mm->mmap_sem);
+	if (down_write_killable(&current->mm->mmap_sem))
+		return -EINTR;
 	ret = apply_vma_lock_flags(start, len, 0);
 	up_write(&current->mm->mmap_sem);
 
@@ -748,9 +750,10 @@ SYSCALL_DEFINE1(mlockall, int, flags)
 	lock_limit = rlimit(RLIMIT_MEMLOCK);
 	lock_limit >>= PAGE_SHIFT;
 
-	ret = -ENOMEM;
-	down_write(&current->mm->mmap_sem);
+	if (down_write_killable(&current->mm->mmap_sem))
+		return -EINTR;
 
+	ret = -ENOMEM;
 	if (!(flags & MCL_CURRENT) || (current->mm->total_vm <= lock_limit) ||
 	    capable(CAP_IPC_LOCK))
 		ret = apply_mlockall_flags(flags);
@@ -765,7 +768,8 @@ SYSCALL_DEFINE0(munlockall)
 {
 	int ret;
 
-	down_write(&current->mm->mmap_sem);
+	if (down_write_killable(&current->mm->mmap_sem))
+		return -EINTR;
 	ret = apply_mlockall_flags(0);
 	up_write(&current->mm->mmap_sem);
 	return ret;
diff --git a/mm/mmap.c b/mm/mmap.c
index b1e30132eb52..a8ea76c22bb6 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -182,7 +182,8 @@ SYSCALL_DEFINE1(brk, unsigned long, brk)
 	unsigned long min_brk;
 	bool populate;
 
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
 
 #ifdef CONFIG_COMPAT_BRK
 	/*
@@ -1336,7 +1337,7 @@ SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
 
 	flags &= ~(MAP_EXECUTABLE | MAP_DENYWRITE);
 
-	retval = vm_mmap_pgoff(file, addr, len, prot, flags, pgoff);
+	retval = vm_mmap_pgoff(file, addr, len, prot, flags, pgoff, true);
 out_fput:
 	if (file)
 		fput(file);
@@ -2498,6 +2499,10 @@ int vm_munmap(unsigned long start, size_t len)
 	int ret;
 	struct mm_struct *mm = current->mm;
 
+	/*
+	 * XXX convert to down_write_killable as soon as all users are able
+	 * to handle the error.
+	 */
 	down_write(&mm->mmap_sem);
 	ret = do_munmap(mm, start, len);
 	up_write(&mm->mmap_sem);
@@ -2507,8 +2512,15 @@ EXPORT_SYMBOL(vm_munmap);
 
 SYSCALL_DEFINE2(munmap, unsigned long, addr, size_t, len)
 {
+	int ret;
+	struct mm_struct *mm = current->mm;
+
 	profile_munmap(addr);
-	return vm_munmap(addr, len);
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
+	ret = do_munmap(mm, addr, len);
+	up_write(&mm->mmap_sem);
+	return ret;
 }
 
 
@@ -2541,7 +2553,9 @@ SYSCALL_DEFINE5(remap_file_pages, unsigned long, start, unsigned long, size,
 	if (pgoff + (size >> PAGE_SHIFT) < pgoff)
 		return ret;
 
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
+
 	vma = find_vma(mm, start);
 
 	if (!vma || !(vma->vm_flags & VM_SHARED))
@@ -2706,6 +2720,11 @@ unsigned long vm_brk(unsigned long addr, unsigned long len)
 	unsigned long ret;
 	bool populate;
 
+	/*
+	 * XXX not all users are chcecking the return value, convert
+	 * to down_write_killable after they are able to cope with
+	 * error
+	 */
 	down_write(&mm->mmap_sem);
 	ret = do_brk(addr, len);
 	populate = ((mm->def_flags & VM_LOCKED) != 0);
diff --git a/mm/mprotect.c b/mm/mprotect.c
index bf44caea638b..ccd0044f74bd 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -378,7 +378,8 @@ SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t, len,
 
 	reqprot = prot;
 
-	down_write(&current->mm->mmap_sem);
+	if (down_write_killable(&current->mm->mmap_sem))
+		return -EINTR;
 
 	vma = find_vma(current->mm, start);
 	error = -ENOMEM;
diff --git a/mm/mremap.c b/mm/mremap.c
index 3fa0a467df66..b635ce0ddfb4 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -502,7 +502,8 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
 	if (!new_len)
 		return ret;
 
-	down_write(&current->mm->mmap_sem);
+	if (down_write_killable(&current->mm->mmap_sem))
+		return -EINTR;
 
 	if (flags & MREMAP_FIXED) {
 		ret = mremap_to(addr, old_len, new_addr, new_len,
diff --git a/mm/nommu.c b/mm/nommu.c
index de8b6b6580c1..50b1d32921c2 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -1448,7 +1448,7 @@ SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
 
 	flags &= ~(MAP_EXECUTABLE | MAP_DENYWRITE);
 
-	retval = vm_mmap_pgoff(file, addr, len, prot, flags, pgoff);
+	retval = vm_mmap_pgoff(file, addr, len, prot, flags, pgoff, true);
 
 	if (file)
 		fput(file);
diff --git a/mm/util.c b/mm/util.c
index 3bf315fefeb6..98eeec742254 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -319,7 +319,7 @@ EXPORT_SYMBOL_GPL(get_user_pages_fast);
 
 unsigned long vm_mmap_pgoff(struct file *file, unsigned long addr,
 	unsigned long len, unsigned long prot,
-	unsigned long flag, unsigned long pgoff)
+	unsigned long flag, unsigned long pgoff, bool killable)
 {
 	unsigned long ret;
 	struct mm_struct *mm = current->mm;
@@ -327,7 +327,12 @@ unsigned long vm_mmap_pgoff(struct file *file, unsigned long addr,
 
 	ret = security_mmap_file(file, prot, flag);
 	if (!ret) {
-		down_write(&mm->mmap_sem);
+		if (killable) {
+			if (down_write_killable(&mm->mmap_sem))
+				return -EINTR;
+		} else {
+			down_write(&mm->mmap_sem);
+		}
 		ret = do_mmap_pgoff(file, addr, len, prot, flag, pgoff,
 				    &populate);
 		up_write(&mm->mmap_sem);
@@ -337,6 +342,7 @@ unsigned long vm_mmap_pgoff(struct file *file, unsigned long addr,
 	return ret;
 }
 
+/* XXX are all callers checking an error */
 unsigned long vm_mmap(struct file *file, unsigned long addr,
 	unsigned long len, unsigned long prot,
 	unsigned long flag, unsigned long offset)
@@ -346,7 +352,7 @@ unsigned long vm_mmap(struct file *file, unsigned long addr,
 	if (unlikely(offset_in_page(offset)))
 		return -EINVAL;
 
-	return vm_mmap_pgoff(file, addr, len, prot, flag, offset >> PAGE_SHIFT);
+	return vm_mmap_pgoff(file, addr, len, prot, flag, offset >> PAGE_SHIFT, false);
 }
 EXPORT_SYMBOL(vm_mmap);
 
-- 
2.7.0

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

#1345838 — Re: [PATCH 01/18] mm: Make mmap_sem for write waits killable for mm syscalls

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

Hi Michal,

[auto build test WARNING on next-20160229]
[also build test WARNING on v4.5-rc6]
[cannot apply to drm/drm-next drm-intel/for-linux-next v4.5-rc6 v4.5-rc5 v4.5-rc4]
[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/change-mmap_sem-taken-for-write-killable/20160229-213258
config: i386-randconfig-x003-201609 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from include/asm-generic/bug.h:4:0,
                    from arch/x86/include/asm/bug.h:35,
                    from include/linux/bug.h:4,
                    from include/linux/mmdebug.h:4,
                    from include/linux/mm.h:8,
                    from mm/util.c:1:
   mm/util.c: In function 'vm_mmap_pgoff':
   mm/util.c:331:8: error: implicit declaration of function 'down_write_killable' [-Werror=implicit-function-declaration]
       if (down_write_killable(&mm->mmap_sem))
           ^
   include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^
>> mm/util.c:331:4: note: in expansion of macro 'if'
       if (down_write_killable(&mm->mmap_sem))
       ^
   cc1: some warnings being treated as errors
--
   In file included from include/uapi/linux/stddef.h:1:0,
                    from include/linux/stddef.h:4,
                    from include/uapi/linux/posix_types.h:4,
                    from include/uapi/linux/types.h:13,
                    from include/linux/types.h:5,
                    from include/uapi/linux/capability.h:16,
                    from include/linux/capability.h:15,
                    from mm/mlock.c:8:
   mm/mlock.c: In function 'do_mlock':
   mm/mlock.c:638:6: error: implicit declaration of function 'down_write_killable' [-Werror=implicit-function-declaration]
     if (down_write_killable(&current->mm->mmap_sem))
         ^
   include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^
>> mm/mlock.c:638:2: note: in expansion of macro 'if'
     if (down_write_killable(&current->mm->mmap_sem))
     ^
   cc1: some warnings being treated as errors
--
   In file included from include/linux/linkage.h:4:0,
                    from include/linux/kernel.h:6,
                    from mm/mmap.c:11:
   mm/mmap.c: In function 'SYSC_brk':
   mm/mmap.c:185:6: error: implicit declaration of function 'down_write_killable' [-Werror=implicit-function-declaration]
     if (down_write_killable(&mm->mmap_sem))
         ^
   include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^
>> mm/mmap.c:185:2: note: in expansion of macro 'if'
     if (down_write_killable(&mm->mmap_sem))
     ^
   cc1: some warnings being treated as errors
--
   In file included from include/asm-generic/bug.h:4:0,
                    from arch/x86/include/asm/bug.h:35,
                    from include/linux/bug.h:4,
                    from include/linux/mmdebug.h:4,
                    from include/linux/mm.h:8,
                    from mm/mprotect.c:11:
   mm/mprotect.c: In function 'SYSC_mprotect':
   mm/mprotect.c:381:6: error: implicit declaration of function 'down_write_killable' [-Werror=implicit-function-declaration]
     if (down_write_killable(&current->mm->mmap_sem))
         ^
   include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^
>> mm/mprotect.c:381:2: note: in expansion of macro 'if'
     if (down_write_killable(&current->mm->mmap_sem))
     ^
   cc1: some warnings being treated as errors
--
   In file included from include/asm-generic/bug.h:4:0,
                    from arch/x86/include/asm/bug.h:35,
                    from include/linux/bug.h:4,
                    from include/linux/mmdebug.h:4,
                    from include/linux/mm.h:8,
                    from mm/mremap.c:10:
   mm/mremap.c: In function 'SYSC_mremap':
   mm/mremap.c:505:6: error: implicit declaration of function 'down_write_killable' [-Werror=implicit-function-declaration]
     if (down_write_killable(&current->mm->mmap_sem))
         ^
   include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^
>> mm/mremap.c:505:2: note: in expansion of macro 'if'
     if (down_write_killable(&current->mm->mmap_sem))
     ^
   cc1: some warnings being treated as errors
--
   In file included from include/asm-generic/bug.h:4:0,
                    from arch/x86/include/asm/bug.h:35,
                    from include/linux/bug.h:4,
                    from include/linux/mmdebug.h:4,
                    from include/linux/mm.h:8,
                    from include/linux/mman.h:4,
                    from mm/madvise.c:8:
   mm/madvise.c: In function 'SYSC_madvise':
   mm/madvise.c:768:7: error: implicit declaration of function 'down_write_killable' [-Werror=implicit-function-declaration]
      if (down_write_killable(&current->mm->mmap_sem))
          ^
   include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^
>> mm/madvise.c:768:3: note: in expansion of macro 'if'
      if (down_write_killable(&current->mm->mmap_sem))
      ^
   cc1: some warnings being treated as errors

vim +/if +331 mm/util.c

   315	{
   316		return get_user_pages_unlocked(start, nr_pages, write, 0, pages);
   317	}
   318	EXPORT_SYMBOL_GPL(get_user_pages_fast);
   319	
   320	unsigned long vm_mmap_pgoff(struct file *file, unsigned long addr,
   321		unsigned long len, unsigned long prot,
   322		unsigned long flag, unsigned long pgoff, bool killable)
   323	{
   324		unsigned long ret;
   325		struct mm_struct *mm = current->mm;
   326		unsigned long populate;
   327	
   328		ret = security_mmap_file(file, prot, flag);
   329		if (!ret) {
   330			if (killable) {
 > 331				if (down_write_killable(&mm->mmap_sem))
   332					return -EINTR;
   333			} else {
   334				down_write(&mm->mmap_sem);
   335			}
   336			ret = do_mmap_pgoff(file, addr, len, prot, flag, pgoff,
   337					    &populate);
   338			up_write(&mm->mmap_sem);
   339			if (populate)

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

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

#1345824 — [PATCH 09/18] ipc, shm: make shmem attach/detach wait for mmap_sem killable

From: Michal Hocko <mhocko@suse.com>

shmat and shmdt rely on 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: Davidlohr Bueso <dave@stgolabs.net>
Cc: Hugh Dickins <hughd@google.com>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 ipc/shm.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/ipc/shm.c b/ipc/shm.c
index 331fc1b0b3c7..b8cfa05940d2 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -1200,7 +1200,11 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr,
 	if (err)
 		goto out_fput;
 
-	down_write(&current->mm->mmap_sem);
+	if (down_write_killable(&current->mm->mmap_sem)) {
+		err = -EINVAL;
+		goto out_fput;
+	}
+
 	if (addr && !(shmflg & SHM_REMAP)) {
 		err = -EINVAL;
 		if (addr + size < addr)
@@ -1271,7 +1275,8 @@ SYSCALL_DEFINE1(shmdt, char __user *, shmaddr)
 	if (addr & ~PAGE_MASK)
 		return retval;
 
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
 
 	/*
 	 * This function tries to be smart and unmap shm segments that
-- 
2.7.0

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

#1345826 — [PATCH 02/18] mm: make vm_mmap killable

From: Michal Hocko <mhocko@suse.com>

All the callers of vm_mmap seem to check for the failure already
and bail out in one way or another on the error which means that
we can change it to use killable version of vm_mmap_pgoff and return
-EINTR if the current task gets killed while waiting for mmap_sem.
This also means that vm_mmap_pgoff can be killable by default and
drop the additional parameter.

This will help in the OOM conditions when the oom victim might be stuck
waiting for the mmap_sem for write which in turn can block oom_reaper
which relies on the mmap_sem for read to make a forward progress
and reclaim the address space of the victim.

Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/mm.h |  2 +-
 mm/internal.h      |  3 +--
 mm/mmap.c          |  2 +-
 mm/nommu.c         |  2 +-
 mm/util.c          | 13 ++++---------
 5 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 8a84cf07da8c..4ee6a3561540 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2079,7 +2079,7 @@ static inline void mm_populate(unsigned long addr, unsigned long len) {}
 /* These take the mm semaphore themselves */
 extern unsigned long vm_brk(unsigned long, unsigned long);
 extern int vm_munmap(unsigned long, size_t);
-extern unsigned long vm_mmap(struct file *, unsigned long,
+extern unsigned long __must_check vm_mmap(struct file *, unsigned long,
         unsigned long, unsigned long,
         unsigned long, unsigned long);
 
diff --git a/mm/internal.h b/mm/internal.h
index 26576cb3247e..d8d4c32bfce4 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -446,8 +446,7 @@ extern u32 hwpoison_filter_enable;
 
 extern unsigned long  __must_check vm_mmap_pgoff(struct file *, unsigned long,
         unsigned long, unsigned long,
-        unsigned long, unsigned long,
-        bool);
+        unsigned long, unsigned long);
 
 extern void set_pageblock_order(void);
 unsigned long reclaim_clean_pages_from_list(struct zone *zone,
diff --git a/mm/mmap.c b/mm/mmap.c
index a8ea76c22bb6..4e1f852a52ff 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1337,7 +1337,7 @@ SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
 
 	flags &= ~(MAP_EXECUTABLE | MAP_DENYWRITE);
 
-	retval = vm_mmap_pgoff(file, addr, len, prot, flags, pgoff, true);
+	retval = vm_mmap_pgoff(file, addr, len, prot, flags, pgoff);
 out_fput:
 	if (file)
 		fput(file);
diff --git a/mm/nommu.c b/mm/nommu.c
index 50b1d32921c2..de8b6b6580c1 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -1448,7 +1448,7 @@ SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
 
 	flags &= ~(MAP_EXECUTABLE | MAP_DENYWRITE);
 
-	retval = vm_mmap_pgoff(file, addr, len, prot, flags, pgoff, true);
+	retval = vm_mmap_pgoff(file, addr, len, prot, flags, pgoff);
 
 	if (file)
 		fput(file);
diff --git a/mm/util.c b/mm/util.c
index 98eeec742254..0e383fe48145 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -319,7 +319,7 @@ EXPORT_SYMBOL_GPL(get_user_pages_fast);
 
 unsigned long vm_mmap_pgoff(struct file *file, unsigned long addr,
 	unsigned long len, unsigned long prot,
-	unsigned long flag, unsigned long pgoff, bool killable)
+	unsigned long flag, unsigned long pgoff)
 {
 	unsigned long ret;
 	struct mm_struct *mm = current->mm;
@@ -327,12 +327,8 @@ unsigned long vm_mmap_pgoff(struct file *file, unsigned long addr,
 
 	ret = security_mmap_file(file, prot, flag);
 	if (!ret) {
-		if (killable) {
-			if (down_write_killable(&mm->mmap_sem))
-				return -EINTR;
-		} else {
-			down_write(&mm->mmap_sem);
-		}
+		if (down_write_killable(&mm->mmap_sem))
+			return -EINTR;
 		ret = do_mmap_pgoff(file, addr, len, prot, flag, pgoff,
 				    &populate);
 		up_write(&mm->mmap_sem);
@@ -342,7 +338,6 @@ unsigned long vm_mmap_pgoff(struct file *file, unsigned long addr,
 	return ret;
 }
 
-/* XXX are all callers checking an error */
 unsigned long vm_mmap(struct file *file, unsigned long addr,
 	unsigned long len, unsigned long prot,
 	unsigned long flag, unsigned long offset)
@@ -352,7 +347,7 @@ unsigned long vm_mmap(struct file *file, unsigned long addr,
 	if (unlikely(offset_in_page(offset)))
 		return -EINVAL;
 
-	return vm_mmap_pgoff(file, addr, len, prot, flag, offset >> PAGE_SHIFT, false);
+	return vm_mmap_pgoff(file, addr, len, prot, flag, offset >> PAGE_SHIFT);
 }
 EXPORT_SYMBOL(vm_mmap);
 
-- 
2.7.0

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

#1345827

On Mon 29-02-16 14:26:39, Michal Hocko wrote:
[...]
> 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).

Damnt it. I thought that git-send-email will not use the same CC list
for all patches and use it only for the cover if the particular patches
have their CC list. I am sorry for the excessive spamming!
-- 
Michal Hocko
SUSE Labs

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

#1345828 — [PATCH 14/18] prctl: make PR_SET_THP_DISABLE wait for mmap_sem killable

From: Michal Hocko <mhocko@suse.com>

PR_SET_THP_DISABLE 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.

Cc: Alex Thorlton <athorlton@sgi.com>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 kernel/sys.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/sys.c b/kernel/sys.c
index cf8ba545c7d3..89d5be418157 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2246,7 +2246,8 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 	case PR_SET_THP_DISABLE:
 		if (arg3 || arg4 || arg5)
 			return -EINVAL;
-		down_write(&me->mm->mmap_sem);
+		if (down_write_killable(&me->mm->mmap_sem))
+			return -EINTR;
 		if (arg2)
 			me->mm->def_flags |= VM_NOHUGEPAGE;
 		else
-- 
2.7.0

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

#1345829 — [PATCH 17/18] drm/radeon: make radeon_mn_get wait for mmap_sem killable

From: Michal Hocko <mhocko@suse.com>

radeon_mn_get which is called during ioct path relies on 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: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: David Airlie <airlied@linux.ie>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 drivers/gpu/drm/radeon/radeon_mn.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_mn.c b/drivers/gpu/drm/radeon/radeon_mn.c
index eef006c48584..896f2cf51e4e 100644
--- a/drivers/gpu/drm/radeon/radeon_mn.c
+++ b/drivers/gpu/drm/radeon/radeon_mn.c
@@ -186,7 +186,9 @@ static struct radeon_mn *radeon_mn_get(struct radeon_device *rdev)
 	struct radeon_mn *rmn;
 	int r;
 
-	down_write(&mm->mmap_sem);
+	if (down_write_killable(&mm->mmap_sem))
+		return ERR_PTR(-EINTR);
+
 	mutex_lock(&rdev->mn_lock);
 
 	hash_for_each_possible(rdev->mn_hash, rmn, node, (unsigned long)mm)
-- 
2.7.0

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

#1345858 — Re: [PATCH 17/18] drm/radeon: make radeon_mn_get wait for mmap_sem killable

[Dropping CCing the individual people, but adding the dri-devel mailing 
list as well instead].

Am 29.02.2016 um 14:26 schrieb Michal Hocko:
> From: Michal Hocko <mhocko@suse.com>
>
> radeon_mn_get which is called during ioct path relies on 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: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: David Airlie <airlied@linux.ie>
> Signed-off-by: Michal Hocko <mhocko@suse.com>

This one and patch #18 in this series are Reviewed-by: Christian König 
<christian.koenig@amd.com>.

Nice to see some improvements on this side,
Christian.

> ---
>   drivers/gpu/drm/radeon/radeon_mn.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_mn.c b/drivers/gpu/drm/radeon/radeon_mn.c
> index eef006c48584..896f2cf51e4e 100644
> --- a/drivers/gpu/drm/radeon/radeon_mn.c
> +++ b/drivers/gpu/drm/radeon/radeon_mn.c
> @@ -186,7 +186,9 @@ static struct radeon_mn *radeon_mn_get(struct radeon_device *rdev)
>   	struct radeon_mn *rmn;
>   	int r;
>   
> -	down_write(&mm->mmap_sem);
> +	if (down_write_killable(&mm->mmap_sem))
> +		return ERR_PTR(-EINTR);
> +
>   	mutex_lock(&rdev->mn_lock);
>   
>   	hash_for_each_possible(rdev->mn_hash, rmn, node, (unsigned long)mm)

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

#1345856

On Mon, Feb 29, 2016 at 02:26:39PM +0100, Michal Hocko wrote:
> 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.

What about effect on userspace? IIUC, we would have now EINTR returned
from bunch of syscall, which haven't had this errno on the table before.
Should we care?

> 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
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
 Kirill A. Shutemov

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

#1345860

On Mon 29-02-16 17:04:16, Kirill A. Shutemov wrote:
[...]
> > 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.
> 
> What about effect on userspace? IIUC, we would have now EINTR returned
> from bunch of syscall, which haven't had this errno on the table before.
> Should we care?

Those function will return EINTR only when the current was _killed_ when
we do not return to the userspace. So there shouldn't be any visible
effect.
-- 
Michal Hocko
SUSE Labs

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

#1345884

On Mon, Feb 29, 2016 at 03:16:22PM +0100, Michal Hocko wrote:
> On Mon 29-02-16 17:04:16, Kirill A. Shutemov wrote:
> [...]
> > > 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.
> > 
> > What about effect on userspace? IIUC, we would have now EINTR returned
> > from bunch of syscall, which haven't had this errno on the table before.
> > Should we care?
> 
> Those function will return EINTR only when the current was _killed_ when
> we do not return to the userspace. So there shouldn't be any visible
> effect.

Ah. I confused killable with interruptible.

-- 
 Kirill A. Shutemov

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web