linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] android: binder: Drop lru lock in isolate callback
@ 2017-09-08 22:39 Sherry Yang
  2017-09-13 21:59 ` [PATCH] mm: Restore mmput_async Sherry Yang
  0 siblings, 1 reply; 9+ messages in thread
From: Sherry Yang @ 2017-09-08 22:39 UTC (permalink / raw)
  To: linux-kernel
  Cc: tkjos, maco, Sherry Yang, Greg Kroah-Hartman,
	Arve Hjønnevåg, Riley Andrews,
	open list:ANDROID DRIVERS

Drop the global lru lock in isolate callback
before calling zap_page_range which calls
cond_resched, and re-acquire the global lru
lock before returning. Also change return
code to LRU_REMOVED_RETRY.

Use mmput_async when fail to acquire mmap sem
in an atomic context.

Fix "BUG: sleeping function called from invalid context"
errors when CONFIG_DEBUG_ATOMIC_SLEEP is enabled.

Fixes: f2517eb76f1f2 ("android: binder: Add global lru shrinker to binder")
Reported-by: Kyle Yan <kyan@codeaurora.org>
Acked-by: Arve Hjønnevåg <arve@android.com>
Signed-off-by: Sherry Yang <sherryy@android.com>
---
 drivers/android/binder_alloc.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 8fe165844e47..064f5e31ec55 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -913,6 +913,7 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
 	struct binder_alloc *alloc;
 	uintptr_t page_addr;
 	size_t index;
+	struct vm_area_struct *vma;
 
 	alloc = page->alloc;
 	if (!mutex_trylock(&alloc->mutex))
@@ -923,16 +924,22 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
 
 	index = page - alloc->pages;
 	page_addr = (uintptr_t)alloc->buffer + index * PAGE_SIZE;
-	if (alloc->vma) {
+	vma = alloc->vma;
+	if (vma) {
 		mm = get_task_mm(alloc->tsk);
 		if (!mm)
 			goto err_get_task_mm_failed;
 		if (!down_write_trylock(&mm->mmap_sem))
 			goto err_down_write_mmap_sem_failed;
+	}
+
+	list_lru_isolate(lru, item);
+	spin_unlock(lock);
 
+	if (vma) {
 		trace_binder_unmap_user_start(alloc, index);
 
-		zap_page_range(alloc->vma,
+		zap_page_range(vma,
 			       page_addr + alloc->user_buffer_offset,
 			       PAGE_SIZE);
 
@@ -950,13 +957,12 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
 
 	trace_binder_unmap_kernel_end(alloc, index);
 
-	list_lru_isolate(lru, item);
-
+	spin_lock(lock);
 	mutex_unlock(&alloc->mutex);
-	return LRU_REMOVED;
+	return LRU_REMOVED_RETRY;
 
 err_down_write_mmap_sem_failed:
-	mmput(mm);
+	mmput_async(mm);
 err_get_task_mm_failed:
 err_page_already_freed:
 	mutex_unlock(&alloc->mutex);
-- 
2.11.0 (Apple Git-81)

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] mm: Restore mmput_async
  2017-09-08 22:39 [PATCH] android: binder: Drop lru lock in isolate callback Sherry Yang
@ 2017-09-13 21:59 ` Sherry Yang
  2017-09-13 22:09   ` Andrew Morton
  2017-09-14  8:19   ` Michal Hocko
  0 siblings, 2 replies; 9+ messages in thread
From: Sherry Yang @ 2017-09-13 21:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: tkjos, maco, arve, gregkh, Sherry Yang, Ingo Molnar,
	Michal Hocko, Andrew Morton, Vlastimil Babka, David Rientjes,
	Andrea Arcangeli, Peter Zijlstra, Thomas Gleixner,
	Andy Lutomirski, Oleg Nesterov, Hoeun Ryu, Christoph Lameter,
	Vegard Nossum, Frederic Weisbecker

Restore asynchronous mmput, allowing mmput_async to be called
from an atomic context in Android binder shrinker callback.

