All of lore.kernel.org
 help / color / mirror / Atom feed
* kswapd using __this_cpu_add() in preemptible code
@ 2014-03-18 18:53 ` Sergey Senozhatsky
  0 siblings, 0 replies; 10+ messages in thread
From: Sergey Senozhatsky @ 2014-03-18 18:53 UTC (permalink / raw)
  To: Alexander Viro
  Cc: Andrew Morton, Mel Gorman, Michal Hocko, Rik van Riel,
	Johannes Weiner, linux-fsdevel, linux-mm, linux-kernel

Hello gentlemen,

Commit 589a606f9539663f162e4a110d117527833b58a4 ("percpu: add preemption
checks to __this_cpu ops") added preempt check to used in __count_vm_events()
__this_cpu ops, causing the following kswapd warning:

 BUG: using __this_cpu_add() in preemptible [00000000] code: kswapd0/56
 caller is __this_cpu_preempt_check+0x2b/0x2d
 Call Trace:
 [<ffffffff813b8d4d>] dump_stack+0x4e/0x7a
 [<ffffffff8121366f>] check_preemption_disabled+0xce/0xdd
 [<ffffffff812136bb>] __this_cpu_preempt_check+0x2b/0x2d
 [<ffffffff810f622e>] inode_lru_isolate+0xed/0x197
 [<ffffffff810be43c>] list_lru_walk_node+0x7b/0x14c
 [<ffffffff810f6141>] ? iput+0x131/0x131
 [<ffffffff810f681f>] prune_icache_sb+0x35/0x4c
 [<ffffffff810e3951>] super_cache_scan+0xe3/0x143
 [<ffffffff810b1301>] shrink_slab_node+0x103/0x16f
 [<ffffffff810b19fd>] shrink_slab+0x75/0xe4
 [<ffffffff810b3f3d>] balance_pgdat+0x2fa/0x47f
 [<ffffffff810b4395>] kswapd+0x2d3/0x2fd
 [<ffffffff81068049>] ? __wake_up_sync+0xd/0xd
 [<ffffffff810b40c2>] ? balance_pgdat+0x47f/0x47f
 [<ffffffff81051e75>] kthread+0xd6/0xde
 [<ffffffff81051d9f>] ? kthread_create_on_node+0x162/0x162
 [<ffffffff813be5bc>] ret_from_fork+0x7c/0xb0
 [<ffffffff81051d9f>] ? kthread_create_on_node+0x162/0x162


list_lru_walk_node() seems to be the only place where __count_vm_events()
called with preemption enabled. remaining __count_vm_events() and
__count_vm_event() calls are done with preemption disabled (unless I
overlooked something).


	-ss

^ permalink raw reply	[flat|nested] 10+ messages in thread
* kswapd using __this_cpu_add() in preemptible code
@ 2014-03-18 18:53 ` Sergey Senozhatsky
  0 siblings, 0 replies; 10+ messages in thread
From: Sergey Senozhatsky @ 2014-03-18 18:53 UTC (permalink / raw)
  To: Alexander Viro
  Cc: Andrew Morton, Mel Gorman, Michal Hocko, Rik van Riel,
	Johannes Weiner, linux-fsdevel, linux-mm, linux-kernel

Hello gentlemen,

