Re: [PATCH -next] slub: Replace __this_cpu_inc usage w/ SLUB_STATS
On Mon, 14 Apr 2014, Johannes Hirte wrote:
> kernel/watchdog.c:
>
> void touch_softlockup_watchdog(void)
> {
> __this_cpu_write(watchdog_touch_ts, 0);
> }
> EXPORT_SYMBOL(touch_softlockup_watchdog);
>
> Don't know if the change to this_cpu_write() is the right way here too.
Well yes lets change that to this_cpu_write(). I doubt there are
significant performance issues here.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] slub: Replace __this_cpu_inc usage w/ SLUB_STATS
On Thu, 6 Mar 2014 12:29:41 -0600 Josh Cartwright wrote: > On Thu, Mar 06, 2014 at 09:53:16AM -0600, Josh Cartwright wrote: > > Booting on my Samsung Series 9 laptop gives me loads and loads of > > BUGs triggered by __this_cpu_add(), making making the system > > completely unusable: > > > > [5.808326] BUG: using __this_cpu_add() in preemptible > > [] code: swapper/0/1 [5.812331] caller is > > __this_cpu_preempt_check+0x2b/0x30 [5.815654] CPU: 0 PID: 1 > > Comm: swapper/0 Not tainted > > 3.14.0-rc5-next-20140306-joshc-08290-g0ffb2fe #1 [5.819553] > > Hardware name: SAMSUNG ELECTRONICS CO., LTD. > > 900X3C/900X3D/900X3E/900X4C/900X4D/NP900X3E-A02US, BIOS P07ABK > > 04/09/2013 [5.823558] 8801182157c0 880118215790 > > 81a64cec [5.827177] 8801182157b0 > > 81462360 8800c3d553e0 ea00030f5500 [5.830744] > > 8801182157e8 814623bb 635f736968745f5f 29286464615f7570 > > [5.834134] Call Trace: [5.836848] [] > > dump_stack+0x4e/0x7a [5.839943] [] > > check_preemption_disabled+0xd0/0xe0 [5.842997] > > [] __this_cpu_preempt_check+0x2b/0x30 > > [5.846022] [] __slab_free+0x38/0x590 > > [5.848863] [] ? get_parent_ip+0xd/0x50 > > [5.850467] BUG: using __this_cpu_add() in preemptible > > [] code: khubd/36 [5.850472] caller is > > __this_cpu_preempt_check+0x2b/0x30 [5.859125] > > [] ? preempt_count_sub+0x6b/0xf0 [5.862521] > > [] ? _raw_spin_unlock_irqrestore+0x4a/0x80 > > [5.865599] [] ? > > __debug_check_no_obj_freed+0x13e/0x240 [5.868738] > > [] ? __this_cpu_preempt_check+0x2b/0x30 > > [5.871799] [] kfree+0x2f7/0x300 > > FWIW, it looks like the magic combination of options are: > - CONFIG_DEBUG_PREEMPT=y > - CONFIG_SLUB=y > - CONFIG_SLUB_STATS=y > > Looks like the new percpu() checks are complaining about SLUB's use of > __this_cpu_inc() for maintaining it's stat counters. The below patch > seems to fix it. > > Although, I'm wondering how exact these statistics need to be. Is > making them preemption safe even a concern? > Looks like there is a similar issue in touch_softlockup_watchdog too: Apr 14 14:56:01 localhost kernel: BUG: using __this_cpu_write() in preemptible [] code: systemd-udevd/1307 Apr 14 14:56:01 localhost kernel: caller is touch_softlockup_watchdog+0x11/0x1f Apr 14 14:56:01 localhost kernel: CPU: 0 PID: 1307 Comm: systemd-udevd Tainted: GW 3.15.0-rc1 #44 Apr 14 14:56:01 localhost kernel: Hardware name: Hewlett-Packard HP ProBook 6450b/146D, BIOS 68CDE Ver. F.23 06/13/2012 Apr 14 14:56:01 localhost kernel: 815b6385 813005a4 Apr 14 14:56:01 localhost kernel: 0032 03e8 810c63bc Apr 14 14:56:01 localhost kernel: 81332592 8800b4ea8800 8800b686e030 Apr 14 14:56:01 localhost kernel: Call Trace: Apr 14 14:56:01 localhost kernel: [] ? dump_stack+0x4a/0x75 Apr 14 14:56:01 localhost kernel: [] ? check_preemption_disabled+0xd6/0xe5 Apr 14 14:56:01 localhost kernel: [] ? touch_softlockup_watchdog+0x11/0x1f Apr 14 14:56:01 localhost kernel: [] ? acpi_os_stall+0x2f/0x36 Apr 14 14:56:01 localhost kernel: [] ? acpi_ex_system_do_stall+0x34/0x37 Apr 14 14:56:01 localhost kernel: [] ? acpi_ds_exec_end_op+0xcc/0x3d5 Apr 14 14:56:01 localhost kernel: [] ? acpi_ps_parse_loop+0x50c/0x564 Apr 14 14:56:01 localhost kernel: [] ? acpi_ps_parse_aml+0x93/0x26f Apr 14 14:56:01 localhost kernel: [] ? acpi_ps_execute_method+0x1b6/0x25f Apr 14 14:56:01 localhost kernel: [] ? acpi_ns_evaluate+0x1ba/0x247 Apr 14 14:56:01 localhost kernel: [] ? acpi_evaluate_object+0x122/0x231 Apr 14 14:56:01 localhost kernel: [] ? lis3lv02d_acpi_init+0x1c/0x27 [hp_accel] Apr 14 14:56:01 localhost kernel: [] ? lis3lv02d_poweron+0xe/0xca [lis3lv02d] Apr 14 14:56:01 localhost kernel: [] ? lis3lv02d_init_device+0x22a/0x4e5 [lis3lv02d] Apr 14 14:56:01 localhost kernel: [] ? lis3lv02d_add+0x10c/0x18a [hp_accel] Apr 14 14:56:01 localhost kernel: [] ? acpi_device_probe+0x3d/0xeb Apr 14 14:56:01 localhost kernel: [] ? driver_probe_device+0x97/0x1b8 Apr 14 14:56:01 localhost kernel: [] ? __driver_attach+0x58/0x78 Apr 14 14:56:01 localhost kernel: [] ? __device_attach+0x36/0x36 Apr 14 14:56:01 localhost kernel: [] ? bus_for_each_dev+0x73/0x7d Apr 14 14:56:01 localhost kernel: [] ? bus_add_driver+0x105/0x1ce Apr 14 14:56:01 localhost kernel: [] ? driver_register+0x88/0xc0 Apr 14 14:56:01 localhost kernel: [] ? 0xa005efff Apr 14 14:56:01 localhost kernel: [] ? do_one_initcall+0x7d/0x101 Apr 14 14:56:01 localhost kernel: [] ? notifier_call_chain+0x37/0x57 Apr 14 14:56:01 localhost kernel: [] ? __blocking_notifier_call_chain+0x53/0x60 Apr 14 14:56:01 localhost kernel: [] ? load_module+0x19f6/0x1ba7 Apr 14 14:56:01 localhost kernel: [] ? module_flags+0x74/0x74 Apr 14 14:56:01 localhost kernel: [] ?
Re: [PATCH -next] slub: Replace __this_cpu_inc usage w/ SLUB_STATS
On Thu, 6 Mar 2014 12:29:41 -0600 Josh Cartwright jo...@codeaurora.org wrote: On Thu, Mar 06, 2014 at 09:53:16AM -0600, Josh Cartwright wrote: Booting on my Samsung Series 9 laptop gives me loads and loads of BUGs triggered by __this_cpu_add(), making making the system completely unusable: [5.808326] BUG: using __this_cpu_add() in preemptible [] code: swapper/0/1 [5.812331] caller is __this_cpu_preempt_check+0x2b/0x30 [5.815654] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.14.0-rc5-next-20140306-joshc-08290-g0ffb2fe #1 [5.819553] Hardware name: SAMSUNG ELECTRONICS CO., LTD. 900X3C/900X3D/900X3E/900X4C/900X4D/NP900X3E-A02US, BIOS P07ABK 04/09/2013 [5.823558] 8801182157c0 880118215790 81a64cec [5.827177] 8801182157b0 81462360 8800c3d553e0 ea00030f5500 [5.830744] 8801182157e8 814623bb 635f736968745f5f 29286464615f7570 [5.834134] Call Trace: [5.836848] [81a64cec] dump_stack+0x4e/0x7a [5.839943] [81462360] check_preemption_disabled+0xd0/0xe0 [5.842997] [814623bb] __this_cpu_preempt_check+0x2b/0x30 [5.846022] [81a6331d] __slab_free+0x38/0x590 [5.848863] [811759dd] ? get_parent_ip+0xd/0x50 [5.850467] BUG: using __this_cpu_add() in preemptible [] code: khubd/36 [5.850472] caller is __this_cpu_preempt_check+0x2b/0x30 [5.859125] [81175b3b] ? preempt_count_sub+0x6b/0xf0 [5.862521] [81a7175a] ? _raw_spin_unlock_irqrestore+0x4a/0x80 [5.865599] [81462e5e] ? __debug_check_no_obj_freed+0x13e/0x240 [5.868738] [814623bb] ? __this_cpu_preempt_check+0x2b/0x30 [5.871799] [81287327] kfree+0x2f7/0x300 FWIW, it looks like the magic combination of options are: - CONFIG_DEBUG_PREEMPT=y - CONFIG_SLUB=y - CONFIG_SLUB_STATS=y Looks like the new percpu() checks are complaining about SLUB's use of __this_cpu_inc() for maintaining it's stat counters. The below patch seems to fix it. Although, I'm wondering how exact these statistics need to be. Is making them preemption safe even a concern? Looks like there is a similar issue in touch_softlockup_watchdog too: Apr 14 14:56:01 localhost kernel: BUG: using __this_cpu_write() in preemptible [] code: systemd-udevd/1307 Apr 14 14:56:01 localhost kernel: caller is touch_softlockup_watchdog+0x11/0x1f Apr 14 14:56:01 localhost kernel: CPU: 0 PID: 1307 Comm: systemd-udevd Tainted: GW 3.15.0-rc1 #44 Apr 14 14:56:01 localhost kernel: Hardware name: Hewlett-Packard HP ProBook 6450b/146D, BIOS 68CDE Ver. F.23 06/13/2012 Apr 14 14:56:01 localhost kernel: 815b6385 813005a4 Apr 14 14:56:01 localhost kernel: 0032 03e8 810c63bc Apr 14 14:56:01 localhost kernel: 81332592 8800b4ea8800 8800b686e030 Apr 14 14:56:01 localhost kernel: Call Trace: Apr 14 14:56:01 localhost kernel: [815b6385] ? dump_stack+0x4a/0x75 Apr 14 14:56:01 localhost kernel: [813005a4] ? check_preemption_disabled+0xd6/0xe5 Apr 14 14:56:01 localhost kernel: [810c63bc] ? touch_softlockup_watchdog+0x11/0x1f Apr 14 14:56:01 localhost kernel: [81332592] ? acpi_os_stall+0x2f/0x36 Apr 14 14:56:01 localhost kernel: [8134b64a] ? acpi_ex_system_do_stall+0x34/0x37 Apr 14 14:56:01 localhost kernel: [813411d4] ? acpi_ds_exec_end_op+0xcc/0x3d5 Apr 14 14:56:01 localhost kernel: [81351fcf] ? acpi_ps_parse_loop+0x50c/0x564 Apr 14 14:56:01 localhost kernel: [81352a21] ? acpi_ps_parse_aml+0x93/0x26f Apr 14 14:56:01 localhost kernel: [813531eb] ? acpi_ps_execute_method+0x1b6/0x25f Apr 14 14:56:01 localhost kernel: [8134debe] ? acpi_ns_evaluate+0x1ba/0x247 Apr 14 14:56:01 localhost kernel: [81350557] ? acpi_evaluate_object+0x122/0x231 Apr 14 14:56:01 localhost kernel: [a005a230] ? lis3lv02d_acpi_init+0x1c/0x27 [hp_accel] Apr 14 14:56:01 localhost kernel: [a005320a] ? lis3lv02d_poweron+0xe/0xca [lis3lv02d] Apr 14 14:56:01 localhost kernel: [a0053b16] ? lis3lv02d_init_device+0x22a/0x4e5 [lis3lv02d] Apr 14 14:56:01 localhost kernel: [a005a347] ? lis3lv02d_add+0x10c/0x18a [hp_accel] Apr 14 14:56:01 localhost kernel: [81335d82] ? acpi_device_probe+0x3d/0xeb Apr 14 14:56:01 localhost kernel: [81418e8b] ? driver_probe_device+0x97/0x1b8 Apr 14 14:56:01 localhost kernel: [8141903a] ? __driver_attach+0x58/0x78 Apr 14 14:56:01 localhost kernel: [81418fe2] ? __device_attach+0x36/0x36 Apr 14 14:56:01 localhost kernel: [81417650] ? bus_for_each_dev+0x73/0x7d Apr 14 14:56:01 localhost kernel: [814186f4] ? bus_add_driver+0x105/0x1ce Apr 14 14:56:01 localhost kernel: [81419577] ? driver_register+0x88/0xc0 Apr 14
Re: [PATCH -next] slub: Replace __this_cpu_inc usage w/ SLUB_STATS
On Mon, 14 Apr 2014, Johannes Hirte wrote:
kernel/watchdog.c:
void touch_softlockup_watchdog(void)
{
__this_cpu_write(watchdog_touch_ts, 0);
}
EXPORT_SYMBOL(touch_softlockup_watchdog);
Don't know if the change to this_cpu_write() is the right way here too.
Well yes lets change that to this_cpu_write(). I doubt there are
significant performance issues here.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] slub: Replace __this_cpu_inc usage w/ SLUB_STATS
On Thu, 6 Mar 2014, Josh Cartwright wrote: > Although, I'm wondering how exact these statistics need to be. Is > making them preemption safe even a concern? Not sure about that. You solution makes it preempt safe. If is can be tolerated that its racy then raw_cpu_inc() could be used. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] slub: Replace __this_cpu_inc usage w/ SLUB_STATS
On Thu, 6 Mar 2014, Josh Cartwright wrote: Although, I'm wondering how exact these statistics need to be. Is making them preemption safe even a concern? Not sure about that. You solution makes it preempt safe. If is can be tolerated that its racy then raw_cpu_inc() could be used. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH -next] slub: Replace __this_cpu_inc usage w/ SLUB_STATS
On Thu, Mar 06, 2014 at 09:53:16AM -0600, Josh Cartwright wrote:
> Booting on my Samsung Series 9 laptop gives me loads and loads of BUGs
> triggered by __this_cpu_add(), making making the system completely
> unusable:
>
> [5.808326] BUG: using __this_cpu_add() in preemptible [] code:
> swapper/0/1
> [5.812331] caller is __this_cpu_preempt_check+0x2b/0x30
> [5.815654] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> 3.14.0-rc5-next-20140306-joshc-08290-g0ffb2fe #1
> [5.819553] Hardware name: SAMSUNG ELECTRONICS CO., LTD.
> 900X3C/900X3D/900X3E/900X4C/900X4D/NP900X3E-A02US, BIOS P07ABK 04/09/2013
> [5.823558] 8801182157c0 880118215790 81a64cec
>
> [5.827177] 8801182157b0 81462360 8800c3d553e0
> ea00030f5500
> [5.830744] 8801182157e8 814623bb 635f736968745f5f
> 29286464615f7570
> [5.834134] Call Trace:
> [5.836848] [] dump_stack+0x4e/0x7a
> [5.839943] [] check_preemption_disabled+0xd0/0xe0
> [5.842997] [] __this_cpu_preempt_check+0x2b/0x30
> [5.846022] [] __slab_free+0x38/0x590
> [5.848863] [] ? get_parent_ip+0xd/0x50
> [5.850467] BUG: using __this_cpu_add() in preemptible [] code:
> khubd/36
> [5.850472] caller is __this_cpu_preempt_check+0x2b/0x30
> [5.859125] [] ? preempt_count_sub+0x6b/0xf0
> [5.862521] [] ? _raw_spin_unlock_irqrestore+0x4a/0x80
> [5.865599] [] ? __debug_check_no_obj_freed+0x13e/0x240
> [5.868738] [] ? __this_cpu_preempt_check+0x2b/0x30
> [5.871799] [] kfree+0x2f7/0x300
FWIW, it looks like the magic combination of options are:
- CONFIG_DEBUG_PREEMPT=y
- CONFIG_SLUB=y
- CONFIG_SLUB_STATS=y
Looks like the new percpu() checks are complaining about SLUB's use of
__this_cpu_inc() for maintaining it's stat counters. The below patch
seems to fix it.
Although, I'm wondering how exact these statistics need to be. Is
making them preemption safe even a concern?
Thanks,
Josh
--8<--
Make slub statistics maintenance preemption-safe.
Fixes the following warning when CONFIG_SLUB_STATS and
CONFIG_DEBUG_PREEMPT:
BUG: using __this_cpu_add() in preemptible [] code:
systemd-journal/226
caller is __this_cpu_preempt_check+0x2b/0x30
Call Trace:
dump_stack+0x4e/0x7a
check_preemption_disabled+0xd0/0xe0
__this_cpu_preempt_check+0x2b/0x30
__slab_free+0x38/0x590
kmem_cache_free+0x367/0x3a0
jbd2_journal_stop+0x24d/0x500
__ext4_journal_stop+0x37/0x90
ext4_truncate+0x1ab/0x570
ext4_setattr+0x2d3/0x810
notify_change+0x159/0x3a0
do_truncate+0x6f/0xa0
do_sys_ftruncate.constprop.18+0x10e/0x160
SyS_ftruncate+0xe/0x10
system_call_fastpath+0x1a/0x1f
Cc: Christoph Lameter
Signed-off-by: Josh Cartwright
---
mm/slub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/slub.c b/mm/slub.c
index c6eb29d..c873e61 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -224,7 +224,7 @@ static inline void memcg_propagate_slab_attrs(struct
kmem_cache *s) { }
static inline void stat(const struct kmem_cache *s, enum stat_item si)
{
#ifdef CONFIG_SLUB_STATS
- __this_cpu_inc(s->cpu_slab->stat[si]);
+ this_cpu_inc(s->cpu_slab->stat[si]);
#endif
}
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH -next] slub: Replace __this_cpu_inc usage w/ SLUB_STATS
On Thu, Mar 06, 2014 at 09:53:16AM -0600, Josh Cartwright wrote:
Booting on my Samsung Series 9 laptop gives me loads and loads of BUGs
triggered by __this_cpu_add(), making making the system completely
unusable:
[5.808326] BUG: using __this_cpu_add() in preemptible [] code:
swapper/0/1
[5.812331] caller is __this_cpu_preempt_check+0x2b/0x30
[5.815654] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
3.14.0-rc5-next-20140306-joshc-08290-g0ffb2fe #1
[5.819553] Hardware name: SAMSUNG ELECTRONICS CO., LTD.
900X3C/900X3D/900X3E/900X4C/900X4D/NP900X3E-A02US, BIOS P07ABK 04/09/2013
[5.823558] 8801182157c0 880118215790 81a64cec
[5.827177] 8801182157b0 81462360 8800c3d553e0
ea00030f5500
[5.830744] 8801182157e8 814623bb 635f736968745f5f
29286464615f7570
[5.834134] Call Trace:
[5.836848] [81a64cec] dump_stack+0x4e/0x7a
[5.839943] [81462360] check_preemption_disabled+0xd0/0xe0
[5.842997] [814623bb] __this_cpu_preempt_check+0x2b/0x30
[5.846022] [81a6331d] __slab_free+0x38/0x590
[5.848863] [811759dd] ? get_parent_ip+0xd/0x50
[5.850467] BUG: using __this_cpu_add() in preemptible [] code:
khubd/36
[5.850472] caller is __this_cpu_preempt_check+0x2b/0x30
[5.859125] [81175b3b] ? preempt_count_sub+0x6b/0xf0
[5.862521] [81a7175a] ? _raw_spin_unlock_irqrestore+0x4a/0x80
[5.865599] [81462e5e] ? __debug_check_no_obj_freed+0x13e/0x240
[5.868738] [814623bb] ? __this_cpu_preempt_check+0x2b/0x30
[5.871799] [81287327] kfree+0x2f7/0x300
FWIW, it looks like the magic combination of options are:
- CONFIG_DEBUG_PREEMPT=y
- CONFIG_SLUB=y
- CONFIG_SLUB_STATS=y
Looks like the new percpu() checks are complaining about SLUB's use of
__this_cpu_inc() for maintaining it's stat counters. The below patch
seems to fix it.
Although, I'm wondering how exact these statistics need to be. Is
making them preemption safe even a concern?
Thanks,
Josh
--8--
Make slub statistics maintenance preemption-safe.
Fixes the following warning when CONFIG_SLUB_STATS and
CONFIG_DEBUG_PREEMPT:
BUG: using __this_cpu_add() in preemptible [] code:
systemd-journal/226
caller is __this_cpu_preempt_check+0x2b/0x30
Call Trace:
dump_stack+0x4e/0x7a
check_preemption_disabled+0xd0/0xe0
__this_cpu_preempt_check+0x2b/0x30
__slab_free+0x38/0x590
kmem_cache_free+0x367/0x3a0
jbd2_journal_stop+0x24d/0x500
__ext4_journal_stop+0x37/0x90
ext4_truncate+0x1ab/0x570
ext4_setattr+0x2d3/0x810
notify_change+0x159/0x3a0
do_truncate+0x6f/0xa0
do_sys_ftruncate.constprop.18+0x10e/0x160
SyS_ftruncate+0xe/0x10
system_call_fastpath+0x1a/0x1f
Cc: Christoph Lameter c...@linux.com
Signed-off-by: Josh Cartwright jo...@codeaurora.org
---
mm/slub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/slub.c b/mm/slub.c
index c6eb29d..c873e61 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -224,7 +224,7 @@ static inline void memcg_propagate_slab_attrs(struct
kmem_cache *s) { }
static inline void stat(const struct kmem_cache *s, enum stat_item si)
{
#ifdef CONFIG_SLUB_STATS
- __this_cpu_inc(s-cpu_slab-stat[si]);
+ this_cpu_inc(s-cpu_slab-stat[si]);
#endif
}
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