mmput_async was initially introduced in ec8d7c14e
("mm, oom_reaper: do not mmput synchronously from the
oom reaper context"), and was removed in 212925802
("mm: oom: let oom_reap_task and exit_mmap run concurrently")

Signed-off-by: Sherry Yang <sherryy@android.com>
---
 include/linux/sched/mm.h |  6 ++++++
 kernel/fork.c            | 18 ++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
index 3a19c253bdb1..ae53e413fb13 100644
--- a/include/linux/sched/mm.h
+++ b/include/linux/sched/mm.h
@@ -84,6 +84,12 @@ static inline bool mmget_not_zero(struct mm_struct *mm)
 
 /* mmput gets rid of the mappings and all user-space */
 extern void mmput(struct mm_struct *);
+#ifdef CONFIG_MMU
+/* same as above but performs the slow path from the async context. Can
+ * be called from the atomic context as well
+ */
+void mmput_async(struct mm_struct *);
+#endif
 
 /* Grab a reference to a task's mm, if it is not already going away */
 extern struct mm_struct *get_task_mm(struct task_struct *task);
diff --git a/kernel/fork.c b/kernel/fork.c
index 10646182440f..e702cb9ffbd8 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -946,6 +946,24 @@ void mmput(struct mm_struct *mm)
 }
 EXPORT_SYMBOL_GPL(mmput);
 
