From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sergey Senozhatsky Subject: kswapd using __this_cpu_add() in preemptible code Date: Tue, 18 Mar 2014 21:53:30 +0300 Message-ID: <20140318185329.GB430@swordfish> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Andrew Morton , Mel Gorman , Michal Hocko , Rik van Riel , Johannes Weiner , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org To: Alexander Viro Return-path: Content-Disposition: inline Sender: owner-linux-mm@kvack.org List-Id: linux-fsdevel.vger.kernel.org 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: [] dump_stack+0x4e/0x7a [] check_preemption_disabled+0xce/0xdd [] __this_cpu_preempt_check+0x2b/0x2d [] inode_lru_isolate+0xed/0x197 [] list_lru_walk_node+0x7b/0x14c [] ? iput+0x131/0x131 [] prune_icache_sb+0x35/0x4c [] super_cache_scan+0xe3/0x143 [] shrink_slab_node+0x103/0x16f [] shrink_slab+0x75/0xe4 [] balance_pgdat+0x2fa/0x47f [] kswapd+0x2d3/0x2fd [] ? __wake_up_sync+0xd/0xd [] ? balance_pgdat+0x47f/0x47f [] kthread+0xd6/0xde [] ? kthread_create_on_node+0x162/0x162 [] ret_from_fork+0x7c/0xb0 [] ? 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Morton Subject: Re: kswapd using __this_cpu_add() in preemptible code Date: Tue, 18 Mar 2014 14:22:16 -0700 Message-ID: <20140318142216.317bf986d10a564881791100@linux-foundation.org> References: <20140318185329.GB430@swordfish> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Alexander Viro , Mel Gorman , Michal Hocko , Rik van Riel , Johannes Weiner , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Christoph Lameter To: Sergey Senozhatsky Return-path: In-Reply-To: <20140318185329.GB430@swordfish> Sender: owner-linux-mm@kvack.org List-Id: linux-fsdevel.vger.kernel.org On Tue, 18 Mar 2014 21:53:30 +0300 Sergey Senozhatsky 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: > [] dump_stack+0x4e/0x7a > [] check_preemption_disabled+0xce/0xdd > [] __this_cpu_preempt_check+0x2b/0x2d > [] inode_lru_isolate+0xed/0x197 > [] list_lru_walk_node+0x7b/0x14c > [] ? iput+0x131/0x131 > [] prune_icache_sb+0x35/0x4c > [] super_cache_scan+0xe3/0x143 > [] shrink_slab_node+0x103/0x16f > [] shrink_slab+0x75/0xe4 > [] balance_pgdat+0x2fa/0x47f > [] kswapd+0x2d3/0x2fd > [] ? __wake_up_sync+0xd/0xd > [] ? balance_pgdat+0x47f/0x47f > [] kthread+0xd6/0xde > [] ? kthread_create_on_node+0x162/0x162 > [] ret_from_fork+0x7c/0xb0 > [] ? 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 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: [] dump_stack+0x4e/0x7a [] check_preemption_disabled+0xce/0xdd [] __this_cpu_preempt_check+0x2b/0x2d [] inode_lru_isolate+0xed/0x197 [] list_lru_walk_node+0x7b/0x14c [] ? iput+0x131/0x131 [] prune_icache_sb+0x35/0x4c Switch from __count_vm_events() to the preempt-safe count_vm_events(). Reported-by: Sergey Senozhatsky Cc: Christoph Lameter Signed-off-by: Andrew Morton --- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sergey Senozhatsky Subject: Re: kswapd using __this_cpu_add() in preemptible code Date: Wed, 19 Mar 2014 09:20:00 +0300 Message-ID: <20140319062000.GA435@swordfish> References: <20140318185329.GB430@swordfish> <20140318142216.317bf986d10a564881791100@linux-foundation.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Alexander Viro , Mel Gorman , Michal Hocko , Rik van Riel , Johannes Weiner , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Christoph Lameter To: Andrew Morton Return-path: Content-Disposition: inline In-Reply-To: <20140318142216.317bf986d10a564881791100@linux-foundation.org> Sender: owner-linux-mm@kvack.org List-Id: linux-fsdevel.vger.kernel.org On (03/18/14 14:22), Andrew Morton wrote: > On Tue, 18 Mar 2014 21:53:30 +0300 Sergey Senozhatsky 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: > > [] dump_stack+0x4e/0x7a > > [] check_preemption_disabled+0xce/0xdd > > [] __this_cpu_preempt_check+0x2b/0x2d > > [] inode_lru_isolate+0xed/0x197 > > [] list_lru_walk_node+0x7b/0x14c > > [] ? iput+0x131/0x131 > > [] prune_icache_sb+0x35/0x4c > > [] super_cache_scan+0xe3/0x143 > > [] shrink_slab_node+0x103/0x16f > > [] shrink_slab+0x75/0xe4 > > [] balance_pgdat+0x2fa/0x47f > > [] kswapd+0x2d3/0x2fd > > [] ? __wake_up_sync+0xd/0xd > > [] ? balance_pgdat+0x47f/0x47f > > [] kthread+0xd6/0xde > > [] ? kthread_create_on_node+0x162/0x162 > > [] ret_from_fork+0x7c/0xb0 > > [] ? 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 -ss > > > From: Andrew Morton > 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: > [] dump_stack+0x4e/0x7a > [] check_preemption_disabled+0xce/0xdd > [] __this_cpu_preempt_check+0x2b/0x2d > [] inode_lru_isolate+0xed/0x197 > [] list_lru_walk_node+0x7b/0x14c > [] ? iput+0x131/0x131 > [] prune_icache_sb+0x35/0x4c > > Switch from __count_vm_events() to the preempt-safe count_vm_events(). > > Reported-by: Sergey Senozhatsky > Cc: Christoph Lameter > Signed-off-by: Andrew Morton > --- > > 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Chinner Subject: Re: kswapd using __this_cpu_add() in preemptible code Date: Thu, 20 Mar 2014 12:01:10 +1100 Message-ID: <20140320010110.GJ7072@dastard> References: <20140318185329.GB430@swordfish> <20140318142216.317bf986d10a564881791100@linux-foundation.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Sergey Senozhatsky , Alexander Viro , Mel Gorman , Michal Hocko , Rik van Riel , Johannes Weiner , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Christoph Lameter To: Andrew Morton Return-path: Content-Disposition: inline In-Reply-To: <20140318142216.317bf986d10a564881791100@linux-foundation.org> Sender: owner-linux-mm@kvack.org List-Id: linux-fsdevel.vger.kernel.org On Tue, Mar 18, 2014 at 02:22:16PM -0700, Andrew Morton wrote: > On Tue, 18 Mar 2014 21:53:30 +0300 Sergey Senozhatsky 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: > > [] dump_stack+0x4e/0x7a > > [] check_preemption_disabled+0xce/0xdd > > [] __this_cpu_preempt_check+0x2b/0x2d > > [] inode_lru_isolate+0xed/0x197 > > [] list_lru_walk_node+0x7b/0x14c > > [] ? iput+0x131/0x131 > > [] prune_icache_sb+0x35/0x4c > > [] super_cache_scan+0xe3/0x143 > > [] shrink_slab_node+0x103/0x16f > > [] shrink_slab+0x75/0xe4 > > [] balance_pgdat+0x2fa/0x47f > > [] kswapd+0x2d3/0x2fd > > [] ? __wake_up_sync+0xd/0xd > > [] ? balance_pgdat+0x47f/0x47f > > [] kthread+0xd6/0xde > > [] ? kthread_create_on_node+0x162/0x162 > > [] ret_from_fork+0x7c/0xb0 > > [] ? 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 > 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: > [] dump_stack+0x4e/0x7a > [] check_preemption_disabled+0xce/0xdd > [] __this_cpu_preempt_check+0x2b/0x2d > [] inode_lru_isolate+0xed/0x197 > [] list_lru_walk_node+0x7b/0x14c > [] ? iput+0x131/0x131 > [] prune_icache_sb+0x35/0x4c > > Switch from __count_vm_events() to the preempt-safe count_vm_events(). > > Reported-by: Sergey Senozhatsky > Cc: Christoph Lameter > Signed-off-by: Andrew Morton > --- > > 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 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Lameter Subject: Re: kswapd using __this_cpu_add() in preemptible code Date: Thu, 20 Mar 2014 09:04:50 -0500 (CDT) Message-ID: References: <20140318185329.GB430@swordfish> <20140318142216.317bf986d10a564881791100@linux-foundation.org> Content-Type: TEXT/PLAIN; charset=US-ASCII Cc: Sergey Senozhatsky , Alexander Viro , Mel Gorman , Michal Hocko , Rik van Riel , Johannes Weiner , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org To: Andrew Morton Return-path: In-Reply-To: <20140318142216.317bf986d10a564881791100@linux-foundation.org> Sender: owner-linux-mm@kvack.org List-Id: linux-fsdevel.vger.kernel.org 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 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757593AbaCRS5W (ORCPT ); Tue, 18 Mar 2014 14:57:22 -0400 Received: from mail-ee0-f52.google.com ([74.125.83.52]:37443 "EHLO mail-ee0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757531AbaCRS5U (ORCPT ); Tue, 18 Mar 2014 14:57:20 -0400 Date: Tue, 18 Mar 2014 21:53:30 +0300 From: Sergey Senozhatsky To: Alexander Viro Cc: Andrew Morton , Mel Gorman , Michal Hocko , Rik van Riel , Johannes Weiner , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: kswapd using __this_cpu_add() in preemptible code Message-ID: <20140318185329.GB430@swordfish> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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: [] dump_stack+0x4e/0x7a [] check_preemption_disabled+0xce/0xdd [] __this_cpu_preempt_check+0x2b/0x2d [] inode_lru_isolate+0xed/0x197 [] list_lru_walk_node+0x7b/0x14c [] ? iput+0x131/0x131 [] prune_icache_sb+0x35/0x4c [] super_cache_scan+0xe3/0x143 [] shrink_slab_node+0x103/0x16f [] shrink_slab+0x75/0xe4 [] balance_pgdat+0x2fa/0x47f [] kswapd+0x2d3/0x2fd [] ? __wake_up_sync+0xd/0xd [] ? balance_pgdat+0x47f/0x47f [] kthread+0xd6/0xde [] ? kthread_create_on_node+0x162/0x162 [] ret_from_fork+0x7c/0xb0 [] ? 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758036AbaCRVWU (ORCPT ); Tue, 18 Mar 2014 17:22:20 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:60410 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751631AbaCRVWS (ORCPT ); Tue, 18 Mar 2014 17:22:18 -0400 Date: Tue, 18 Mar 2014 14:22:16 -0700 From: Andrew Morton To: Sergey Senozhatsky Cc: Alexander Viro , Mel Gorman , Michal Hocko , Rik van Riel , Johannes Weiner , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Christoph Lameter Subject: Re: kswapd using __this_cpu_add() in preemptible code Message-Id: <20140318142216.317bf986d10a564881791100@linux-foundation.org> In-Reply-To: <20140318185329.GB430@swordfish> References: <20140318185329.GB430@swordfish> X-Mailer: Sylpheed 3.2.0beta5 (GTK+ 2.24.10; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 18 Mar 2014 21:53:30 +0300 Sergey Senozhatsky 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: > [] dump_stack+0x4e/0x7a > [] check_preemption_disabled+0xce/0xdd > [] __this_cpu_preempt_check+0x2b/0x2d > [] inode_lru_isolate+0xed/0x197 > [] list_lru_walk_node+0x7b/0x14c > [] ? iput+0x131/0x131 > [] prune_icache_sb+0x35/0x4c > [] super_cache_scan+0xe3/0x143 > [] shrink_slab_node+0x103/0x16f > [] shrink_slab+0x75/0xe4 > [] balance_pgdat+0x2fa/0x47f > [] kswapd+0x2d3/0x2fd > [] ? __wake_up_sync+0xd/0xd > [] ? balance_pgdat+0x47f/0x47f > [] kthread+0xd6/0xde > [] ? kthread_create_on_node+0x162/0x162 > [] ret_from_fork+0x7c/0xb0 > [] ? 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 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: [] dump_stack+0x4e/0x7a [] check_preemption_disabled+0xce/0xdd [] __this_cpu_preempt_check+0x2b/0x2d [] inode_lru_isolate+0xed/0x197 [] list_lru_walk_node+0x7b/0x14c [] ? iput+0x131/0x131 [] prune_icache_sb+0x35/0x4c Switch from __count_vm_events() to the preempt-safe count_vm_events(). Reported-by: Sergey Senozhatsky Cc: Christoph Lameter Signed-off-by: Andrew Morton --- 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; } _ From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755174AbaCSGXz (ORCPT ); Wed, 19 Mar 2014 02:23:55 -0400 Received: from mail-lb0-f169.google.com ([209.85.217.169]:53153 "EHLO mail-lb0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751313AbaCSGXw (ORCPT ); Wed, 19 Mar 2014 02:23:52 -0400 Date: Wed, 19 Mar 2014 09:20:00 +0300 From: Sergey Senozhatsky To: Andrew Morton Cc: Alexander Viro , Mel Gorman , Michal Hocko , Rik van Riel , Johannes Weiner , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Christoph Lameter Subject: Re: kswapd using __this_cpu_add() in preemptible code Message-ID: <20140319062000.GA435@swordfish> References: <20140318185329.GB430@swordfish> <20140318142216.317bf986d10a564881791100@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140318142216.317bf986d10a564881791100@linux-foundation.org> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On (03/18/14 14:22), Andrew Morton wrote: > On Tue, 18 Mar 2014 21:53:30 +0300 Sergey Senozhatsky 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: > > [] dump_stack+0x4e/0x7a > > [] check_preemption_disabled+0xce/0xdd > > [] __this_cpu_preempt_check+0x2b/0x2d > > [] inode_lru_isolate+0xed/0x197 > > [] list_lru_walk_node+0x7b/0x14c > > [] ? iput+0x131/0x131 > > [] prune_icache_sb+0x35/0x4c > > [] super_cache_scan+0xe3/0x143 > > [] shrink_slab_node+0x103/0x16f > > [] shrink_slab+0x75/0xe4 > > [] balance_pgdat+0x2fa/0x47f > > [] kswapd+0x2d3/0x2fd > > [] ? __wake_up_sync+0xd/0xd > > [] ? balance_pgdat+0x47f/0x47f > > [] kthread+0xd6/0xde > > [] ? kthread_create_on_node+0x162/0x162 > > [] ret_from_fork+0x7c/0xb0 > > [] ? 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 -ss > > > From: Andrew Morton > 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: > [] dump_stack+0x4e/0x7a > [] check_preemption_disabled+0xce/0xdd > [] __this_cpu_preempt_check+0x2b/0x2d > [] inode_lru_isolate+0xed/0x197 > [] list_lru_walk_node+0x7b/0x14c > [] ? iput+0x131/0x131 > [] prune_icache_sb+0x35/0x4c > > Switch from __count_vm_events() to the preempt-safe count_vm_events(). > > Reported-by: Sergey Senozhatsky > Cc: Christoph Lameter > Signed-off-by: Andrew Morton > --- > > 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; > } > _ > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757589AbaCTBBc (ORCPT ); Wed, 19 Mar 2014 21:01:32 -0400 Received: from ipmail05.adl6.internode.on.net ([150.101.137.143]:37052 "EHLO ipmail05.adl6.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750825AbaCTBB3 (ORCPT ); Wed, 19 Mar 2014 21:01:29 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: Ag5SAE89KlN5LMWSPGdsb2JhbABagwaDR4UMuwyBHBcDAQEBATg1giUBAQEEJxMcDxQQCAMOBwIBCSUPBQ0YAwcMDhOHZQMQyQMNhyUXFow3ghgHhDgEjlCICoFsilqCD4kJKQ Date: Thu, 20 Mar 2014 12:01:10 +1100 From: Dave Chinner To: Andrew Morton Cc: Sergey Senozhatsky , Alexander Viro , Mel Gorman , Michal Hocko , Rik van Riel , Johannes Weiner , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Christoph Lameter Subject: Re: kswapd using __this_cpu_add() in preemptible code Message-ID: <20140320010110.GJ7072@dastard> References: <20140318185329.GB430@swordfish> <20140318142216.317bf986d10a564881791100@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140318142216.317bf986d10a564881791100@linux-foundation.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 18, 2014 at 02:22:16PM -0700, Andrew Morton wrote: > On Tue, 18 Mar 2014 21:53:30 +0300 Sergey Senozhatsky 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: > > [] dump_stack+0x4e/0x7a > > [] check_preemption_disabled+0xce/0xdd > > [] __this_cpu_preempt_check+0x2b/0x2d > > [] inode_lru_isolate+0xed/0x197 > > [] list_lru_walk_node+0x7b/0x14c > > [] ? iput+0x131/0x131 > > [] prune_icache_sb+0x35/0x4c > > [] super_cache_scan+0xe3/0x143 > > [] shrink_slab_node+0x103/0x16f > > [] shrink_slab+0x75/0xe4 > > [] balance_pgdat+0x2fa/0x47f > > [] kswapd+0x2d3/0x2fd > > [] ? __wake_up_sync+0xd/0xd > > [] ? balance_pgdat+0x47f/0x47f > > [] kthread+0xd6/0xde > > [] ? kthread_create_on_node+0x162/0x162 > > [] ret_from_fork+0x7c/0xb0 > > [] ? 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 > 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: > [] dump_stack+0x4e/0x7a > [] check_preemption_disabled+0xce/0xdd > [] __this_cpu_preempt_check+0x2b/0x2d > [] inode_lru_isolate+0xed/0x197 > [] list_lru_walk_node+0x7b/0x14c > [] ? iput+0x131/0x131 > [] prune_icache_sb+0x35/0x4c > > Switch from __count_vm_events() to the preempt-safe count_vm_events(). > > Reported-by: Sergey Senozhatsky > Cc: Christoph Lameter > Signed-off-by: Andrew Morton > --- > > 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 Cheers, Dave. -- Dave Chinner david@fromorbit.com From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758649AbaCTOEz (ORCPT ); Thu, 20 Mar 2014 10:04:55 -0400 Received: from qmta07.emeryville.ca.mail.comcast.net ([76.96.30.64]:50685 "EHLO qmta07.emeryville.ca.mail.comcast.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755870AbaCTOEx (ORCPT ); Thu, 20 Mar 2014 10:04:53 -0400 Date: Thu, 20 Mar 2014 09:04:50 -0500 (CDT) From: Christoph Lameter X-X-Sender: cl@nuc To: Andrew Morton cc: Sergey Senozhatsky , Alexander Viro , Mel Gorman , Michal Hocko , Rik van Riel , Johannes Weiner , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: kswapd using __this_cpu_add() in preemptible code In-Reply-To: <20140318142216.317bf986d10a564881791100@linux-foundation.org> Message-ID: References: <20140318185329.GB430@swordfish> <20140318142216.317bf986d10a564881791100@linux-foundation.org> Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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)