Commit 589a606f9539663f162e4a110d117527833b58a4 ("percpu: add preemption
checks to __this_cpu ops") added preempt check to used in __count_vm_events()
__this_cpu ops, causing the following kswapd warning:

 BUG: using __this_cpu_add() in preemptible [00000000] code: kswapd0/56
 caller is __this_cpu_preempt_check+0x2b/0x2d
 Call Trace:
 [<ffffffff813b8d4d>] dump_stack+0x4e/0x7a
 [<ffffffff8121366f>] check_preemption_disabled+0xce/0xdd
 [<ffffffff812136bb>] __this_cpu_preempt_check+0x2b/0x2d
 [<ffffffff810f622e>] inode_lru_isolate+0xed/0x197
 [<ffffffff810be43c>] list_lru_walk_node+0x7b/0x14c
 [<ffffffff810f6141>] ? iput+0x131/0x131
 [<ffffffff810f681f>] prune_icache_sb+0x35/0x4c
 [<ffffffff810e3951>] super_cache_scan+0xe3/0x143
 [<ffffffff810b1301>] shrink_slab_node+0x103/0x16f
 [<ffffffff810b19fd>] shrink_slab+0x75/0xe4
 [<ffffffff810b3f3d>] balance_pgdat+0x2fa/0x47f
 [<ffffffff810b4395>] kswapd+0x2d3/0x2fd
 [<ffffffff81068049>] ? __wake_up_sync+0xd/0xd
 [<ffffffff810b40c2>] ? balance_pgdat+0x47f/0x47f
 [<ffffffff81051e75>] kthread+0xd6/0xde
 [<ffffffff81051d9f>] ? kthread_create_on_node+0x162/0x162
 [<ffffffff813be5bc>] ret_from_fork+0x7c/0xb0
 [<ffffffff81051d9f>] ? kthread_create_on_node+0x162/0x162


list_lru_walk_node() seems to be the only place where __count_vm_events()
called with preemption enabled. remaining __count_vm_events() and
__count_vm_event() calls are done with preemption disabled (unless I
overlooked something).


	-ss

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: kswapd using __this_cpu_add() in preemptible code
  2014-03-18 18:53 ` Sergey Senozhatsky
@ 2014-03-18 21:22   ` Andrew Morton
  -1 siblings, 0 replies; 10+ messages in thread
From: Andrew Morton @ 2014-03-18 21:22 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Alexander Viro, Mel Gorman, Michal Hocko, Rik van Riel,
	Johannes Weiner, linux-fsdevel, linux-mm, linux-kernel,
	Christoph Lameter

On Tue, 18 Mar 2014 21:53:30 +0300 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:

> Hello gentlemen,
> 
> Commit 589a606f9539663f162e4a110d117527833b58a4 ("percpu: add preemption
> checks to __this_cpu ops") added preempt check to used in __count_vm_events()
> __this_cpu ops, causing the following kswapd warning:
> 
>  BUG: using __this_cpu_add() in preemptible [00000000] code: kswapd0/56
>  caller is __this_cpu_preempt_check+0x2b/0x2d
>  Call Trace:
>  [<ffffffff813b8d4d>] dump_stack+0x4e/0x7a
>  [<ffffffff8121366f>] check_preemption_disabled+0xce/0xdd
>  [<ffffffff812136bb>] __this_cpu_preempt_check+0x2b/0x2d
>  [<ffffffff810f622e>] inode_lru_isolate+0xed/0x197
>  [<ffffffff810be43c>] list_lru_walk_node+0x7b/0x14c
>  [<ffffffff810f6141>] ? iput+0x131/0x131
>  [<ffffffff810f681f>] prune_icache_sb+0x35/0x4c
>  [<ffffffff810e3951>] super_cache_scan+0xe3/0x143
>  [<ffffffff810b1301>] shrink_slab_node+0x103/0x16f
>  [<ffffffff810b19fd>] shrink_slab+0x75/0xe4
>  [<ffffffff810b3f3d>] balance_pgdat+0x2fa/0x47f
>  [<ffffffff810b4395>] kswapd+0x2d3/0x2fd
>  [<ffffffff81068049>] ? __wake_up_sync+0xd/0xd
>  [<ffffffff810b40c2>] ? balance_pgdat+0x47f/0x47f
>  [<ffffffff81051e75>] kthread+0xd6/0xde
>  [<ffffffff81051d9f>] ? kthread_create_on_node+0x162/0x162
>  [<ffffffff813be5bc>] ret_from_fork+0x7c/0xb0
>  [<ffffffff81051d9f>] ? kthread_create_on_node+0x162/0x162
> 
> 
> list_lru_walk_node() seems to be the only place where __count_vm_events()
> called with preemption enabled. remaining __count_vm_events() and
> __count_vm_event() calls are done with preemption disabled (unless I
> overlooked something).

Christoph caught one.  How does this look?



From: Andrew Morton <akpm@linux-foundation.org>
Subject: fs/inode.c:inode_lru_isolate(): use atomic count_vm_events()

"percpu: add preemption checks to __this_cpu ops" added preempt check to
used in __count_vm_events() __this_cpu ops, causing the following kswapd
warning:

 BUG: using __this_cpu_add() in preemptible [00000000] code: kswapd0/56
 caller is __this_cpu_preempt_check+0x2b/0x2d
 Call Trace:
 [<ffffffff813b8d4d>] dump_stack+0x4e/0x7a
 [<ffffffff8121366f>] check_preemption_disabled+0xce/0xdd
 [<ffffffff812136bb>] __this_cpu_preempt_check+0x2b/0x2d
 [<ffffffff810f622e>] inode_lru_isolate+0xed/0x197
 [<ffffffff810be43c>] list_lru_walk_node+0x7b/0x14c
 [<ffffffff810f6141>] ? iput+0x131/0x131
 [<ffffffff810f681f>] prune_icache_sb+0x35/0x4c

Switch from __count_vm_events() to the preempt-safe count_vm_events().

Reported-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Christoph Lameter <cl@linux-foundation.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 fs/inode.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff -puN fs/inode.c~fs-inodec-inode_lru_isolate-use-atomic-count_vm_events fs/inode.c
--- a/fs/inode.c~fs-inodec-inode_lru_isolate-use-atomic-count_vm_events
+++ a/fs/inode.c
@@ -722,9 +722,9 @@ inode_lru_isolate(struct list_head *item
 			unsigned long reap;
 			reap = invalidate_mapping_pages(&inode->i_data, 0, -1);
 			if (current_is_kswapd())
-				__count_vm_events(KSWAPD_INODESTEAL, reap);
+				count_vm_events(KSWAPD_INODESTEAL, reap);
 			else
-				__count_vm_events(PGINODESTEAL, reap);
+				count_vm_events(PGINODESTEAL, reap);
 			if (current->reclaim_state)
 				current->reclaim_state->reclaimed_slab += reap;
 		}
_


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: kswapd using __this_cpu_add() in preemptible code
@ 2014-03-18 21:22   ` Andrew Morton
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Morton @ 2014-03-18 21:22 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Alexander Viro, Mel Gorman, Michal Hocko, Rik van Riel,
	Johannes Weiner, linux-fsdevel, linux-mm, linux-kernel,
	Christoph Lameter

On Tue, 18 Mar 2014 21:53:30 +0300 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:

> Hello gentlemen,
> 
> Commit 589a606f9539663f162e4a110d117527833b58a4 ("percpu: add preemption
> checks to __this_cpu ops") added preempt check to used in __count_vm_events()
> __this_cpu ops, causing the following kswapd warning:
> 
>  BUG: using __this_cpu_add() in preemptible [00000000] code: kswapd0/56
>  caller is __this_cpu_preempt_check+0x2b/0x2d
>  Call Trace:
>  [<ffffffff813b8d4d>] dump_stack+0x4e/0x7a
>  [<ffffffff8121366f>] check_preemption_disabled+0xce/0xdd
>  [<ffffffff812136bb>] __this_cpu_preempt_check+0x2b/0x2d
>  [<ffffffff810f622e>] inode_lru_isolate+0xed/0x197
>  [<ffffffff810be43c>] list_lru_walk_node+0x7b/0x14c
>  [<ffffffff810f6141>] ? iput+0x131/0x131
>  [<ffffffff810f681f>] prune_icache_sb+0x35/0x4c
>  [<ffffffff810e3951>] super_cache_scan+0xe3/0x143
>  [<ffffffff810b1301>] shrink_slab_node+0x103/0x16f
>  [<ffffffff810b19fd>] shrink_slab+0x75/0xe4
>  [<ffffffff810b3f3d>] balance_pgdat+0x2fa/0x47f
>  [<ffffffff810b4395>] kswapd+0x2d3/0x2fd
>  [<ffffffff81068049>] ? __wake_up_sync+0xd/0xd
>  [<ffffffff810b40c2>] ? balance_pgdat+0x47f/0x47f
>  [<ffffffff81051e75>] kthread+0xd6/0xde
>  [<ffffffff81051d9f>] ? kthread_create_on_node+0x162/0x162
>  [<ffffffff813be5bc>] ret_from_fork+0x7c/0xb0
>  [<ffffffff81051d9f>] ? kthread_create_on_node+0x162/0x162
> 
> 
> list_lru_walk_node() seems to be the only place where __count_vm_events()
> called with preemption enabled. remaining __count_vm_events() and
> __count_vm_event() calls are done with preemption disabled (unless I
> overlooked something).

Christoph caught one.  How does this look?



From: Andrew Morton <akpm@linux-foundation.org>
Subject: fs/inode.c:inode_lru_isolate(): use atomic count_vm_events()

"percpu: add preemption checks to __this_cpu ops" added preempt check to
used in __count_vm_events() __this_cpu ops, causing the following kswapd
warning:

 BUG: using __this_cpu_add() in preemptible [00000000] code: kswapd0/56
 caller is __this_cpu_preempt_check+0x2b/0x2d
 Call Trace:
 [<ffffffff813b8d4d>] dump_stack+0x4e/0x7a
 [<ffffffff8121366f>] check_preemption_disabled+0xce/0xdd
 [<ffffffff812136bb>] __this_cpu_preempt_check+0x2b/0x2d
 [<ffffffff810f622e>] inode_lru_isolate+0xed/0x197
 [<ffffffff810be43c>] list_lru_walk_node+0x7b/0x14c
 [<ffffffff810f6141>] ? iput+0x131/0x131
 [<ffffffff810f681f>] prune_icache_sb+0x35/0x4c

Switch from __count_vm_events() to the preempt-safe count_vm_events().

Reported-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Christoph Lameter <cl@linux-foundation.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 fs/inode.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff -puN fs/inode.c~fs-inodec-inode_lru_isolate-use-atomic-count_vm_events fs/inode.c
--- a/fs/inode.c~fs-inodec-inode_lru_isolate-use-atomic-count_vm_events
+++ a/fs/inode.c
@@ -722,9 +722,9 @@ inode_lru_isolate(struct list_head *item
 			unsigned long reap;
 			reap = invalidate_mapping_pages(&inode->i_data, 0, -1);
 			if (current_is_kswapd())
-				__count_vm_events(KSWAPD_INODESTEAL, reap);
+				count_vm_events(KSWAPD_INODESTEAL, reap);
 			else
-				__count_vm_events(PGINODESTEAL, reap);
+				count_vm_events(PGINODESTEAL, reap);
 			if (current->reclaim_state)
 				current->reclaim_state->reclaimed_slab += reap;
 		}
_

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: kswapd using __this_cpu_add() in preemptible code
  2014-03-18 21:22   ` Andrew Morton
@ 2014-03-19  6:20     ` Sergey Senozhatsky
  -1 siblings, 0 replies; 10+ messages in thread
From: Sergey Senozhatsky @ 2014-03-19  6:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Alexander Viro, Mel Gorman, Michal Hocko, Rik van Riel,
	Johannes Weiner, linux-fsdevel, linux-mm, linux-kernel,
	Christoph Lameter

On (03/18/14 14:22), Andrew Morton wrote:
> On Tue, 18 Mar 2014 21:53:30 +0300 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:
> 
> > Hello gentlemen,
> > 
> > Commit 589a606f9539663f162e4a110d117527833b58a4 ("percpu: add preemption
> > checks to __this_cpu ops") added preempt check to used in __count_vm_events()
> > __this_cpu ops, causing the following kswapd warning:
> > 
> >  BUG: using __this_cpu_add() in preemptible [00000000] code: kswapd0/56
> >  caller is __this_cpu_preempt_check+0x2b/0x2d
> >  Call Trace:
> >  [<ffffffff813b8d4d>] dump_stack+0x4e/0x7a
> >  [<ffffffff8121366f>] check_preemption_disabled+0xce/0xdd
> >  [<ffffffff812136bb>] __this_cpu_preempt_check+0x2b/0x2d
> >  [<ffffffff810f622e>] inode_lru_isolate+0xed/0x197
> >  [<ffffffff810be43c>] list_lru_walk_node+0x7b/0x14c
> >  [<ffffffff810f6141>] ? iput+0x131/0x131
> >  [<ffffffff810f681f>] prune_icache_sb+0x35/0x4c
> >  [<ffffffff810e3951>] super_cache_scan+0xe3/0x143
> >  [<ffffffff810b1301>] shrink_slab_node+0x103/0x16f
> >  [<ffffffff810b19fd>] shrink_slab+0x75/0xe4
> >  [<ffffffff810b3f3d>] balance_pgdat+0x2fa/0x47f
> >  [<ffffffff810b4395>] kswapd+0x2d3/0x2fd
> >  [<ffffffff81068049>] ? __wake_up_sync+0xd/0xd
> >  [<ffffffff810b40c2>] ? balance_pgdat+0x47f/0x47f
> >  [<ffffffff81051e75>] kthread+0xd6/0xde
> >  [<ffffffff81051d9f>] ? kthread_create_on_node+0x162/0x162
> >  [<ffffffff813be5bc>] ret_from_fork+0x7c/0xb0
> >  [<ffffffff81051d9f>] ? kthread_create_on_node+0x162/0x162
> > 
> > 
> > list_lru_walk_node() seems to be the only place where __count_vm_events()
> > called with preemption enabled. remaining __count_vm_events() and
> > __count_vm_event() calls are done with preemption disabled (unless I
> > overlooked something).
> 
> Christoph caught one.  How does this look?
> 

Thank you.

Tested-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

	-ss

> 
> 
> From: Andrew Morton <akpm@linux-foundation.org>
> Subject: fs/inode.c:inode_lru_isolate(): use atomic count_vm_events()
> 
> "percpu: add preemption checks to __this_cpu ops" added preempt check to
> used in __count_vm_events() __this_cpu ops, causing the following kswapd
> warning:
> 
>  BUG: using __this_cpu_add() in preemptible [00000000] code: kswapd0/56
>  caller is __this_cpu_preempt_check+0x2b/0x2d
>  Call Trace:
>  [<ffffffff813b8d4d>] dump_stack+0x4e/0x7a
>  [<ffffffff8121366f>] check_preemption_disabled+0xce/0xdd
>  [<ffffffff812136bb>] __this_cpu_preempt_check+0x2b/0x2d
>  [<ffffffff810f622e>] inode_lru_isolate+0xed/0x197
>  [<ffffffff810be43c>] list_lru_walk_node+0x7b/0x14c
>  [<ffffffff810f6141>] ? iput+0x131/0x131
>  [<ffffffff810f681f>] prune_icache_sb+0x35/0x4c
> 
> Switch from __count_vm_events() to the preempt-safe count_vm_events().
> 
> Reported-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Cc: Christoph Lameter <cl@linux-foundation.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  fs/inode.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff -puN fs/inode.c~fs-inodec-inode_lru_isolate-use-atomic-count_vm_events fs/inode.c
> --- a/fs/inode.c~fs-inodec-inode_lru_isolate-use-atomic-count_vm_events
> +++ a/fs/inode.c
> @@ -722,9 +722,9 @@ inode_lru_isolate(struct list_head *item
>  			unsigned long reap;
>  			reap = invalidate_mapping_pages(&inode->i_data, 0, -1);
>  			if (current_is_kswapd())
> -				__count_vm_events(KSWAPD_INODESTEAL, reap);
> +				count_vm_events(KSWAPD_INODESTEAL, reap);
>  			else
> -				__count_vm_events(PGINODESTEAL, reap);
> +				count_vm_events(PGINODESTEAL, reap);
>  			if (current->reclaim_state)
>  				current->reclaim_state->reclaimed_slab += reap;
>  		}
> _
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: kswapd using __this_cpu_add() in preemptible code
@ 2014-03-19  6:20     ` Sergey Senozhatsky
  0 siblings, 0 replies; 10+ messages in thread
From: Sergey Senozhatsky @ 2014-03-19  6:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Alexander Viro, Mel Gorman, Michal Hocko, Rik van Riel,
	Johannes Weiner, linux-fsdevel, linux-mm, linux-kernel,
	Christoph Lameter

On (03/18/14 14:22), Andrew Morton wrote:
> On Tue, 18 Mar 2014 21:53:30 +0300 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:
> 
> > Hello gentlemen,
> > 
> > Commit 589a606f9539663f162e4a110d117527833b58a4 ("percpu: add preemption
> > checks to __this_cpu ops") added preempt check to used in __count_vm_events()
> > __this_cpu ops, causing the following kswapd warning:
> > 
> >  BUG: using __this_cpu_add() in preemptible [00000000] code: kswapd0/56
> >  caller is __this_cpu_preempt_check+0x2b/0x2d
> >  Call Trace:
> >  [<ffffffff813b8d4d>] dump_stack+0x4e/0x7a
> >  [<ffffffff8121366f>] check_preemption_disabled+0xce/0xdd
> >  [<ffffffff812136bb>] __this_cpu_preempt_check+0x2b/0x2d
> >  [<ffffffff810f622e>] inode_lru_isolate+0xed/0x197
> >  [<ffffffff810be43c>] list_lru_walk_node+0x7b/0x14c
> >  [<ffffffff810f6141>] ? iput+0x131/0x131
> >  [<ffffffff810f681f>] prune_icache_sb+0x35/0x4c
> >  [<ffffffff810e3951>] super_cache_scan+0xe3/0x143
> >  [<ffffffff810b1301>] shrink_slab_node+0x103/0x16f
> >  [<ffffffff810b19fd>] shrink_slab+0x75/0xe4
> >  [<ffffffff810b3f3d>] balance_pgdat+0x2fa/0x47f
> >  [<ffffffff810b4395>] kswapd+0x2d3/0x2fd
> >  [<ffffffff81068049>] ? __wake_up_sync+0xd/0xd
> >  [<ffffffff810b40c2>] ? balance_pgdat+0x47f/0x47f
> >  [<ffffffff81051e75>] kthread+0xd6/0xde
> >  [<ffffffff81051d9f>] ? kthread_create_on_node+0x162/0x162
> >  [<ffffffff813be5bc>] ret_from_fork+0x7c/0xb0
> >  [<ffffffff81051d9f>] ? kthread_create_on_node+0x162/0x162
> > 
> > 
> > list_lru_walk_node() seems to be the only place where __count_vm_events()
> > called with preemption enabled. remaining __count_vm_events() and
> > __count_vm_event() calls are done with preemption disabled (unless I
> > overlooked something).
> 
> Christoph caught one.  How does this look?
> 

Thank you.

Tested-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

	-ss

> 
> 
> From: Andrew Morton <akpm@linux-foundation.org>
> Subject: fs/inode.c:inode_lru_isolate(): use atomic count_vm_events()
> 
> "percpu: add preemption checks to __this_cpu ops" added preempt check to
> used in __count_vm_events() __this_cpu ops, causing the following kswapd
> warning:
> 
>  BUG: using __this_cpu_add() in preemptible [00000000] code: kswapd0/56
>  caller is __this_cpu_preempt_check+0x2b/0x2d
>  Call Trace:
>  [<ffffffff813b8d4d>] dump_stack+0x4e/0x7a
>  [<ffffffff8121366f>] check_preemption_disabled+0xce/0xdd
>  [<ffffffff812136bb>] __this_cpu_preempt_check+0x2b/0x2d
>  [<ffffffff810f622e>] inode_lru_isolate+0xed/0x197
>  [<ffffffff810be43c>] list_lru_walk_node+0x7b/0x14c
>  [<ffffffff810f6141>] ? iput+0x131/0x131
>  [<ffffffff810f681f>] prune_icache_sb+0x35/0x4c
> 
> Switch from __count_vm_events() to the preempt-safe count_vm_events().
> 
> Reported-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Cc: Christoph Lameter <cl@linux-foundation.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  fs/inode.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff -puN fs/inode.c~fs-inodec-inode_lru_isolate-use-atomic-count_vm_events fs/inode.c
> --- a/fs/inode.c~fs-inodec-inode_lru_isolate-use-atomic-count_vm_events
> +++ a/fs/inode.c
> @@ -722,9 +722,9 @@ inode_lru_isolate(struct list_head *item
>  			unsigned long reap;
>  			reap = invalidate_mapping_pages(&inode->i_data, 0, -1);
>  			if (current_is_kswapd())
> -				__count_vm_events(KSWAPD_INODESTEAL, reap);
> +				count_vm_events(KSWAPD_INODESTEAL, reap);
>  			else
> -				__count_vm_events(PGINODESTEAL, reap);
> +				count_vm_events(PGINODESTEAL, reap);
>  			if (current->reclaim_state)
>  				current->reclaim_state->reclaimed_slab += reap;
>  		}
> _
> 

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: kswapd using __this_cpu_add() in preemptible code
  2014-03-18 21:22   ` Andrew Morton
@ 2014-03-20  1:01     ` Dave Chinner
  -1 siblings, 0 replies; 10+ messages in thread
From: Dave Chinner @ 2014-03-20  1:01 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Sergey Senozhatsky, Alexander Viro, Mel Gorman, Michal Hocko,
	Rik van Riel, Johannes Weiner, linux-fsdevel, linux-mm,
	linux-kernel, Christoph Lameter

On Tue, Mar 18, 2014 at 02:22:16PM -0700, Andrew Morton wrote:
> On Tue, 18 Mar 2014 21:53:30 +0300 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:
> 
> > Hello gentlemen,
> > 
> > Commit 589a606f9539663f162e4a110d117527833b58a4 ("percpu: add preemption
> > checks to __this_cpu ops") added preempt check to used in __count_vm_events()
> > __this_cpu ops, causing the following kswapd warning:
> > 
> >  BUG: using __this_cpu_add() in preemptible [00000000] code: kswapd0/56
> >  caller is __this_cpu_preempt_check+0x2b/0x2d
> >  Call Trace:
> >  [<ffffffff813b8d4d>] dump_stack+0x4e/0x7a
> >  [<ffffffff8121366f>] check_preemption_disabled+0xce/0xdd
> >  [<ffffffff812136bb>] __this_cpu_preempt_check+0x2b/0x2d
> >  [<ffffffff810f622e>] inode_lru_isolate+0xed/0x197
> >  [<ffffffff810be43c>] list_lru_walk_node+0x7b/0x14c
> >  [<ffffffff810f6141>] ? iput+0x131/0x131
> >  [<ffffffff810f681f>] prune_icache_sb+0x35/0x4c
> >  [<ffffffff810e3951>] super_cache_scan+0xe3/0x143
> >  [<ffffffff810b1301>] shrink_slab_node+0x103/0x16f
> >  [<ffffffff810b19fd>] shrink_slab+0x75/0xe4
> >  [<ffffffff810b3f3d>] balance_pgdat+0x2fa/0x47f
> >  [<ffffffff810b4395>] kswapd+0x2d3/0x2fd
> >  [<ffffffff81068049>] ? __wake_up_sync+0xd/0xd
> >  [<ffffffff810b40c2>] ? balance_pgdat+0x47f/0x47f
> >  [<ffffffff81051e75>] kthread+0xd6/0xde
> >  [<ffffffff81051d9f>] ? kthread_create_on_node+0x162/0x162
> >  [<ffffffff813be5bc>] ret_from_fork+0x7c/0xb0
> >  [<ffffffff81051d9f>] ? kthread_create_on_node+0x162/0x162
> > 
> > 
> > list_lru_walk_node() seems to be the only place where __count_vm_events()
> > called with preemption enabled. remaining __count_vm_events() and
> > __count_vm_event() calls are done with preemption disabled (unless I
> > overlooked something).
> 
> Christoph caught one.  How does this look?
> 
> 
> 
> From: Andrew Morton <akpm@linux-foundation.org>
> Subject: fs/inode.c:inode_lru_isolate(): use atomic count_vm_events()
> 
> "percpu: add preemption checks to __this_cpu ops" added preempt check to
> used in __count_vm_events() __this_cpu ops, causing the following kswapd
> warning:
> 
>  BUG: using __this_cpu_add() in preemptible [00000000] code: kswapd0/56
>  caller is __this_cpu_preempt_check+0x2b/0x2d
>  Call Trace:
>  [<ffffffff813b8d4d>] dump_stack+0x4e/0x7a
>  [<ffffffff8121366f>] check_preemption_disabled+0xce/0xdd
>  [<ffffffff812136bb>] __this_cpu_preempt_check+0x2b/0x2d
>  [<ffffffff810f622e>] inode_lru_isolate+0xed/0x197
>  [<ffffffff810be43c>] list_lru_walk_node+0x7b/0x14c
>  [<ffffffff810f6141>] ? iput+0x131/0x131
>  [<ffffffff810f681f>] prune_icache_sb+0x35/0x4c
> 
> Switch from __count_vm_events() to the preempt-safe count_vm_events().
> 
> Reported-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Cc: Christoph Lameter <cl@linux-foundation.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  fs/inode.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff -puN fs/inode.c~fs-inodec-inode_lru_isolate-use-atomic-count_vm_events fs/inode.c
> --- a/fs/inode.c~fs-inodec-inode_lru_isolate-use-atomic-count_vm_events
> +++ a/fs/inode.c
> @@ -722,9 +722,9 @@ inode_lru_isolate(struct list_head *item
>  			unsigned long reap;
>  			reap = invalidate_mapping_pages(&inode->i_data, 0, -1);
>  			if (current_is_kswapd())
> -				__count_vm_events(KSWAPD_INODESTEAL, reap);
> +				count_vm_events(KSWAPD_INODESTEAL, reap);
>  			else
> -				__count_vm_events(PGINODESTEAL, reap);
> +				count_vm_events(PGINODESTEAL, reap);
>  			if (current->reclaim_state)
>  				current->reclaim_state->reclaimed_slab += reap;
>  		}

Acked-by: Dave Chinner <dchinner@redhat.com>

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: kswapd using __this_cpu_add() in preemptible code
@ 2014-03-20  1:01     ` Dave Chinner
  0 siblings, 0 replies; 10+ messages in thread
From: Dave Chinner @ 2014-03-20  1:01 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Sergey Senozhatsky, Alexander Viro, Mel Gorman, Michal Hocko,
	Rik van Riel, Johannes Weiner, linux-fsdevel, linux-mm,
	linux-kernel, Christoph Lameter

On Tue, Mar 18, 2014 at 02:22:16PM -0700, Andrew Morton wrote:
> On Tue, 18 Mar 2014 21:53:30 +0300 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:
> 
> > Hello gentlemen,
> > 
> > Commit 589a606f9539663f162e4a110d117527833b58a4 ("percpu: add preemption
> > checks to __this_cpu ops") added preempt check to used in __count_vm_events()
> > __this_cpu ops, causing the following kswapd warning:
> > 
> >  BUG: using __this_cpu_add() in preemptible [00000000] code: kswapd0/56
> >  caller is __this_cpu_preempt_check+0x2b/0x2d
> >  Call Trace:
> >  [<ffffffff813b8d4d>] dump_stack+0x4e/0x7a
> >  [<ffffffff8121366f>] check_preemption_disabled+0xce/0xdd
> >  [<ffffffff812136bb>] __this_cpu_preempt_check+0x2b/0x2d
> >  [<ffffffff810f622e>] inode_lru_isolate+0xed/0x197
> >  [<ffffffff810be43c>] list_lru_walk_node+0x7b/0x14c
> >  [<ffffffff810f6141>] ? iput+0x131/0x131
> >  [<ffffffff810f681f>] prune_icache_sb+0x35/0x4c
> >  [<ffffffff810e3951>] super_cache_scan+0xe3/0x143
> >  [<ffffffff810b1301>] shrink_slab_node+0x103/0x16f
> >  [<ffffffff810b19fd>] shrink_slab+0x75/0xe4
> >  [<ffffffff810b3f3d>] balance_pgdat+0x2fa/0x47f
> >  [<ffffffff810b4395>] kswapd+0x2d3/0x2fd
> >  [<ffffffff81068049>] ? __wake_up_sync+0xd/0xd
> >  [<ffffffff810b40c2>] ? balance_pgdat+0x47f/0x47f
> >  [<ffffffff81051e75>] kthread+0xd6/0xde
> >  [<ffffffff81051d9f>] ? kthread_create_on_node+0x162/0x162
> >  [<ffffffff813be5bc>] ret_from_fork+0x7c/0xb0
> >  [<ffffffff81051d9f>] ? kthread_create_on_node+0x162/0x162
> > 
> > 
> > list_lru_walk_node() seems to be the only place where __count_vm_events()
> > called with preemption enabled. remaining __count_vm_events() and
> > __count_vm_event() calls are done with preemption disabled (unless I
> > overlooked something).
> 
> Christoph caught one.  How does this look?
> 
> 
> 
> From: Andrew Morton <akpm@linux-foundation.org>
> Subject: fs/inode.c:inode_lru_isolate(): use atomic count_vm_events()
> 
> "percpu: add preemption checks to __this_cpu ops" added preempt check to
> used in __count_vm_events() __this_cpu ops, causing the following kswapd
> warning:
> 
>  BUG: using __this_cpu_add() in preemptible [00000000] code: kswapd0/56
>  caller is __this_cpu_preempt_check+0x2b/0x2d
>  Call Trace:
>  [<ffffffff813b8d4d>] dump_stack+0x4e/0x7a
>  [<ffffffff8121366f>] check_preemption_disabled+0xce/0xdd
>  [<ffffffff812136bb>] __this_cpu_preempt_check+0x2b/0x2d
>  [<ffffffff810f622e>] inode_lru_isolate+0xed/0x197
>  [<ffffffff810be43c>] list_lru_walk_node+0x7b/0x14c
>  [<ffffffff810f6141>] ? iput+0x131/0x131
>  [<ffffffff810f681f>] prune_icache_sb+0x35/0x4c
> 
> Switch from __count_vm_events() to the preempt-safe count_vm_events().
> 
> Reported-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Cc: Christoph Lameter <cl@linux-foundation.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  fs/inode.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff -puN fs/inode.c~fs-inodec-inode_lru_isolate-use-atomic-count_vm_events fs/inode.c
> --- a/fs/inode.c~fs-inodec-inode_lru_isolate-use-atomic-count_vm_events
> +++ a/fs/inode.c
> @@ -722,9 +722,9 @@ inode_lru_isolate(struct list_head *item
>  			unsigned long reap;
>  			reap = invalidate_mapping_pages(&inode->i_data, 0, -1);
>  			if (current_is_kswapd())
> -				__count_vm_events(KSWAPD_INODESTEAL, reap);
> +				count_vm_events(KSWAPD_INODESTEAL, reap);
>  			else
> -				__count_vm_events(PGINODESTEAL, reap);
> +				count_vm_events(PGINODESTEAL, reap);
>  			if (current->reclaim_state)
>  				current->reclaim_state->reclaimed_slab += reap;
>  		}

Acked-by: Dave Chinner <dchinner@redhat.com>

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: kswapd using __this_cpu_add() in preemptible code
  2014-03-18 21:22   ` Andrew Morton
@ 2014-03-20 14:04     ` Christoph Lameter
  -1 siblings, 0 replies; 10+ messages in thread
From: Christoph Lameter @ 2014-03-20 14:04 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Sergey Senozhatsky, Alexander Viro, Mel Gorman, Michal Hocko,
	Rik van Riel, Johannes Weiner, linux-fsdevel, linux-mm,
	linux-kernel

On Tue, 18 Mar 2014, Andrew Morton wrote:

> Christoph caught one.  How does this look?

The fundamental decision to be made here is if we want the counter
overhead coming on platforms that do not have lockless percpu atomics
and therefore would require an irq on/off sequence for safe counter
increments.

So far we have said that we do allow the counters to be racy for
performance sake. Your patch would remove the races.

If we want to keep the races and the performance than we need to change
__count_vm_events to use raw_cpu_add instead of __this_cpu_add.


Subject: vmstat: Use raw_cpu_ops to avoid false positives on preemption checks

vm counters are allowed to be racy. Use raw_cpu_ops to avoid preemption checks.

Signed-off-by: Christoph Lameter <cl@linux.com>

Index: linux/include/linux/vmstat.h
===================================================================
--- linux.orig/include/linux/vmstat.h	2014-02-10 08:54:02.318697828 -0600
+++ linux/include/linux/vmstat.h	2014-03-20 09:02:05.132852038 -0500
@@ -29,7 +29,7 @@ DECLARE_PER_CPU(struct vm_event_state, v

 static inline void __count_vm_event(enum vm_event_item item)
 {
-	__this_cpu_inc(vm_event_states.event[item]);
+	raw_cpu_inc(vm_event_states.event[item]);
 }

 static inline void count_vm_event(enum vm_event_item item)
@@ -39,7 +39,7 @@ static inline void count_vm_event(enum v

 static inline void __count_vm_events(enum vm_event_item item, long delta)
 {
-	__this_cpu_add(vm_event_states.event[item], delta);
+	raw_cpu_add(vm_event_states.event[item], delta);
 }

 static inline void count_vm_events(enum vm_event_item item, long delta)

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: kswapd using __this_cpu_add() in preemptible code
@ 2014-03-20 14:04     ` Christoph Lameter
  0 siblings, 0 replies; 10+ messages in thread
From: Christoph Lameter @ 2014-03-20 14:04 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Sergey Senozhatsky, Alexander Viro, Mel Gorman, Michal Hocko,
	Rik van Riel, Johannes Weiner, linux-fsdevel, linux-mm,
	linux-kernel

On Tue, 18 Mar 2014, Andrew Morton wrote:

> Christoph caught one.  How does this look?

The fundamental decision to be made here is if we want the counter
overhead coming on platforms that do not have lockless percpu atomics
and therefore would require an irq on/off sequence for safe counter
increments.

So far we have said that we do allow the counters to be racy for
performance sake. Your patch would remove the races.

If we want to keep the races and the performance than we need to change
__count_vm_events to use raw_cpu_add instead of __this_cpu_add.


Subject: vmstat: Use raw_cpu_ops to avoid false positives on preemption checks

vm counters are allowed to be racy. Use raw_cpu_ops to avoid preemption checks.

Signed-off-by: Christoph Lameter <cl@linux.com>

Index: linux/include/linux/vmstat.h
===================================================================
--- linux.orig/include/linux/vmstat.h	2014-02-10 08:54:02.318697828 -0600
+++ linux/include/linux/vmstat.h	2014-03-20 09:02:05.132852038 -0500
@@ -29,7 +29,7 @@ DECLARE_PER_CPU(struct vm_event_state, v

 static inline void __count_vm_event(enum vm_event_item item)
 {
-	__this_cpu_inc(vm_event_states.event[item]);
+	raw_cpu_inc(vm_event_states.event[item]);
 }

 static inline void count_vm_event(enum vm_event_item item)
@@ -39,7 +39,7 @@ static inline void count_vm_event(enum v

 static inline void __count_vm_events(enum vm_event_item item, long delta)
 {
-	__this_cpu_add(vm_event_states.event[item], delta);
+	raw_cpu_add(vm_event_states.event[item], delta);
 }

 static inline void count_vm_events(enum vm_event_item item, long delta)

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-03-20 14:04 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-18 18:53 kswapd using __this_cpu_add() in preemptible code Sergey Senozhatsky
2014-03-18 18:53 ` Sergey Senozhatsky
2014-03-18 21:22 ` Andrew Morton
2014-03-18 21:22   ` Andrew Morton
2014-03-19  6:20   ` Sergey Senozhatsky
2014-03-19  6:20     ` Sergey Senozhatsky
2014-03-20  1:01   ` Dave Chinner
2014-03-20  1:01     ` Dave Chinner
2014-03-20 14:04   ` Christoph Lameter
2014-03-20 14:04     ` Christoph Lameter

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.