+#ifdef CONFIG_MMU
+static void mmput_async_fn(struct work_struct *work)
+{
+	struct mm_struct *mm = container_of(work, struct mm_struct,
+					    async_put_work);
+
+	__mmput(mm);
+}
+
+void mmput_async(struct mm_struct *mm)
+{
+	if (atomic_dec_and_test(&mm->mm_users)) {
+		INIT_WORK(&mm->async_put_work, mmput_async_fn);
+		schedule_work(&mm->async_put_work);
+	}
+}
+#endif
+
 /**
  * set_mm_exe_file - change a reference to the mm's executable file
  *
-- 
2.11.0 (Apple Git-81)

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm: Restore mmput_async
  2017-09-13 21:59 ` [PATCH] mm: Restore mmput_async Sherry Yang
@ 2017-09-13 22:09   ` Andrew Morton
  2017-09-13 22:44     ` Sherry Yang
  2017-09-14  8:19   ` Michal Hocko
  1 sibling, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2017-09-13 22:09 UTC (permalink / raw)
  To: Sherry Yang
  Cc: linux-kernel, tkjos, maco, arve, gregkh, Ingo Molnar,
	Michal Hocko, Vlastimil Babka, David Rientjes, Andrea Arcangeli,
	Peter Zijlstra, Thomas Gleixner, Andy Lutomirski, Oleg Nesterov,
	Hoeun Ryu, Christoph Lameter, Vegard Nossum, Frederic Weisbecker

On Wed, 13 Sep 2017 17:59:27 -0400 Sherry Yang <sherryy@android.com> wrote:

> Restore asynchronous mmput, allowing mmput_async to be called
> from an atomic context in Android binder shrinker callback.
> 
> mmput_async was initially introduced in ec8d7c14e
> ("mm, oom_reaper: do not mmput synchronously from the
> oom reaper context"), and was removed in 212925802
> ("mm: oom: let oom_reap_task and exit_mmap run concurrently")

Presumably there's a patch somewhere which adds a call to mmput_async()
into drivers/android/binder.c?  Where is that patch?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm: Restore mmput_async
  2017-09-13 22:09   ` Andrew Morton
@ 2017-09-13 22:44     ` Sherry Yang
  2017-09-13 22:57       ` Andrew Morton
  0 siblings, 1 reply; 9+ messages in thread
From: Sherry Yang @ 2017-09-13 22:44 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, Todd Kjos, Martijn Coenen,
	Arve Hjønnevåg, Greg Kroah-Hartman, Ingo Molnar,
	Michal Hocko, Vlastimil Babka, David Rientjes, Andrea Arcangeli,
	Peter Zijlstra, Thomas Gleixner, Andy Lutomirski, Oleg Nesterov,
	Hoeun Ryu, Christoph Lameter, Vegard Nossum, Frederic Weisbecker

The patch that uses mmput_async() is
https://lkml.org/lkml/2017/9/8/785. Gmail doesn't seem to respect
in-reply-to.

On Wed, Sep 13, 2017 at 6:09 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Wed, 13 Sep 2017 17:59:27 -0400 Sherry Yang <sherryy@android.com> wrote:
>
>> Restore asynchronous mmput, allowing mmput_async to be called
>> from an atomic context in Android binder shrinker callback.
>>
>> mmput_async was initially introduced in ec8d7c14e
>> ("mm, oom_reaper: do not mmput synchronously from the
>> oom reaper context"), and was removed in 212925802
>> ("mm: oom: let oom_reap_task and exit_mmap run concurrently")
>
> Presumably there's a patch somewhere which adds a call to mmput_async()
> into drivers/android/binder.c?  Where is that patch?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm: Restore mmput_async
  2017-09-13 22:44     ` Sherry Yang
@ 2017-09-13 22:57       ` Andrew Morton
  2017-09-14  0:08         ` Arve Hjønnevåg
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2017-09-13 22:57 UTC (permalink / raw)
  To: Sherry Yang
  Cc: linux-kernel, Todd Kjos, Martijn Coenen,
	Arve Hjønnevåg, Greg Kroah-Hartman, Ingo Molnar,
	Michal Hocko, Vlastimil Babka, David Rientjes, Andrea Arcangeli,
	Peter Zijlstra, Thomas Gleixner, Andy Lutomirski, Oleg Nesterov,
	Hoeun Ryu, Christoph Lameter, Vegard Nossum, Frederic Weisbecker

On Wed, 13 Sep 2017 18:44:11 -0400 Sherry Yang <sherryy@android.com> wrote:

> On Wed, Sep 13, 2017 at 6:09 PM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
> > On Wed, 13 Sep 2017 17:59:27 -0400 Sherry Yang <sherryy@android.com> wrote:
> >
> >> Restore asynchronous mmput, allowing mmput_async to be called
> >> from an atomic context in Android binder shrinker callback.
> >>
> >> mmput_async was initially introduced in ec8d7c14e
> >> ("mm, oom_reaper: do not mmput synchronously from the
> >> oom reaper context"), and was removed in 212925802
> >> ("mm: oom: let oom_reap_task and exit_mmap run concurrently")
> >
> > Presumably there's a patch somewhere which adds a call to mmput_async()
> > into drivers/android/binder.c?  Where is that patch?
>
> The patch that uses mmput_async() is
> https://lkml.org/lkml/2017/9/8/785. Gmail doesn't seem to respect
> in-reply-to.

(Top-posting repaired.  Please don't!)

Is it necessary for binder_alloc_free_page() to take a ref on the mm? 
As long as alloc->tsk doesn't exit during binder_alloc_free_page()'s
execution, that task's reference on the mm should be sufficient to keep
the mm alive?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm: Restore mmput_async
  2017-09-13 22:57       ` Andrew Morton
@ 2017-09-14  0:08         ` Arve Hjønnevåg
  0 siblings, 0 replies; 9+ messages in thread
From: Arve Hjønnevåg @ 2017-09-14  0:08 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Sherry Yang, LKML, Todd Kjos, Martijn Coenen, Greg Kroah-Hartman,
	Ingo Molnar, Michal Hocko, Vlastimil Babka, David Rientjes,
	Andrea Arcangeli, Peter Zijlstra, Thomas Gleixner,
	Andy Lutomirski, Oleg Nesterov, Hoeun Ryu, Christoph Lameter,
	Vegard Nossum, Frederic Weisbecker

On Wed, Sep 13, 2017 at 3:57 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Wed, 13 Sep 2017 18:44:11 -0400 Sherry Yang <sherryy@android.com> wrote:
>
>> On Wed, Sep 13, 2017 at 6:09 PM, Andrew Morton
>> <akpm@linux-foundation.org> wrote:
>> > On Wed, 13 Sep 2017 17:59:27 -0400 Sherry Yang <sherryy@android.com> wrote:
>> >
>> >> Restore asynchronous mmput, allowing mmput_async to be called
>> >> from an atomic context in Android binder shrinker callback.
>> >>
>> >> mmput_async was initially introduced in ec8d7c14e
>> >> ("mm, oom_reaper: do not mmput synchronously from the
>> >> oom reaper context"), and was removed in 212925802
>> >> ("mm: oom: let oom_reap_task and exit_mmap run concurrently")
>> >
>> > Presumably there's a patch somewhere which adds a call to mmput_async()
>> > into drivers/android/binder.c?  Where is that patch?
>>
>> The patch that uses mmput_async() is
>> https://lkml.org/lkml/2017/9/8/785. Gmail doesn't seem to respect
>> in-reply-to.
>
> (Top-posting repaired.  Please don't!)
>
> Is it necessary for binder_alloc_free_page() to take a ref on the mm?
> As long as alloc->tsk doesn't exit during binder_alloc_free_page()'s
> execution, that task's reference on the mm should be sufficient to keep
> the mm alive?
>

alloc->tsk can exit during binder_alloc_free_page. We don't hold a
reference to the task's mm struct while we don't actively use it, as
this would prevent the driver from getting closed when a process dies.

-- 
Arve Hjønnevåg

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm: Restore mmput_async
  2017-09-13 21:59 ` [PATCH] mm: Restore mmput_async Sherry Yang
  2017-09-13 22:09   ` Andrew Morton
@ 2017-09-14  8:19   ` Michal Hocko
  2017-09-14 18:22     ` [PATCH v2] android: binder: Drop lru lock in isolate callback Sherry Yang
  1 sibling, 1 reply; 9+ messages in thread
From: Michal Hocko @ 2017-09-14  8:19 UTC (permalink / raw)
  To: Sherry Yang
  Cc: linux-kernel, tkjos, maco, arve, gregkh, Ingo Molnar,
	Andrew Morton, Vlastimil Babka, David Rientjes, Andrea Arcangeli,
	Peter Zijlstra, Thomas Gleixner, Andy Lutomirski, Oleg Nesterov,
	Hoeun Ryu, Christoph Lameter, Vegard Nossum, Frederic Weisbecker

On Wed 13-09-17 17:59:27, Sherry Yang wrote:
> Restore asynchronous mmput, allowing mmput_async to be called
> from an atomic context in Android binder shrinker callback.
> 
> mmput_async was initially introduced in ec8d7c14e
> ("mm, oom_reaper: do not mmput synchronously from the
> oom reaper context"), and was removed in 212925802
> ("mm: oom: let oom_reap_task and exit_mmap run concurrently")
> 
> Signed-off-by: Sherry Yang <sherryy@android.com>

I would just fold this into https://lkml.org/lkml/2017/9/8/785. The
patch is simple enough to take this in one go and it will be clear who
and why needs to restore the api.

No objections otherwise.
Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  include/linux/sched/mm.h |  6 ++++++
>  kernel/fork.c            | 18 ++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
> index 3a19c253bdb1..ae53e413fb13 100644
> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -84,6 +84,12 @@ static inline bool mmget_not_zero(struct mm_struct *mm)
>  
>  /* mmput gets rid of the mappings and all user-space */
>  extern void mmput(struct mm_struct *);
> +#ifdef CONFIG_MMU
> +/* same as above but performs the slow path from the async context. Can
> + * be called from the atomic context as well
> + */
> +void mmput_async(struct mm_struct *);
> +#endif
>  
>  /* Grab a reference to a task's mm, if it is not already going away */
>  extern struct mm_struct *get_task_mm(struct task_struct *task);
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 10646182440f..e702cb9ffbd8 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -946,6 +946,24 @@ void mmput(struct mm_struct *mm)
>  }
>  EXPORT_SYMBOL_GPL(mmput);
>  
> +#ifdef CONFIG_MMU
> +static void mmput_async_fn(struct work_struct *work)
> +{
> +	struct mm_struct *mm = container_of(work, struct mm_struct,
> +					    async_put_work);
> +
> +	__mmput(mm);
> +}
> +
> +void mmput_async(struct mm_struct *mm)
> +{
> +	if (atomic_dec_and_test(&mm->mm_users)) {
> +		INIT_WORK(&mm->async_put_work, mmput_async_fn);
> +		schedule_work(&mm->async_put_work);
> +	}
> +}
> +#endif
> +
>  /**
>   * set_mm_exe_file - change a reference to the mm's executable file
>   *
> -- 
> 2.11.0 (Apple Git-81)
> 

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] android: binder: Drop lru lock in isolate callback
  2017-09-14  8:19   ` Michal Hocko
@ 2017-09-14 18:22     ` Sherry Yang
  2017-09-14 20:12       ` Andrew Morton
  0 siblings, 1 reply; 9+ messages in thread
From: Sherry Yang @ 2017-09-14 18:22 UTC (permalink / raw)
  To: linux-kernel
  Cc: maco, tkjos, Sherry Yang, Greg Kroah-Hartman,
	Arve Hjønnevåg, Riley Andrews, Ingo Molnar,
	Andrew Morton, Michal Hocko, Vlastimil Babka, Hillf Danton,
	Peter Zijlstra, Andrea Arcangeli, Thomas Gleixner,
	Andy Lutomirski, Oleg Nesterov, Hoeun Ryu, Christoph Lameter,
	Vegard Nossum, Frederic Weisbecker, open list:ANDROID DRIVERS

Drop the global lru lock in isolate callback
before calling zap_page_range which calls
cond_resched, and re-acquire the global lru
lock before returning. Also change return
code to LRU_REMOVED_RETRY.

Use mmput_async when fail to acquire mmap sem
in an atomic context.

Fix "BUG: sleeping function called from invalid context"
errors when CONFIG_DEBUG_ATOMIC_SLEEP is enabled.

Also restore mmput_async, which was initially introduced in
ec8d7c14e ("mm, oom_reaper: do not mmput synchronously from
the oom reaper context"), and was removed in 212925802
("mm: oom: let oom_reap_task and exit_mmap run concurrently").

Fixes: f2517eb76f1f2 ("android: binder: Add global lru shrinker to binder")
Reported-by: Kyle Yan <kyan@codeaurora.org>
Acked-by: Arve Hjønnevåg <arve@android.com>
Signed-off-by: Sherry Yang <sherryy@android.com>
---
Changes in v2:
Combined '[PATCH] android: binder: Drop lru lock in isolate callback'
with '[PATCH] mm: Restore mmput_async'.

 drivers/android/binder_alloc.c | 18 ++++++++++++------
 include/linux/sched/mm.h       |  6 ++++++
 kernel/fork.c                  | 18 ++++++++++++++++++
 3 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 8fe165844e47..064f5e31ec55 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -913,6 +913,7 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
 	struct binder_alloc *alloc;
 	uintptr_t page_addr;
 	size_t index;
+	struct vm_area_struct *vma;
 
 	alloc = page->alloc;
 	if (!mutex_trylock(&alloc->mutex))
@@ -923,16 +924,22 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
 
 	index = page - alloc->pages;
 	page_addr = (uintptr_t)alloc->buffer + index * PAGE_SIZE;
-	if (alloc->vma) {
+	vma = alloc->vma;
+	if (vma) {
 		mm = get_task_mm(alloc->tsk);
 		if (!mm)
 			goto err_get_task_mm_failed;
 		if (!down_write_trylock(&mm->mmap_sem))
 			goto err_down_write_mmap_sem_failed;
+	}
+
+	list_lru_isolate(lru, item);
+	spin_unlock(lock);
 
+	if (vma) {
 		trace_binder_unmap_user_start(alloc, index);
 
-		zap_page_range(alloc->vma,
+		zap_page_range(vma,
 			       page_addr + alloc->user_buffer_offset,
 			       PAGE_SIZE);
 
@@ -950,13 +957,12 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
 
 	trace_binder_unmap_kernel_end(alloc, index);
 
-	list_lru_isolate(lru, item);
-
+	spin_lock(lock);
 	mutex_unlock(&alloc->mutex);
-	return LRU_REMOVED;
+	return LRU_REMOVED_RETRY;
 
 err_down_write_mmap_sem_failed:
-	mmput(mm);
+	mmput_async(mm);
 err_get_task_mm_failed:
 err_page_already_freed:
 	mutex_unlock(&alloc->mutex);
diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
index 3a19c253bdb1..ae53e413fb13 100644
--- a/include/linux/sched/mm.h
+++ b/include/linux/sched/mm.h
@@ -84,6 +84,12 @@ static inline bool mmget_not_zero(struct mm_struct *mm)
 
 /* mmput gets rid of the mappings and all user-space */
 extern void mmput(struct mm_struct *);
