All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrey Ryabinin <aryabinin@virtuozzo.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: <linux-kernel@vger.kernel.org>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	<stable@vger.kernel.org>, <penguin-kernel@I-love.SAKURA.ne.jp>,
	<mhocko@kernel.org>, <linux-mm@kvack.org>, <hpa@zytor.com>,
	<chris@chris-wilson.co.uk>, <hch@lst.de>, <mingo@elte.hu>,
	<jszhang@marvell.com>, <joelaf@google.com>, <joaodias@google.com>,
	<willy@infradead.org>, <tglx@linutronix.de>,
	<thellstrom@vmware.com>
Subject: [PATCH v2 1/5] mm/vmalloc: allow to call vfree() in atomic context
Date: Wed, 12 Apr 2017 15:49:01 +0300	[thread overview]
Message-ID: <20170412124905.25443-2-aryabinin@virtuozzo.com> (raw)
In-Reply-To: <20170412124905.25443-1-aryabinin@virtuozzo.com>

Commit 5803ed292e63 ("mm: mark all calls into the vmalloc subsystem
as potentially sleeping") added might_sleep() to remove_vm_area() from
vfree(), and commit 763b218ddfaf ("mm: add preempt points into
__purge_vmap_area_lazy()") actually made vfree() potentially sleeping.

This broke vmwgfx driver which calls vfree() under spin_lock().

    BUG: sleeping function called from invalid context at mm/vmalloc.c:1480
    in_atomic(): 1, irqs_disabled(): 0, pid: 341, name: plymouthd
    2 locks held by plymouthd/341:
     #0:  (drm_global_mutex){+.+.+.}, at: [<ffffffffc01c274b>] drm_release+0x3b/0x3b0 [drm]
     #1:  (&(&tfile->lock)->rlock){+.+...}, at: [<ffffffffc0173038>] ttm_object_file_release+0x28/0x90 [ttm]

    Call Trace:
     dump_stack+0x86/0xc3
     ___might_sleep+0x17d/0x250
     __might_sleep+0x4a/0x80
     remove_vm_area+0x22/0x90
     __vunmap+0x2e/0x110
     vfree+0x42/0x90
     kvfree+0x2c/0x40
     drm_ht_remove+0x1a/0x30 [drm]
     ttm_object_file_release+0x50/0x90 [ttm]
     vmw_postclose+0x47/0x60 [vmwgfx]
     drm_release+0x290/0x3b0 [drm]
     __fput+0xf8/0x210
     ____fput+0xe/0x10
     task_work_run+0x85/0xc0
     exit_to_usermode_loop+0xb4/0xc0
     do_syscall_64+0x185/0x1f0
     entry_SYSCALL64_slow_path+0x25/0x25

This can be fixed in vmgfx, but it would be better to make vfree()
non-sleeping again because we may have other bugs like this one.

__purge_vmap_area_lazy() is the only function in the vfree() path that
wants to be able to sleep. So it make sense to schedule
__purge_vmap_area_lazy() via schedule_work() so it runs only in sleepable
context. This will have a minimal effect on the regular vfree() path.
since __purge_vmap_area_lazy() is rarely called.

Fixes: 5803ed292e63 ("mm: mark all calls into the vmalloc subsystem as
                      potentially sleeping")
Reported-by: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Cc: <stable@vger.kernel.org>
---
 mm/vmalloc.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 8ef8ea1..1c74b26 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -701,7 +701,7 @@ static bool __purge_vmap_area_lazy(unsigned long start, unsigned long end)
  * Kick off a purge of the outstanding lazy areas. Don't bother if somebody
  * is already purging.
  */
-static void try_purge_vmap_area_lazy(void)
+static void try_purge_vmap_area_lazy(struct work_struct *work)
 {
 	if (mutex_trylock(&vmap_purge_lock)) {
 		__purge_vmap_area_lazy(ULONG_MAX, 0);
@@ -720,6 +720,8 @@ static void purge_vmap_area_lazy(void)
 	mutex_unlock(&vmap_purge_lock);
 }
 
+static DECLARE_WORK(purge_vmap_work, try_purge_vmap_area_lazy);
+
 /*
  * Free a vmap area, caller ensuring that the area has been unmapped
  * and flush_cache_vunmap had been called for the correct range
@@ -736,7 +738,7 @@ static void free_vmap_area_noflush(struct vmap_area *va)
 	llist_add(&va->purge_list, &vmap_purge_list);
 
 	if (unlikely(nr_lazy > lazy_max_pages()))
-		try_purge_vmap_area_lazy();
+		schedule_work(&purge_vmap_work);
 }
 
 /*
@@ -1125,7 +1127,6 @@ void vm_unmap_ram(const void *mem, unsigned int count)
 	unsigned long addr = (unsigned long)mem;
 	struct vmap_area *va;
 
-	might_sleep();
 	BUG_ON(!addr);
 	BUG_ON(addr < VMALLOC_START);
 	BUG_ON(addr > VMALLOC_END);
@@ -1477,8 +1478,6 @@ struct vm_struct *remove_vm_area(const void *addr)
 {
 	struct vmap_area *va;
 
-	might_sleep();
-
 	va = find_vmap_area((unsigned long)addr);
 	if (va && va->flags & VM_VM_AREA) {
 		struct vm_struct *vm = va->vm;
-- 
2.10.2

WARNING: multiple messages have this Message-ID (diff)
From: Andrey Ryabinin <aryabinin@virtuozzo.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: <linux-kernel@vger.kernel.org>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	<stable@vger.kernel.org>, <penguin-kernel@I-love.SAKURA.ne.jp>,
	<mhocko@kernel.org>, <linux-mm@kvack.org>, <hpa@zytor.com>,
	<chris@chris-wilson.co.uk>, <hch@lst.de>, <mingo@elte.hu>,
	<jszhang@marvell.com>, <joelaf@google.com>, <joaodias@google.com>,
	<willy@infradead.org>, <tglx@linutronix.de>,
	<thellstrom@vmware.com>
Subject: [PATCH v2 1/5] mm/vmalloc: allow to call vfree() in atomic context
Date: Wed, 12 Apr 2017 15:49:01 +0300	[thread overview]
Message-ID: <20170412124905.25443-2-aryabinin@virtuozzo.com> (raw)
In-Reply-To: <20170412124905.25443-1-aryabinin@virtuozzo.com>

Commit 5803ed292e63 ("mm: mark all calls into the vmalloc subsystem
as potentially sleeping") added might_sleep() to remove_vm_area() from
vfree(), and commit 763b218ddfaf ("mm: add preempt points into
__purge_vmap_area_lazy()") actually made vfree() potentially sleeping.

This broke vmwgfx driver which calls vfree() under spin_lock().

    BUG: sleeping function called from invalid context at mm/vmalloc.c:1480
    in_atomic(): 1, irqs_disabled(): 0, pid: 341, name: plymouthd
    2 locks held by plymouthd/341:
     #0:  (drm_global_mutex){+.+.+.}, at: [<ffffffffc01c274b>] drm_release+0x3b/0x3b0 [drm]
     #1:  (&(&tfile->lock)->rlock){+.+...}, at: [<ffffffffc0173038>] ttm_object_file_release+0x28/0x90 [ttm]

    Call Trace:
     dump_stack+0x86/0xc3
     ___might_sleep+0x17d/0x250
     __might_sleep+0x4a/0x80
     remove_vm_area+0x22/0x90
     __vunmap+0x2e/0x110
     vfree+0x42/0x90
     kvfree+0x2c/0x40
     drm_ht_remove+0x1a/0x30 [drm]
     ttm_object_file_release+0x50/0x90 [ttm]
     vmw_postclose+0x47/0x60 [vmwgfx]
     drm_release+0x290/0x3b0 [drm]
     __fput+0xf8/0x210
     ____fput+0xe/0x10
     task_work_run+0x85/0xc0
     exit_to_usermode_loop+0xb4/0xc0
     do_syscall_64+0x185/0x1f0
     entry_SYSCALL64_slow_path+0x25/0x25

This can be fixed in vmgfx, but it would be better to make vfree()
non-sleeping again because we may have other bugs like this one.

__purge_vmap_area_lazy() is the only function in the vfree() path that
wants to be able to sleep. So it make sense to schedule
__purge_vmap_area_lazy() via schedule_work() so it runs only in sleepable
context. This will have a minimal effect on the regular vfree() path.
since __purge_vmap_area_lazy() is rarely called.

Fixes: 5803ed292e63 ("mm: mark all calls into the vmalloc subsystem as
                      potentially sleeping")
Reported-by: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Cc: <stable@vger.kernel.org>
---
 mm/vmalloc.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 8ef8ea1..1c74b26 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -701,7 +701,7 @@ static bool __purge_vmap_area_lazy(unsigned long start, unsigned long end)
  * Kick off a purge of the outstanding lazy areas. Don't bother if somebody
  * is already purging.
  */
-static void try_purge_vmap_area_lazy(void)
+static void try_purge_vmap_area_lazy(struct work_struct *work)
 {
 	if (mutex_trylock(&vmap_purge_lock)) {
 		__purge_vmap_area_lazy(ULONG_MAX, 0);
@@ -720,6 +720,8 @@ static void purge_vmap_area_lazy(void)
 	mutex_unlock(&vmap_purge_lock);
 }
 
+static DECLARE_WORK(purge_vmap_work, try_purge_vmap_area_lazy);
+
 /*
  * Free a vmap area, caller ensuring that the area has been unmapped
  * and flush_cache_vunmap had been called for the correct range
@@ -736,7 +738,7 @@ static void free_vmap_area_noflush(struct vmap_area *va)
 	llist_add(&va->purge_list, &vmap_purge_list);
 
 	if (unlikely(nr_lazy > lazy_max_pages()))
-		try_purge_vmap_area_lazy();
+		schedule_work(&purge_vmap_work);
 }
 
 /*
@@ -1125,7 +1127,6 @@ void vm_unmap_ram(const void *mem, unsigned int count)
 	unsigned long addr = (unsigned long)mem;
 	struct vmap_area *va;
 
-	might_sleep();
 	BUG_ON(!addr);
 	BUG_ON(addr < VMALLOC_START);
 	BUG_ON(addr > VMALLOC_END);
@@ -1477,8 +1478,6 @@ struct vm_struct *remove_vm_area(const void *addr)
 {
 	struct vmap_area *va;
 
-	might_sleep();
-
 	va = find_vmap_area((unsigned long)addr);
 	if (va && va->flags & VM_VM_AREA) {
 		struct vm_struct *vm = va->vm;
-- 
2.10.2

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

WARNING: multiple messages have this Message-ID (diff)
From: Andrey Ryabinin <aryabinin@virtuozzo.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	stable@vger.kernel.org, penguin-kernel@I-love.SAKURA.ne.jp,
	mhocko@kernel.org, linux-mm@kvack.org, hpa@zytor.com,
	chris@chris-wilson.co.uk, hch@lst.de, mingo@elte.hu,
	jszhang@marvell.com, joelaf@google.com, joaodias@google.com,
	willy@infradead.org, tglx@linutronix.de, thellstrom@vmware.com
Subject: [PATCH v2 1/5] mm/vmalloc: allow to call vfree() in atomic context
Date: Wed, 12 Apr 2017 15:49:01 +0300	[thread overview]
Message-ID: <20170412124905.25443-2-aryabinin@virtuozzo.com> (raw)
In-Reply-To: <20170412124905.25443-1-aryabinin@virtuozzo.com>

Commit 5803ed292e63 ("mm: mark all calls into the vmalloc subsystem
as potentially sleeping") added might_sleep() to remove_vm_area() from
vfree(), and commit 763b218ddfaf ("mm: add preempt points into
__purge_vmap_area_lazy()") actually made vfree() potentially sleeping.

This broke vmwgfx driver which calls vfree() under spin_lock().

    BUG: sleeping function called from invalid context at mm/vmalloc.c:1480
    in_atomic(): 1, irqs_disabled(): 0, pid: 341, name: plymouthd
    2 locks held by plymouthd/341:
     #0:  (drm_global_mutex){+.+.+.}, at: [<ffffffffc01c274b>] drm_release+0x3b/0x3b0 [drm]
     #1:  (&(&tfile->lock)->rlock){+.+...}, at: [<ffffffffc0173038>] ttm_object_file_release+0x28/0x90 [ttm]

    Call Trace:
     dump_stack+0x86/0xc3
     ___might_sleep+0x17d/0x250
     __might_sleep+0x4a/0x80
     remove_vm_area+0x22/0x90
     __vunmap+0x2e/0x110
     vfree+0x42/0x90
     kvfree+0x2c/0x40
     drm_ht_remove+0x1a/0x30 [drm]
     ttm_object_file_release+0x50/0x90 [ttm]
     vmw_postclose+0x47/0x60 [vmwgfx]
     drm_release+0x290/0x3b0 [drm]
     __fput+0xf8/0x210
     ____fput+0xe/0x10
     task_work_run+0x85/0xc0
     exit_to_usermode_loop+0xb4/0xc0
     do_syscall_64+0x185/0x1f0
     entry_SYSCALL64_slow_path+0x25/0x25

This can be fixed in vmgfx, but it would be better to make vfree()
non-sleeping again because we may have other bugs like this one.

__purge_vmap_area_lazy() is the only function in the vfree() path that
wants to be able to sleep. So it make sense to schedule
__purge_vmap_area_lazy() via schedule_work() so it runs only in sleepable
context. This will have a minimal effect on the regular vfree() path.
since __purge_vmap_area_lazy() is rarely called.

Fixes: 5803ed292e63 ("mm: mark all calls into the vmalloc subsystem as
                      potentially sleeping")
Reported-by: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Cc: <stable@vger.kernel.org>
---
 mm/vmalloc.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 8ef8ea1..1c74b26 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -701,7 +701,7 @@ static bool __purge_vmap_area_lazy(unsigned long start, unsigned long end)
  * Kick off a purge of the outstanding lazy areas. Don't bother if somebody
  * is already purging.
  */
-static void try_purge_vmap_area_lazy(void)
+static void try_purge_vmap_area_lazy(struct work_struct *work)
 {
 	if (mutex_trylock(&vmap_purge_lock)) {
 		__purge_vmap_area_lazy(ULONG_MAX, 0);
@@ -720,6 +720,8 @@ static void purge_vmap_area_lazy(void)
 	mutex_unlock(&vmap_purge_lock);
 }
 
+static DECLARE_WORK(purge_vmap_work, try_purge_vmap_area_lazy);
+
 /*
  * Free a vmap area, caller ensuring that the area has been unmapped
  * and flush_cache_vunmap had been called for the correct range
@@ -736,7 +738,7 @@ static void free_vmap_area_noflush(struct vmap_area *va)
 	llist_add(&va->purge_list, &vmap_purge_list);
 
 	if (unlikely(nr_lazy > lazy_max_pages()))
-		try_purge_vmap_area_lazy();
+		schedule_work(&purge_vmap_work);
 }
 
 /*
@@ -1125,7 +1127,6 @@ void vm_unmap_ram(const void *mem, unsigned int count)
 	unsigned long addr = (unsigned long)mem;
 	struct vmap_area *va;
 
-	might_sleep();
 	BUG_ON(!addr);
 	BUG_ON(addr < VMALLOC_START);
 	BUG_ON(addr > VMALLOC_END);
@@ -1477,8 +1478,6 @@ struct vm_struct *remove_vm_area(const void *addr)
 {
 	struct vmap_area *va;
 
-	might_sleep();
-
 	va = find_vmap_area((unsigned long)addr);
 	if (va && va->flags & VM_VM_AREA) {
 		struct vm_struct *vm = va->vm;
-- 
2.10.2

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

  reply	other threads:[~2017-04-12 12:49 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-30 10:27 [PATCH 1/4] mm/vmalloc: allow to call vfree() in atomic context Andrey Ryabinin
2017-03-30 10:27 ` Andrey Ryabinin
2017-03-30 10:27 ` Andrey Ryabinin
2017-03-30 10:27 ` [PATCH 2/4] x86/ldt: use vfree() instead of vfree_atomic() Andrey Ryabinin
2017-03-30 10:27   ` Andrey Ryabinin
2017-03-31  8:05   ` Thomas Gleixner
2017-03-31  8:05     ` Thomas Gleixner
2017-03-30 10:27 ` [PATCH 3/4] kernel/fork: use vfree() instead of vfree_atomic() to free thread stack Andrey Ryabinin
2017-03-30 10:27   ` Andrey Ryabinin
2017-03-30 10:27 ` [PATCH 4/4] mm/vmalloc: remove vfree_atomic() Andrey Ryabinin
2017-03-30 10:27   ` Andrey Ryabinin
2017-03-30 17:18   ` Matthew Wilcox
2017-03-30 17:18     ` Matthew Wilcox
2017-03-30 15:27     ` Andrey Ryabinin
2017-03-30 15:27       ` Andrey Ryabinin
2017-04-04  9:40   ` Michal Hocko
2017-04-04  9:40     ` Michal Hocko
2017-03-30 12:00 ` [PATCH 1/4] mm/vmalloc: allow to call vfree() in atomic context Thomas Hellstrom
2017-03-30 12:00   ` Thomas Hellstrom
2017-03-30 12:00   ` Thomas Hellstrom
2017-03-30 14:48   ` Andrey Ryabinin
2017-03-30 14:48     ` Andrey Ryabinin
2017-03-30 14:48     ` Andrey Ryabinin
2017-03-30 15:04     ` Thomas Hellstrom
2017-03-30 15:04       ` Thomas Hellstrom
2017-03-30 15:04       ` Thomas Hellstrom
2017-04-04  9:41     ` Michal Hocko
2017-04-04  9:41       ` Michal Hocko
2017-04-04  9:49       ` Thomas Hellstrom
2017-04-04  9:49         ` Thomas Hellstrom
2017-04-04  9:49         ` Thomas Hellstrom
2017-04-05 10:31       ` Andrey Ryabinin
2017-04-05 10:31         ` Andrey Ryabinin
2017-04-05 10:31         ` Andrey Ryabinin
2017-04-05 10:42         ` Michal Hocko
2017-04-05 10:42           ` Michal Hocko
2017-04-05 11:42     ` Vlastimil Babka
2017-04-05 11:42       ` Vlastimil Babka
2017-04-05 12:14       ` Michal Hocko
2017-04-05 12:14         ` Michal Hocko
2017-03-30 22:22 ` Andrew Morton
2017-03-30 22:22   ` Andrew Morton
2017-03-30 22:22   ` Andrew Morton
2017-03-31  7:12   ` Joel Fernandes
2017-03-31  7:12     ` Joel Fernandes
2017-03-31  9:26   ` Andrey Ryabinin
2017-03-31  9:26     ` Andrey Ryabinin
2017-03-31  9:26     ` Andrey Ryabinin
2017-04-04  9:36   ` Michal Hocko
2017-04-04  9:36     ` Michal Hocko
2017-04-04  9:38 ` Michal Hocko
2017-04-04  9:38   ` Michal Hocko
2017-04-12 12:49 ` [PATCH v2 0/5] " Andrey Ryabinin
2017-04-12 12:49   ` Andrey Ryabinin
2017-04-12 12:49   ` Andrey Ryabinin [this message]
2017-04-12 12:49     ` [PATCH v2 1/5] mm/vmalloc: " Andrey Ryabinin
2017-04-12 12:49     ` Andrey Ryabinin
2017-04-12 12:49   ` [PATCH v2 2/5] x86/ldt: use vfree() instead of vfree_atomic() Andrey Ryabinin
2017-04-12 12:49     ` Andrey Ryabinin
2017-04-12 12:49   ` [PATCH v2 3/5] kernel/fork: use vfree() instead of vfree_atomic() to free thread stack Andrey Ryabinin
2017-04-12 12:49     ` Andrey Ryabinin
2017-04-12 12:49   ` [PATCH v2 4/5] mm/vmalloc: remove vfree_atomic() Andrey Ryabinin
2017-04-12 12:49     ` Andrey Ryabinin
2017-04-12 12:49   ` [PATCH v2 5/5] mm/vmalloc: Don't spawn workers if somebody already purging Andrey Ryabinin
2017-04-12 12:49     ` Andrey Ryabinin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170412124905.25443-2-aryabinin@virtuozzo.com \
    --to=aryabinin@virtuozzo.com \
    --cc=akpm@linux-foundation.org \
    --cc=chris@chris-wilson.co.uk \
    --cc=hch@lst.de \
    --cc=hpa@zytor.com \
    --cc=joaodias@google.com \
    --cc=joelaf@google.com \
    --cc=jszhang@marvell.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=mingo@elte.hu \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=thellstrom@vmware.com \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.