+#ifdef CONFIG_MMU
+/* same as above but performs the slow path from the async context. Can
+ * be called from the atomic context as well
+ */
+void mmput_async(struct mm_struct *);
+#endif
 
 /* Grab a reference to a task's mm, if it is not already going away */
 extern struct mm_struct *get_task_mm(struct task_struct *task);
diff --git a/kernel/fork.c b/kernel/fork.c
index 10646182440f..e702cb9ffbd8 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -946,6 +946,24 @@ void mmput(struct mm_struct *mm)
 }
 EXPORT_SYMBOL_GPL(mmput);
 
+#ifdef CONFIG_MMU
+static void mmput_async_fn(struct work_struct *work)
+{
+	struct mm_struct *mm = container_of(work, struct mm_struct,
+					    async_put_work);
+
+	__mmput(mm);
+}
+
+void mmput_async(struct mm_struct *mm)
+{
+	if (atomic_dec_and_test(&mm->mm_users)) {
+		INIT_WORK(&mm->async_put_work, mmput_async_fn);
+		schedule_work(&mm->async_put_work);
+	}
+}
+#endif
+
 /**
  * set_mm_exe_file - change a reference to the mm's executable file
  *
-- 
2.11.0 (Apple Git-81)

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] android: binder: Drop lru lock in isolate callback
  2017-09-14 18:22     ` [PATCH v2] android: binder: Drop lru lock in isolate callback Sherry Yang
@ 2017-09-14 20:12       ` Andrew Morton
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Morton @ 2017-09-14 20:12 UTC (permalink / raw)
  To: Sherry Yang
  Cc: linux-kernel, maco, tkjos, Greg Kroah-Hartman,
	Arve Hjønnevåg, Riley Andrews, Ingo Molnar,
	Michal Hocko, Vlastimil Babka, Hillf Danton, Peter Zijlstra,
	Andrea Arcangeli, Thomas Gleixner, Andy Lutomirski,
	Oleg Nesterov, Hoeun Ryu, Christoph Lameter, Vegard Nossum,
	Frederic Weisbecker, open list:ANDROID DRIVERS

On Thu, 14 Sep 2017 14:22:25 -0400 Sherry Yang <sherryy@android.com> wrote:

> Drop the global lru lock in isolate callback
> before calling zap_page_range which calls
> cond_resched, and re-acquire the global lru
> lock before returning. Also change return
> code to LRU_REMOVED_RETRY.
> 
> Use mmput_async when fail to acquire mmap sem
> in an atomic context.
> 
> Fix "BUG: sleeping function called from invalid context"
> errors when CONFIG_DEBUG_ATOMIC_SLEEP is enabled.
> 
> Also restore mmput_async, which was initially introduced in
> ec8d7c14e ("mm, oom_reaper: do not mmput synchronously from
> the oom reaper context"), and was removed in 212925802
> ("mm: oom: let oom_reap_task and exit_mmap run concurrently").

Ho hum, OK.  It's a bit sad to bring this back for one caller but
mm.async_put_work is there and won't be going away.

The mmdrop_async stuff should be moved into fork.c (and maybe made
static) as well.  It's pointless having this stuff global and inlined,
especially mmdrop_async_fn().


Something like the below, not yet tested...


From: Andrew Morton <akpm@linux-foundation.org>
Subject: include/linux/sched/mm.h: uninline mmdrop_async(), etc

mmdrop_async() is only used in fork.c.  Move that and its support functions
into fork.c, uninline it all.



Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 include/linux/sched/mm.h |   24 +--------------
 kernel/fork.c            |   58 +++++++++++++++++++++++++------------
 2 files changed, 42 insertions(+), 40 deletions(-)

diff -puN include/linux/sched/mm.h~a include/linux/sched/mm.h
--- a/include/linux/sched/mm.h~a
+++ a/include/linux/sched/mm.h
@@ -10,7 +10,7 @@
 /*
  * Routines for handling mm_structs
  */
-extern struct mm_struct * mm_alloc(void);
+extern struct mm_struct *mm_alloc(void);
 
 /**
  * mmgrab() - Pin a &struct mm_struct.
@@ -34,27 +34,7 @@ static inline void mmgrab(struct mm_stru
 	atomic_inc(&mm->mm_count);
 }
 
-/* mmdrop drops the mm and the page tables */
-extern void __mmdrop(struct mm_struct *);
-static inline void mmdrop(struct mm_struct *mm)
-{
-	if (unlikely(atomic_dec_and_test(&mm->mm_count)))
-		__mmdrop(mm);
-}
-
-static inline void mmdrop_async_fn(struct work_struct *work)
-{
-	struct mm_struct *mm = container_of(work, struct mm_struct, async_put_work);
-	__mmdrop(mm);
-}
-
-static inline void mmdrop_async(struct mm_struct *mm)
-{
-	if (unlikely(atomic_dec_and_test(&mm->mm_count))) {
-		INIT_WORK(&mm->async_put_work, mmdrop_async_fn);
-		schedule_work(&mm->async_put_work);
-	}
-}
+extern void mmdrop(struct mm_struct *mm);
 
 /**
  * mmget() - Pin the address space associated with a &struct mm_struct.
diff -puN kernel/fork.c~a kernel/fork.c
--- a/kernel/fork.c~a
+++ a/kernel/fork.c
@@ -386,6 +386,46 @@ void free_task(struct task_struct *tsk)
 }
 EXPORT_SYMBOL(free_task);
 
+/*
+ * Called when the last reference to the mm
+ * is dropped: either by a lazy thread or by
+ * mmput. Free the page directory and the mm.
+ */
+static void __mmdrop(struct mm_struct *mm)
+{
+	BUG_ON(mm == &init_mm);
+	mm_free_pgd(mm);
+	destroy_context(mm);
+	hmm_mm_destroy(mm);
+	mmu_notifier_mm_destroy(mm);
+	check_mm(mm);
+	put_user_ns(mm->user_ns);
+	free_mm(mm);
+}
+
+void mmdrop(struct mm_struct *mm)
+{
+	if (unlikely(atomic_dec_and_test(&mm->mm_count)))
+		__mmdrop(mm);
+}
+EXPORT_SYMBOL_GPL(mmdrop)
+
+static void mmdrop_async_fn(struct work_struct *work)
+{
+	struct mm_struct *mm;
+
+	mm = container_of(work, struct mm_struct, async_put_work);
+	__mmdrop(mm);
+}
+
+static void mmdrop_async(struct mm_struct *mm)
+{
+	if (unlikely(atomic_dec_and_test(&mm->mm_count))) {
+		INIT_WORK(&mm->async_put_work, mmdrop_async_fn);
+		schedule_work(&mm->async_put_work);
+	}
+}
+
 static inline void free_signal_struct(struct signal_struct *sig)
 {
 	taskstats_tgid_free(sig);
@@ -895,24 +935,6 @@ struct mm_struct *mm_alloc(void)
 	return mm_init(mm, current, current_user_ns());
 }
 
-/*
- * Called when the last reference to the mm
- * is dropped: either by a lazy thread or by
- * mmput. Free the page directory and the mm.
- */
-void __mmdrop(struct mm_struct *mm)
-{
-	BUG_ON(mm == &init_mm);
-	mm_free_pgd(mm);
-	destroy_context(mm);
-	hmm_mm_destroy(mm);
-	mmu_notifier_mm_destroy(mm);
-	check_mm(mm);
-	put_user_ns(mm->user_ns);
-	free_mm(mm);
-}
-EXPORT_SYMBOL_GPL(__mmdrop);
-
 static inline void __mmput(struct mm_struct *mm)
 {
 	VM_BUG_ON(atomic_read(&mm->mm_users));
_

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-09-14 20:12 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-08 22:39 [PATCH] android: binder: Drop lru lock in isolate callback Sherry Yang
2017-09-13 21:59 ` [PATCH] mm: Restore mmput_async Sherry Yang
2017-09-13 22:09   ` Andrew Morton
2017-09-13 22:44     ` Sherry Yang
2017-09-13 22:57       ` Andrew Morton
2017-09-14  0:08         ` Arve Hjønnevåg
2017-09-14  8:19   ` Michal Hocko
2017-09-14 18:22     ` [PATCH v2] android: binder: Drop lru lock in isolate callback Sherry Yang
2017-09-14 20:12       ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).