* Re: Regression - locking (all from 2.6.28)
[not found] <49AC334A.9030800@gmail.com>
@ 2009-03-02 20:11 ` Andrew Morton
2009-03-03 10:41 ` Catalin Marinas
2009-03-03 18:12 ` Peter Zijlstra
0 siblings, 2 replies; 19+ messages in thread
From: Andrew Morton @ 2009-03-02 20:11 UTC (permalink / raw)
To: jan sonnek; +Cc: linux-kernel, viro, Catalin Marinas, Peter Zijlstra
On Mon, 02 Mar 2009 20:28:10 +0100
jan sonnek <ha2nny@gmail.com> wrote:
> Later I have reported regression, now I have better debug info in the
> attachements.
>
> Later reporter - still actual for (2.6.29-rc6-mm1):
> From 2.6.28 and other 2.6.29-rc3-mm1 I have problem with starting
> system with GDM (gdm-2.24.0-12). Without login screen system stop and
> generate error (all kernel soon then 2.6.27 are ok).
>
> It is bug: http://bugzilla.kernel.org/show_bug.cgi?id=12619
>
The linux-kernel mailing list probably won't accept a 662 kbyte email.
Please trim these reports down to some sane size.
> Mar 1 00:07:03 localhost kernel: [ 86.440261] =========================================================
> Mar 1 00:07:03 localhost kernel: [ 86.440266] [ INFO: possible irq lock inversion dependency detected ]
> Mar 1 00:07:03 localhost kernel: [ 86.440271] 2.6.29-rc6-mm1-hanny #17
> Mar 1 00:07:03 localhost kernel: [ 86.440273] ---------------------------------------------------------
I stared at this for a while, but my brain broke trying to work out
what lockdep is trying to tell us.
> Mar 1 00:07:03 localhost kernel: [ 86.440277] Xorg/2733 just changed the state of lock:
> Mar 1 00:07:03 localhost kernel: [ 86.440280] (fasync_lock){.-....}, at: [<c01952bb>] kill_fasync+0x20/0x3a
> Mar 1 00:07:03 localhost kernel: [ 86.440292] but this lock took another, HARDIRQ-READ-irq-unsafe lock in the past:
> Mar 1 00:07:03 localhost kernel: [ 86.440296] (&f->f_lock){+.+...}
This message needs help. A lock cannot "take" another lock. And why
is f_lock described as "HARDIRQ-READ-irq-unsafe"? It's a spinlock and
the "READ" part is not relevant.
> Mar 1 00:07:03 localhost kernel: [ 86.440299]
> Mar 1 00:07:03 localhost kernel: [ 86.440300] and interrupts could create inverse lock ordering between them.
> Mar 1 00:07:03 localhost kernel: [ 86.440302]
> Mar 1 00:07:03 localhost kernel: [ 86.440305]
> Mar 1 00:07:03 localhost kernel: [ 86.440305] other info that might help us debug this:
> Mar 1 00:07:03 localhost kernel: [ 86.440309] 3 locks held by Xorg/2733:
> Mar 1 00:07:03 localhost kernel: [ 86.440312] #0: (&dev->event_lock){-.-...}, at: [<c02d5e8c>] input_event+0x35/0x69
> Mar 1 00:07:03 localhost kernel: [ 86.440322] #1: (rcu_read_lock){.+.+..}, at: [<c02d4af3>] __rcu_read_lock+0x0/0x30
> Mar 1 00:07:03 localhost kernel: [ 86.440331] #2: (rcu_read_lock){.+.+..}, at: [<c02d872a>] evdev_event+0x0/0xe2
> Mar 1 00:07:03 localhost kernel: [ 86.440340]
> Mar 1 00:07:03 localhost kernel: [ 86.440341] the first lock's dependencies:
> Mar 1 00:07:03 localhost kernel: [ 86.440344] -> (fasync_lock){.-....} ops: 190 {
> Mar 1 00:07:03 localhost kernel: [ 86.440351] IN-HARDIRQ-R at:
> Mar 1 00:07:03 localhost kernel: [ 86.440355] [<c01471a2>] __lock_acquire+0x204/0xb4a
> Mar 1 00:07:03 localhost kernel: [ 86.440362] [<c0147b45>] lock_acquire+0x5d/0x7a
> Mar 1 00:07:03 localhost kernel: [ 86.440368] [<c0398bc2>] _read_lock+0x2d/0x5d
> Mar 1 00:07:03 localhost kernel: [ 86.440375] [<c01952bb>] kill_fasync+0x20/0x3a
> Mar 1 00:07:03 localhost kernel: [ 86.440381] [<c02d84a7>] evdev_pass_event+0x60/0x66
> Mar 1 00:07:03 localhost kernel: [ 86.440387] [<c02d879d>] evdev_event+0x73/0xe2
> Mar 1 00:07:03 localhost kernel: [ 86.440393] [<c02d4bb9>] input_pass_event+0x5c/0x7f
> Mar 1 00:07:03 localhost kernel: [ 86.440399] [<c02d5dd6>] input_handle_event+0x366/0x36f
> Mar 1 00:07:03 localhost kernel: [ 86.440405] [<c02d5eab>] input_event+0x54/0x69
> Mar 1 00:07:03 localhost kernel: [ 86.440410] [<c02f1e03>] hidinput_hid_event+0x24c/0x279
> Mar 1 00:07:03 localhost kernel: [ 86.440418] [<c02ef1ed>] hid_process_event+0x8d/0xbc
> Mar 1 00:07:03 localhost kernel: [ 86.440424] [<c02ef558>] hid_report_raw_event+0x33c/0x348
> Mar 1 00:07:03 localhost kernel: [ 86.440431] [<c02ef60f>] hid_input_report+0xab/0xbc
> Mar 1 00:07:03 localhost kernel: [ 86.440437] [<c02f576c>] hid_irq_in+0x86/0x182
> Mar 1 00:07:03 localhost kernel: [ 86.440443] [<c02af7fa>] usb_hcd_giveback_urb+0x68/0x9c
> Mar 1 00:07:03 localhost kernel: [ 86.440451] [<c02cec0f>] uhci_giveback_urb+0xf6/0x1f1
> Mar 1 00:07:03 localhost kernel: [ 86.440458] [<c02cf404>] uhci_scan_schedule+0x5f8/0x85f
> Mar 1 00:07:03 localhost kernel: [ 86.440464] [<c02d10e7>] uhci_irq+0x12b/0x13f
> Mar 1 00:07:03 localhost kernel: [ 86.440470] [<c02af3cc>] usb_hcd_irq+0x32/0x81
> Mar 1 00:07:03 localhost kernel: [ 86.440475] [<c0156b6b>] handle_IRQ_event+0x1f/0x4b
> Mar 1 00:07:03 localhost kernel: [ 86.440483] [<c0157d09>] handle_fasteoi_irq+0x77/0xb0
> Mar 1 00:07:03 localhost kernel: [ 86.440489] [<ffffffff>] 0xffffffff
> Mar 1 00:07:03 localhost kernel: [ 86.440494] INITIAL USE at:
> Mar 1 00:07:03 localhost kernel: [ 86.440498] [<c01472e2>] __lock_acquire+0x344/0xb4a
> Mar 1 00:07:03 localhost kernel: [ 86.440504] [<c0147b45>] lock_acquire+0x5d/0x7a
> Mar 1 00:07:03 localhost kernel: [ 86.440510] [<c039895d>] _write_lock_irq+0x33/0x63
> Mar 1 00:07:03 localhost kernel: [ 86.440515] [<c0194ef6>] fasync_helper+0x44/0xe4
> Mar 1 00:07:03 localhost kernel: [ 86.440521] [<c0247242>] tty_fasync+0x50/0xea
> Mar 1 00:07:03 localhost kernel: [ 86.440528] [<c0249322>] tty_release_dev+0x57/0x409
> Mar 1 00:07:03 localhost kernel: [ 86.440534] [<c02496eb>] tty_release+0x17/0x21
> Mar 1 00:07:03 localhost kernel: [ 86.440539] [<c018c369>] __fput+0xcf/0x158
> Mar 1 00:07:03 localhost kernel: [ 86.440546] [<c018c410>] fput+0x1e/0x20
> Mar 1 00:07:03 localhost kernel: [ 86.440551] [<c0189b12>] filp_close+0x56/0x60
> Mar 1 00:07:03 localhost kernel: [ 86.440557] [<c0189b8b>] sys_close+0x6f/0xa9
> Mar 1 00:07:03 localhost kernel: [ 86.440562] [<c0102f47>] sysenter_do_call+0x12/0x35
> Mar 1 00:07:03 localhost kernel: [ 86.440569] [<ffffffff>] 0xffffffff
> Mar 1 00:07:03 localhost kernel: [ 86.440574] }
> Mar 1 00:07:03 localhost kernel: [ 86.440576] ... key at: [<c04fea24>] fasync_lock+0x10/0x24
> Mar 1 00:07:03 localhost kernel: [ 86.440583] -> (&f->f_lock){+.+...} ops: 493 {
> Mar 1 00:07:03 localhost kernel: [ 86.440590] HARDIRQ-ON-W at:
> Mar 1 00:07:03 localhost kernel: [ 86.440594] [<c0147260>] __lock_acquire+0x2c2/0xb4a
> Mar 1 00:07:03 localhost kernel: [ 86.440600] [<c0147b45>] lock_acquire+0x5d/0x7a
> Mar 1 00:07:03 localhost kernel: [ 86.440606] [<c039869c>] _spin_lock+0x2d/0x5d
> Mar 1 00:07:03 localhost kernel: [ 86.440612] [<c019561e>] do_fcntl+0x222/0x2bc
> Mar 1 00:07:03 localhost kernel: [ 86.440617] [<c0195712>] sys_fcntl64+0x5a/0x6e
> Mar 1 00:07:03 localhost kernel: [ 86.440623] [<c0102f47>] sysenter_do_call+0x12/0x35
> Mar 1 00:07:03 localhost kernel: [ 86.440629] [<ffffffff>] 0xffffffff
> Mar 1 00:07:03 localhost kernel: [ 86.440633] SOFTIRQ-ON-W at:
> Mar 1 00:07:03 localhost kernel: [ 86.440637] [<c0147283>] __lock_acquire+0x2e5/0xb4a
> Mar 1 00:07:03 localhost kernel: [ 86.440643] [<c0147b45>] lock_acquire+0x5d/0x7a
> Mar 1 00:07:03 localhost kernel: [ 86.440649] [<c039869c>] _spin_lock+0x2d/0x5d
> Mar 1 00:07:03 localhost kernel: [ 86.440654] [<c019561e>] do_fcntl+0x222/0x2bc
> Mar 1 00:07:03 localhost kernel: [ 86.440660] [<c0195712>] sys_fcntl64+0x5a/0x6e
> Mar 1 00:07:03 localhost kernel: [ 86.440666] [<c0102f47>] sysenter_do_call+0x12/0x35
> Mar 1 00:07:03 localhost kernel: [ 86.440672] [<ffffffff>] 0xffffffff
> Mar 1 00:07:03 localhost kernel: [ 86.440676] INITIAL USE at:
> Mar 1 00:07:03 localhost kernel: [ 86.440680] [<c01472e2>] __lock_acquire+0x344/0xb4a
> Mar 1 00:07:03 localhost kernel: [ 86.440686] [<c0147b45>] lock_acquire+0x5d/0x7a
> Mar 1 00:07:03 localhost kernel: [ 86.440691] [<c039869c>] _spin_lock+0x2d/0x5d
> Mar 1 00:07:03 localhost kernel: [ 86.440697] [<c0194f66>] fasync_helper+0xb4/0xe4
> Mar 1 00:07:03 localhost kernel: [ 86.440703] [<c0247242>] tty_fasync+0x50/0xea
> Mar 1 00:07:03 localhost kernel: [ 86.440708] [<c0249322>] tty_release_dev+0x57/0x409
> Mar 1 00:07:03 localhost kernel: [ 86.440714] [<c02496eb>] tty_release+0x17/0x21
> Mar 1 00:07:03 localhost kernel: [ 86.440720] [<c018c369>] __fput+0xcf/0x158
> Mar 1 00:07:03 localhost kernel: [ 86.440725] [<c018c410>] fput+0x1e/0x20
> Mar 1 00:07:03 localhost kernel: [ 86.440731] [<c0189b12>] filp_close+0x56/0x60
> Mar 1 00:07:03 localhost kernel: [ 86.440736] [<c0189b8b>] sys_close+0x6f/0xa9
> Mar 1 00:07:03 localhost kernel: [ 86.440741] [<c0102f47>] sysenter_do_call+0x12/0x35
> Mar 1 00:07:03 localhost kernel: [ 86.440747] [<ffffffff>] 0xffffffff
> Mar 1 00:07:03 localhost kernel: [ 86.440752] }
> Mar 1 00:07:03 localhost kernel: [ 86.440754] ... key at: [<c0b839d0>] __key.20190+0x0/0x8
> Mar 1 00:07:03 localhost kernel: [ 86.440760] ... acquired at:
> Mar 1 00:07:03 localhost kernel: [ 86.440763] [<c0147965>] __lock_acquire+0x9c7/0xb4a
> Mar 1 00:07:03 localhost kernel: [ 86.440768] [<c0147b45>] lock_acquire+0x5d/0x7a
> Mar 1 00:07:03 localhost kernel: [ 86.440773] [<c039869c>] _spin_lock+0x2d/0x5d
> Mar 1 00:07:03 localhost kernel: [ 86.440778] [<c0194f66>] fasync_helper+0xb4/0xe4
> Mar 1 00:07:03 localhost kernel: [ 86.440783] [<c0247242>] tty_fasync+0x50/0xea
> Mar 1 00:07:03 localhost kernel: [ 86.440788] [<c0249322>] tty_release_dev+0x57/0x409
> Mar 1 00:07:03 localhost kernel: [ 86.440793] [<c02496eb>] tty_release+0x17/0x21
> Mar 1 00:07:03 localhost kernel: [ 86.440798] [<c018c369>] __fput+0xcf/0x158
> Mar 1 00:07:03 localhost kernel: [ 86.440803] [<c018c410>] fput+0x1e/0x20
> Mar 1 00:07:03 localhost kernel: [ 86.440807] [<c0189b12>] filp_close+0x56/0x60
> Mar 1 00:07:03 localhost kernel: [ 86.440812] [<c0189b8b>] sys_close+0x6f/0xa9
> Mar 1 00:07:03 localhost kernel: [ 86.440817] [<c0102f47>] sysenter_do_call+0x12/0x35
> Mar 1 00:07:03 localhost kernel: [ 86.440822] [<ffffffff>] 0xffffffff
> Mar 1 00:07:03 localhost kernel: [ 86.440826]
> Mar 1 00:07:03 localhost kernel: [ 86.440828]
> Mar 1 00:07:03 localhost kernel: [ 86.440829] the second lock's dependencies:
> Mar 1 00:07:03 localhost kernel: [ 86.440832] -> (&f->f_lock){+.+...} ops: 493 {
> Mar 1 00:07:03 localhost kernel: [ 86.440839] HARDIRQ-ON-W at:
> Mar 1 00:07:03 localhost kernel: [ 86.440842] [<c0147260>] __lock_acquire+0x2c2/0xb4a
> Mar 1 00:07:03 localhost kernel: [ 86.440848] [<c0147b45>] lock_acquire+0x5d/0x7a
> Mar 1 00:07:03 localhost kernel: [ 86.440854] [<c039869c>] _spin_lock+0x2d/0x5d
> Mar 1 00:07:03 localhost kernel: [ 86.440859] [<c019561e>] do_fcntl+0x222/0x2bc
> Mar 1 00:07:03 localhost kernel: [ 86.440865] [<c0195712>] sys_fcntl64+0x5a/0x6e
> Mar 1 00:07:03 localhost kernel: [ 86.440871] [<c0102f47>] sysenter_do_call+0x12/0x35
> Mar 1 00:07:03 localhost kernel: [ 86.440876] [<ffffffff>] 0xffffffff
> Mar 1 00:07:03 localhost kernel: [ 86.440881] SOFTIRQ-ON-W at:
> Mar 1 00:07:03 localhost kernel: [ 86.440884] [<c0147283>] __lock_acquire+0x2e5/0xb4a
> Mar 1 00:07:03 localhost kernel: [ 86.440890] [<c0147b45>] lock_acquire+0x5d/0x7a
> Mar 1 00:07:03 localhost kernel: [ 86.440896] [<c039869c>] _spin_lock+0x2d/0x5d
> Mar 1 00:07:03 localhost kernel: [ 86.440901] [<c019561e>] do_fcntl+0x222/0x2bc
> Mar 1 00:07:03 localhost kernel: [ 86.440907] [<c0195712>] sys_fcntl64+0x5a/0x6e
> Mar 1 00:07:03 localhost kernel: [ 86.440913] [<c0102f47>] sysenter_do_call+0x12/0x35
> Mar 1 00:07:03 localhost kernel: [ 86.440918] [<ffffffff>] 0xffffffff
> Mar 1 00:07:03 localhost kernel: [ 86.440923] INITIAL USE at:
> Mar 1 00:07:03 localhost kernel: [ 86.440926] [<c01472e2>] __lock_acquire+0x344/0xb4a
> Mar 1 00:07:03 localhost kernel: [ 86.440932] [<c0147b45>] lock_acquire+0x5d/0x7a
> Mar 1 00:07:03 localhost kernel: [ 86.440938] [<c039869c>] _spin_lock+0x2d/0x5d
> Mar 1 00:07:03 localhost kernel: [ 86.440943] [<c0194f66>] fasync_helper+0xb4/0xe4
> Mar 1 00:07:03 localhost kernel: [ 86.440949] [<c0247242>] tty_fasync+0x50/0xea
> Mar 1 00:07:03 localhost kernel: [ 86.440955] [<c0249322>] tty_release_dev+0x57/0x409
> Mar 1 00:07:03 localhost kernel: [ 86.440960] [<c02496eb>] tty_release+0x17/0x21
> Mar 1 00:07:03 localhost kernel: [ 86.440966] [<c018c369>] __fput+0xcf/0x158
> Mar 1 00:07:03 localhost kernel: [ 86.440971] [<c018c410>] fput+0x1e/0x20
> Mar 1 00:07:03 localhost kernel: [ 86.440977] [<c0189b12>] filp_close+0x56/0x60
> Mar 1 00:07:03 localhost kernel: [ 86.440982] [<c0189b8b>] sys_close+0x6f/0xa9
> Mar 1 00:07:03 localhost kernel: [ 86.440988] [<c0102f47>] sysenter_do_call+0x12/0x35
> Mar 1 00:07:03 localhost kernel: [ 86.440993] [<ffffffff>] 0xffffffff
> Mar 1 00:07:03 localhost kernel: [ 86.440998] }
> Mar 1 00:07:03 localhost kernel: [ 86.441000] ... key at: [<c0b839d0>] __key.20190+0x0/0x8
> Mar 1 00:07:03 localhost kernel: [ 86.441005]
> Mar 1 00:07:03 localhost kernel: [ 86.441006] stack backtrace:
> Mar 1 00:07:03 localhost kernel: [ 86.441010] Pid: 2733, comm: Xorg Not tainted 2.6.29-rc6-mm1-hanny #17
> Mar 1 00:07:03 localhost kernel: [ 86.441013] Call Trace:
> Mar 1 00:07:03 localhost kernel: [ 86.441018] [<c0396150>] ? printk+0x14/0x1c
> Mar 1 00:07:03 localhost kernel: [ 86.441023] [<c0146a58>] print_irq_inversion_bug+0xea/0xf7
> Mar 1 00:07:03 localhost kernel: [ 86.441029] [<c0146a9b>] check_usage_forwards+0x36/0x3f
> Mar 1 00:07:03 localhost kernel: [ 86.441034] [<c0146425>] mark_lock+0x129/0x20b
> Mar 1 00:07:03 localhost kernel: [ 86.441038] [<c0146a65>] ? check_usage_forwards+0x0/0x3f
> Mar 1 00:07:03 localhost kernel: [ 86.441043] [<c01471a2>] __lock_acquire+0x204/0xb4a
> Mar 1 00:07:03 localhost kernel: [ 86.441048] [<c02d8467>] ? evdev_pass_event+0x20/0x66
> Mar 1 00:07:03 localhost kernel: [ 86.441054] [<c0147b45>] lock_acquire+0x5d/0x7a
> Mar 1 00:07:03 localhost kernel: [ 86.441059] [<c01952bb>] ? kill_fasync+0x20/0x3a
> Mar 1 00:07:03 localhost kernel: [ 86.441063] [<c0398bc2>] _read_lock+0x2d/0x5d
> Mar 1 00:07:03 localhost kernel: [ 86.441068] [<c01952bb>] ? kill_fasync+0x20/0x3a
> Mar 1 00:07:03 localhost kernel: [ 86.441073] [<c01952bb>] kill_fasync+0x20/0x3a
> Mar 1 00:07:03 localhost kernel: [ 86.441078] [<c02d84a7>] evdev_pass_event+0x60/0x66
> Mar 1 00:07:03 localhost kernel: [ 86.441083] [<c02d879d>] evdev_event+0x73/0xe2
> Mar 1 00:07:03 localhost kernel: [ 86.441087] [<c02d4bb9>] input_pass_event+0x5c/0x7f
> Mar 1 00:07:03 localhost kernel: [ 86.441092] [<c02d5dd6>] input_handle_event+0x366/0x36f
> Mar 1 00:07:03 localhost kernel: [ 86.441098] [<c0246eac>] ? add_timer_randomness+0xee/0x108
> Mar 1 00:07:03 localhost kernel: [ 86.441103] [<c02d5eab>] input_event+0x54/0x69
> Mar 1 00:07:03 localhost kernel: [ 86.441108] [<c02f1e03>] hidinput_hid_event+0x24c/0x279
> Mar 1 00:07:03 localhost kernel: [ 86.441114] [<c02ef1ed>] hid_process_event+0x8d/0xbc
> Mar 1 00:07:03 localhost kernel: [ 86.441119] [<c02ef558>] hid_report_raw_event+0x33c/0x348
> Mar 1 00:07:03 localhost kernel: [ 86.441125] [<c02ef60f>] hid_input_report+0xab/0xbc
> Mar 1 00:07:03 localhost kernel: [ 86.441130] [<c02f576c>] hid_irq_in+0x86/0x182
> Mar 1 00:07:03 localhost kernel: [ 86.441135] [<c02af7fa>] usb_hcd_giveback_urb+0x68/0x9c
> Mar 1 00:07:03 localhost kernel: [ 86.441140] [<c02cec0f>] uhci_giveback_urb+0xf6/0x1f1
> Mar 1 00:07:03 localhost kernel: [ 86.441145] [<c0147ad9>] ? __lock_acquire+0xb3b/0xb4a
> Mar 1 00:07:03 localhost kernel: [ 86.441151] [<c02cf404>] uhci_scan_schedule+0x5f8/0x85f
> Mar 1 00:07:03 localhost kernel: [ 86.441156] [<c014548a>] ? put_lock_stats+0xd/0x21
> Mar 1 00:07:03 localhost kernel: [ 86.441161] [<c02d10e7>] uhci_irq+0x12b/0x13f
> Mar 1 00:07:03 localhost kernel: [ 86.441166] [<c02af3cc>] usb_hcd_irq+0x32/0x81
> Mar 1 00:07:03 localhost kernel: [ 86.441172] [<c0156b6b>] handle_IRQ_event+0x1f/0x4b
> Mar 1 00:07:03 localhost kernel: [ 86.441176] [<c0157d09>] handle_fasteoi_irq+0x77/0xb0
> Mar 1 00:07:03 localhost kernel: [ 86.441181] [<c0157c92>] ? handle_fasteoi_irq+0x0/0xb0
> Mar 1 00:07:03 localhost kernel: [ 86.441184] <IRQ> [<c0398faa>] ? __irqentry_text_start+0x4a/0x8c
>
And bazillions of these:
> Mar 1 00:06:51 localhost kernel: [ 74.007988] unreferenced object 0xf6c4da80 (size 52):
> Mar 1 00:06:51 localhost kernel: [ 74.007991] comm "swapper", pid 1, jiffies 4294893427
> Mar 1 00:06:51 localhost kernel: [ 74.007994] backtrace:
> Mar 1 00:06:51 localhost kernel: [ 74.007997] [<c018978c>] kmemleak_alloc+0x17e/0x28e
> Mar 1 00:06:51 localhost kernel: [ 74.008002] [<c0186b86>] kmem_cache_alloc+0xdc/0xe7
> Mar 1 00:06:51 localhost kernel: [ 74.008006] [<c01a53bd>] alloc_buffer_head+0x16/0x71
> Mar 1 00:06:51 localhost kernel: [ 74.008011] [<c01a5b91>] alloc_page_buffers+0x23/0xad
> Mar 1 00:06:51 localhost kernel: [ 74.008015] [<c01a5fd4>] __getblk+0x192/0x26b
> Mar 1 00:06:51 localhost kernel: [ 74.008020] [<c01d91f4>] jread+0x105/0x1de
> Mar 1 00:06:51 localhost kernel: [ 74.008026] [<c01d932b>] do_one_pass+0x5e/0x38c
> Mar 1 00:06:51 localhost kernel: [ 74.008031] [<c01d96f8>] journal_recover+0x41/0x9d
> Mar 1 00:06:51 localhost kernel: [ 74.008037] [<c01db8d4>] journal_load+0x47/0x7b
> Mar 1 00:06:51 localhost kernel: [ 74.008042] [<c01d43d1>] ext3_fill_super+0xe9d/0x144c
> Mar 1 00:06:51 localhost kernel: [ 74.008047] [<c018d721>] get_sb_bdev+0xfa/0x140
> Mar 1 00:06:51 localhost kernel: [ 74.008052] [<c01d2070>] ext3_get_sb+0x18/0x1a
> Mar 1 00:06:51 localhost kernel: [ 74.008057] [<c018c71f>] vfs_kern_mount+0x41/0x7c
> Mar 1 00:06:51 localhost kernel: [ 74.008062] [<c018c7a8>] do_kern_mount+0x37/0xbe
> Mar 1 00:06:51 localhost kernel: [ 74.008067] [<c019f0bf>] do_mount+0x5f7/0x630
> Mar 1 00:06:51 localhost kernel: [ 74.008070] [<c019f167>] sys_mount+0x6f/0xac
> Mar 1 00:06:51 localhost kernel: [ 74.008075] unreferenced object 0xf6c4dab8 (size 52):
> Mar 1 00:06:51 localhost kernel: [ 74.008078] comm "swapper", pid 1, jiffies 4294893427
> Mar 1 00:06:51 localhost kernel: [ 74.008081] backtrace:
> Mar 1 00:06:51 localhost kernel: [ 74.008085] [<c018978c>] kmemleak_alloc+0x17e/0x28e
> Mar 1 00:06:51 localhost kernel: [ 74.008091] [<c0186b86>] kmem_cache_alloc+0xdc/0xe7
> Mar 1 00:06:51 localhost kernel: [ 74.008097] [<c01a53bd>] alloc_buffer_head+0x16/0x71
> Mar 1 00:06:51 localhost kernel: [ 74.008103] [<c01a5b91>] alloc_page_buffers+0x23/0xad
> Mar 1 00:06:51 localhost kernel: [ 74.008109] [<c01a5fd4>] __getblk+0x192/0x26b
> Mar 1 00:06:51 localhost kernel: [ 74.008114] [<c01d91f4>] jread+0x105/0x1de
> Mar 1 00:06:51 localhost kernel: [ 74.008118] [<c01d932b>] do_one_pass+0x5e/0x38c
> Mar 1 00:06:51 localhost kernel: [ 74.008122] [<c01d96f8>] journal_recover+0x41/0x9d
> Mar 1 00:06:51 localhost kernel: [ 74.008127] [<c01db8d4>] journal_load+0x47/0x7b
> Mar 1 00:06:51 localhost kernel: [ 74.008132] [<c01d43d1>] ext3_fill_super+0xe9d/0x144c
> Mar 1 00:06:51 localhost kernel: [ 74.008136] [<c018d721>] get_sb_bdev+0xfa/0x140
> Mar 1 00:06:51 localhost kernel: [ 74.008141] [<c01d2070>] ext3_get_sb+0x18/0x1a
> Mar 1 00:06:51 localhost kernel: [ 74.008145] [<c018c71f>] vfs_kern_mount+0x41/0x7c
> Mar 1 00:06:51 localhost kernel: [ 74.008149] [<c018c7a8>] do_kern_mount+0x37/0xbe
> Mar 1 00:06:51 localhost kernel: [ 74.008154] [<c019f0bf>] do_mount+0x5f7/0x630
> Mar 1 00:06:51 localhost kernel: [ 74.008159] [<c019f167>] sys_mount+0x6f/0xac
> Mar 1 00:06:51 localhost kernel: [ 74.008165] unreferenced object 0xf6c4daf0 (size 52):
> Mar 1 00:06:51 localhost kernel: [ 74.008170] comm "swapper", pid 1, jiffies 4294893427
> Mar 1 00:06:51 localhost kernel: [ 74.008175] backtrace:
> Mar 1 00:06:51 localhost kernel: [ 74.008179] [<c018978c>] kmemleak_alloc+0x17e/0x28e
> Mar 1 00:06:51 localhost kernel: [ 74.008185] [<c0186b86>] kmem_cache_alloc+0xdc/0xe7
> Mar 1 00:06:51 localhost kernel: [ 74.008190] [<c01a53bd>] alloc_buffer_head+0x16/0x71
> Mar 1 00:06:51 localhost kernel: [ 74.008196] [<c01a5b91>] alloc_page_buffers+0x23/0xad
> Mar 1 00:06:51 localhost kernel: [ 74.008200] [<c01a5fd4>] __getblk+0x192/0x26b
> Mar 1 00:06:51 localhost kernel: [ 74.008205] [<c01d91f4>] jread+0x105/0x1de
> Mar 1 00:06:51 localhost kernel: [ 74.008209] [<c01d932b>] do_one_pass+0x5e/0x38c
> Mar 1 00:06:51 localhost kernel: [ 74.008213] [<c01d96f8>] journal_recover+0x41/0x9d
> Mar 1 00:06:51 localhost kernel: [ 74.008218] [<c01db8d4>] journal_load+0x47/0x7b
> Mar 1 00:06:51 localhost kernel: [ 74.008221] [<c01d43d1>] ext3_fill_super+0xe9d/0x144c
> Mar 1 00:06:51 localhost kernel: [ 74.008225] [<c018d721>] get_sb_bdev+0xfa/0x140
> Mar 1 00:06:51 localhost kernel: [ 74.008231] [<c01d2070>] ext3_get_sb+0x18/0x1a
> Mar 1 00:06:51 localhost kernel: [ 74.008235] [<c018c71f>] vfs_kern_mount+0x41/0x7c
> Mar 1 00:06:51 localhost kernel: [ 74.008241] [<c018c7a8>] do_kern_mount+0x37/0xbe
> Mar 1 00:06:51 localhost kernel: [ 74.008247] [<c019f0bf>] do_mount+0x5f7/0x630
> Mar 1 00:06:51 localhost kernel: [ 74.008253] [<c019f167>] sys_mount+0x6f/0xac
I suspect kmemleak has gone nuts here.
kmemleak has no MAINTAINERS entry, btw.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Regression - locking (all from 2.6.28)
2009-03-02 20:11 ` Regression - locking (all from 2.6.28) Andrew Morton
@ 2009-03-03 10:41 ` Catalin Marinas
2009-03-03 15:01 ` Catalin Marinas
2009-03-03 18:12 ` Peter Zijlstra
1 sibling, 1 reply; 19+ messages in thread
From: Catalin Marinas @ 2009-03-03 10:41 UTC (permalink / raw)
To: Andrew Morton; +Cc: jan sonnek, linux-kernel, viro, Peter Zijlstra
On Mon, 2009-03-02 at 12:11 -0800, Andrew Morton wrote:
> > Mar 1 00:06:51 localhost kernel: [ 74.008165] unreferenced object 0xf6c4daf0 (size 52):
> > Mar 1 00:06:51 localhost kernel: [ 74.008170] comm "swapper", pid 1, jiffies 4294893427
> > Mar 1 00:06:51 localhost kernel: [ 74.008175] backtrace:
> > Mar 1 00:06:51 localhost kernel: [ 74.008179] [<c018978c>] kmemleak_alloc+0x17e/0x28e
> > Mar 1 00:06:51 localhost kernel: [ 74.008185] [<c0186b86>] kmem_cache_alloc+0xdc/0xe7
> > Mar 1 00:06:51 localhost kernel: [ 74.008190] [<c01a53bd>] alloc_buffer_head+0x16/0x71
> > Mar 1 00:06:51 localhost kernel: [ 74.008196] [<c01a5b91>] alloc_page_buffers+0x23/0xad
> > Mar 1 00:06:51 localhost kernel: [ 74.008200] [<c01a5fd4>] __getblk+0x192/0x26b
> > Mar 1 00:06:51 localhost kernel: [ 74.008205] [<c01d91f4>] jread+0x105/0x1de
> > Mar 1 00:06:51 localhost kernel: [ 74.008209] [<c01d932b>] do_one_pass+0x5e/0x38c
> > Mar 1 00:06:51 localhost kernel: [ 74.008213] [<c01d96f8>] journal_recover+0x41/0x9d
> > Mar 1 00:06:51 localhost kernel: [ 74.008218] [<c01db8d4>] journal_load+0x47/0x7b
> > Mar 1 00:06:51 localhost kernel: [ 74.008221] [<c01d43d1>] ext3_fill_super+0xe9d/0x144c
> > Mar 1 00:06:51 localhost kernel: [ 74.008225] [<c018d721>] get_sb_bdev+0xfa/0x140
> > Mar 1 00:06:51 localhost kernel: [ 74.008231] [<c01d2070>] ext3_get_sb+0x18/0x1a
> > Mar 1 00:06:51 localhost kernel: [ 74.008235] [<c018c71f>] vfs_kern_mount+0x41/0x7c
> > Mar 1 00:06:51 localhost kernel: [ 74.008241] [<c018c7a8>] do_kern_mount+0x37/0xbe
> > Mar 1 00:06:51 localhost kernel: [ 74.008247] [<c019f0bf>] do_mount+0x5f7/0x630
> > Mar 1 00:06:51 localhost kernel: [ 74.008253] [<c019f167>] sys_mount+0x6f/0xac
>
> I suspect kmemleak has gone nuts here.
It seems that the buffer_head structure allocated above is stored in
page->private. However, the page structures are no longer scanned in
newer versions of kmemleak. That's the hunk that was removed after
comments about the contiguity of a node's memory:
+ /* mem_map scanning */
+ for_each_online_node(i) {
+ struct page *page, *end;
+
+ page = NODE_MEM_MAP(i);
+ end = page + NODE_DATA(i)->node_spanned_pages;
+
+ scan_block(page, end, NULL);
+ }
The alternative is to inform kmemleak about the page structures returned
from __alloc_pages_internal() but there would be problems with recursive
calls into kmemleak when it allocates its own data structures.
I'll look at re-adding the hunk above, maybe with some extra checks like
pfn_valid().
> kmemleak has no MAINTAINERS entry, btw.
The entry is called "KERNEL MEMORY LEAK DETECTOR" but it makes sense to
change it to KMEMLEAK.
Thanks.
--
Catalin
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Regression - locking (all from 2.6.28)
2009-03-03 10:41 ` Catalin Marinas
@ 2009-03-03 15:01 ` Catalin Marinas
2009-03-05 0:54 ` Dave Hansen
0 siblings, 1 reply; 19+ messages in thread
From: Catalin Marinas @ 2009-03-03 15:01 UTC (permalink / raw)
To: Andrew Morton; +Cc: jan sonnek, linux-kernel, viro, Peter Zijlstra
On Tue, 2009-03-03 at 10:41 +0000, Catalin Marinas wrote:
> On Mon, 2009-03-02 at 12:11 -0800, Andrew Morton wrote:
> > > Mar 1 00:06:51 localhost kernel: [ 74.008165] unreferenced object 0xf6c4daf0 (size 52):
> > > Mar 1 00:06:51 localhost kernel: [ 74.008170] comm "swapper", pid 1, jiffies 4294893427
> > > Mar 1 00:06:51 localhost kernel: [ 74.008175] backtrace:
> > > Mar 1 00:06:51 localhost kernel: [ 74.008179] [<c018978c>] kmemleak_alloc+0x17e/0x28e
> > > Mar 1 00:06:51 localhost kernel: [ 74.008185] [<c0186b86>] kmem_cache_alloc+0xdc/0xe7
> > > Mar 1 00:06:51 localhost kernel: [ 74.008190] [<c01a53bd>] alloc_buffer_head+0x16/0x71
> > > Mar 1 00:06:51 localhost kernel: [ 74.008196] [<c01a5b91>] alloc_page_buffers+0x23/0xad
> > > Mar 1 00:06:51 localhost kernel: [ 74.008200] [<c01a5fd4>] __getblk+0x192/0x26b
> > > Mar 1 00:06:51 localhost kernel: [ 74.008205] [<c01d91f4>] jread+0x105/0x1de
> > > Mar 1 00:06:51 localhost kernel: [ 74.008209] [<c01d932b>] do_one_pass+0x5e/0x38c
> > > Mar 1 00:06:51 localhost kernel: [ 74.008213] [<c01d96f8>] journal_recover+0x41/0x9d
> > > Mar 1 00:06:51 localhost kernel: [ 74.008218] [<c01db8d4>] journal_load+0x47/0x7b
> > > Mar 1 00:06:51 localhost kernel: [ 74.008221] [<c01d43d1>] ext3_fill_super+0xe9d/0x144c
> > > Mar 1 00:06:51 localhost kernel: [ 74.008225] [<c018d721>] get_sb_bdev+0xfa/0x140
> > > Mar 1 00:06:51 localhost kernel: [ 74.008231] [<c01d2070>] ext3_get_sb+0x18/0x1a
> > > Mar 1 00:06:51 localhost kernel: [ 74.008235] [<c018c71f>] vfs_kern_mount+0x41/0x7c
> > > Mar 1 00:06:51 localhost kernel: [ 74.008241] [<c018c7a8>] do_kern_mount+0x37/0xbe
> > > Mar 1 00:06:51 localhost kernel: [ 74.008247] [<c019f0bf>] do_mount+0x5f7/0x630
> > > Mar 1 00:06:51 localhost kernel: [ 74.008253] [<c019f167>] sys_mount+0x6f/0xac
> >
> > I suspect kmemleak has gone nuts here.
>
> It seems that the buffer_head structure allocated above is stored in
> page->private. However, the page structures are no longer scanned in
> newer versions of kmemleak. That's the hunk that was removed after
> comments about the contiguity of a node's memory:
>
> + /* mem_map scanning */
> + for_each_online_node(i) {
> + struct page *page, *end;
> +
> + page = NODE_MEM_MAP(i);
> + end = page + NODE_DATA(i)->node_spanned_pages;
> +
> + scan_block(page, end, NULL);
> + }
>
> The alternative is to inform kmemleak about the page structures returned
> from __alloc_pages_internal() but there would be problems with recursive
> calls into kmemleak when it allocates its own data structures.
>
> I'll look at re-adding the hunk above, maybe with some extra checks like
> pfn_valid().
Looking again at this, the node_mem_map is always contiguous and the
code above only scans the node_mem_map, not the memory represented by
the node (which may not be contiguous). So I think it is a valid code
sequence.
If the above gets too deep into the nodes structure, an alternative
would be:
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 09b6fd7..0f17e62 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3552,6 +3552,11 @@ static void __init_refok alloc_node_mem_map(struct pglist_data *pgdat)
map = alloc_remap(pgdat->node_id, size);
if (!map)
map = alloc_bootmem_node(pgdat, size);
+ /*
+ * Inform kmemleak to scan the node_mem_map arrays as the page
+ * structure may contain pointers to other objects.
+ */
+ kmemleak_alloc(map, size, 1, GFP_ATOMIC);
pgdat->node_mem_map = map + (pgdat->node_start_pfn - start);
}
#ifndef CONFIG_NEED_MULTIPLE_NODES
--
Catalin
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: Regression - locking (all from 2.6.28)
2009-03-02 20:11 ` Regression - locking (all from 2.6.28) Andrew Morton
2009-03-03 10:41 ` Catalin Marinas
@ 2009-03-03 18:12 ` Peter Zijlstra
1 sibling, 0 replies; 19+ messages in thread
From: Peter Zijlstra @ 2009-03-03 18:12 UTC (permalink / raw)
To: Andrew Morton
Cc: jan sonnek, linux-kernel, viro, Catalin Marinas, Wu Fengguang
On Mon, 2009-03-02 at 12:11 -0800, Andrew Morton wrote:
> > Mar 1 00:07:03 localhost kernel: [ 86.440261] =========================================================
> > Mar 1 00:07:03 localhost kernel: [ 86.440266] [ INFO: possible irq lock inversion dependency detected ]
> > Mar 1 00:07:03 localhost kernel: [ 86.440271] 2.6.29-rc6-mm1-hanny #17
> > Mar 1 00:07:03 localhost kernel: [ 86.440273] ---------------------------------------------------------
>
> I stared at this for a while, but my brain broke trying to work out
> what lockdep is trying to tell us.
>
> > Mar 1 00:07:03 localhost kernel: [ 86.440277] Xorg/2733 just changed the state of lock:
> > Mar 1 00:07:03 localhost kernel: [ 86.440280] (fasync_lock){.-....}, at: [<c01952bb>] kill_fasync+0x20/0x3a
> > Mar 1 00:07:03 localhost kernel: [ 86.440292] but this lock took another, HARDIRQ-READ-irq-unsafe lock in the past:
> > Mar 1 00:07:03 localhost kernel: [ 86.440296] (&f->f_lock){+.+...}
>
> This message needs help. A lock cannot "take" another lock.
It seemed a simple enough way to tell that the latter lock nests inside
the former lock.
So what its saying is that we have:
fasync_lock
f->f_lock
nesting, and fasync_lock got used in hardirq context, but the lock that
was previously found to nest inside, was an IRQ-unsafe lock.
So $CODE code take f->f_lock, then IRQ could happen and fasync_lock,
f->f_lock could happen and we'd be stuck.
Would something like:
"but this lock had a %s-irq-unsafe nestee in the past:" read better?
> And why
> is f_lock described as "HARDIRQ-READ-irq-unsafe"? It's a spinlock and
> the "READ" part is not relevant.
I think that's a bug due to the recent irq state tracking generalization
patches, will hunt.
> > Mar 1 00:07:03 localhost kernel: [ 86.440299]
> > Mar 1 00:07:03 localhost kernel: [ 86.440300] and interrupts could create inverse lock ordering between them.
> > Mar 1 00:07:03 localhost kernel: [ 86.440302]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Regression - locking (all from 2.6.28)
2009-03-03 15:01 ` Catalin Marinas
@ 2009-03-05 0:54 ` Dave Hansen
2009-03-05 18:04 ` Catalin Marinas
2009-03-06 16:40 ` Catalin Marinas
0 siblings, 2 replies; 19+ messages in thread
From: Dave Hansen @ 2009-03-05 0:54 UTC (permalink / raw)
To: Catalin Marinas
Cc: Andrew Morton, jan sonnek, linux-kernel, viro, Peter Zijlstra,
Andy Whitcroft
On Tue, 2009-03-03 at 15:01 +0000, Catalin Marinas wrote:
> > + /* mem_map scanning */
> > + for_each_online_node(i) {
> > + struct page *page, *end;
> > +
> > + page = NODE_MEM_MAP(i);
> > + end = page + NODE_DATA(i)->node_spanned_pages;
> > +
> > + scan_block(page, end, NULL);
> > + }
> >
> > The alternative is to inform kmemleak about the page structures returned
> > from __alloc_pages_internal() but there would be problems with recursive
> > calls into kmemleak when it allocates its own data structures.
> >
> > I'll look at re-adding the hunk above, maybe with some extra checks like
> > pfn_valid().
>
> Looking again at this, the node_mem_map is always contiguous and the
> code above only scans the node_mem_map, not the memory represented by
> the node (which may not be contiguous). So I think it is a valid code
> sequence.
The above is *not* a valid code sequence.
It is valid with discontig, but isn't valid for sparsemem. You simply
can't expect to do math on 'struct page' pointers for any granularity
larger than MAX_ORDER_NR_PAGES.
Also, we don't even define NODE_MEM_MAP() for all configurations so that
code snippet won't even compile. We would be smart to kill that macro.
One completely unoptimized thing you can do which will scan a 'struct
page' at a time is this:
for_each_online_node(i) {
unsigned long pfn;
for (pfn = node_start_pfn(i); pfn < node_end_pfn(i); pfn++) {
struct page *page;
if (!pfn_valid(pfn))
continue;
page = pfn_to_page(pfn);
scan_block(page, page+1, NULL);
}
}
The way to optimize it would be to call scan_block() only once for each
MAX_ORDER_NR_PAGES that you encounter. The other option would be to use
the active_regions functions to walk the memory.
Is there a requirement to reduce the number of calls to scan_block()
here?
-- Dave
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Regression - locking (all from 2.6.28)
2009-03-05 0:54 ` Dave Hansen
@ 2009-03-05 18:04 ` Catalin Marinas
2009-03-05 18:29 ` Peter Zijlstra
2009-03-06 16:40 ` Catalin Marinas
1 sibling, 1 reply; 19+ messages in thread
From: Catalin Marinas @ 2009-03-05 18:04 UTC (permalink / raw)
To: Dave Hansen
Cc: Andrew Morton, jan sonnek, linux-kernel, viro, Peter Zijlstra,
Andy Whitcroft
On Wed, 2009-03-04 at 16:54 -0800, Dave Hansen wrote:
> On Tue, 2009-03-03 at 15:01 +0000, Catalin Marinas wrote:
> > > + /* mem_map scanning */
> > > + for_each_online_node(i) {
> > > + struct page *page, *end;
> > > +
> > > + page = NODE_MEM_MAP(i);
> > > + end = page + NODE_DATA(i)->node_spanned_pages;
> > > +
> > > + scan_block(page, end, NULL);
> > > + }
[...]
> The above is *not* a valid code sequence.
>
> It is valid with discontig, but isn't valid for sparsemem. You simply
> can't expect to do math on 'struct page' pointers for any granularity
> larger than MAX_ORDER_NR_PAGES.
>
> Also, we don't even define NODE_MEM_MAP() for all configurations so that
> code snippet won't even compile. We would be smart to kill that macro.
>
> One completely unoptimized thing you can do which will scan a 'struct
> page' at a time is this:
>
> for_each_online_node(i) {
> unsigned long pfn;
> for (pfn = node_start_pfn(i); pfn < node_end_pfn(i); pfn++) {
> struct page *page;
> if (!pfn_valid(pfn))
> continue;
> page = pfn_to_page(pfn);
> scan_block(page, page+1, NULL);
> }
> }
>
> The way to optimize it would be to call scan_block() only once for each
> MAX_ORDER_NR_PAGES that you encounter. The other option would be to use
> the active_regions functions to walk the memory.
>
> Is there a requirement to reduce the number of calls to scan_block()
> here?
I think the improvement wouldn't be that big since scan_block() is
pretty time consuming as it checks every value that looks like a pointer
against a prio_tree.
BTW, is there a way to know whether the page is in use or on the free
list? Is page_count(page) feasible?
Thanks.
--
Catalin
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Regression - locking (all from 2.6.28)
2009-03-05 18:04 ` Catalin Marinas
@ 2009-03-05 18:29 ` Peter Zijlstra
0 siblings, 0 replies; 19+ messages in thread
From: Peter Zijlstra @ 2009-03-05 18:29 UTC (permalink / raw)
To: Catalin Marinas
Cc: Dave Hansen, Andrew Morton, jan sonnek, linux-kernel, viro,
Andy Whitcroft
On Thu, 2009-03-05 at 18:04 +0000, Catalin Marinas wrote:
>
> BTW, is there a way to know whether the page is in use or on the free
> list? Is page_count(page) feasible?
page_count() == 0, means that the page is not used.
PageBuddy() means its on the freelist.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Regression - locking (all from 2.6.28)
2009-03-05 0:54 ` Dave Hansen
2009-03-05 18:04 ` Catalin Marinas
@ 2009-03-06 16:40 ` Catalin Marinas
2009-03-06 16:52 ` Dave Hansen
1 sibling, 1 reply; 19+ messages in thread
From: Catalin Marinas @ 2009-03-06 16:40 UTC (permalink / raw)
To: Dave Hansen
Cc: Andrew Morton, jan sonnek, linux-kernel, viro, Peter Zijlstra,
Andy Whitcroft
On Wed, 2009-03-04 at 16:54 -0800, Dave Hansen wrote:
> On Tue, 2009-03-03 at 15:01 +0000, Catalin Marinas wrote:
> > > + /* mem_map scanning */
> > > + for_each_online_node(i) {
> > > + struct page *page, *end;
> > > +
> > > + page = NODE_MEM_MAP(i);
> > > + end = page + NODE_DATA(i)->node_spanned_pages;
> > > +
> > > + scan_block(page, end, NULL);
> > > + }
[...]
> One completely unoptimized thing you can do which will scan a 'struct
> page' at a time is this:
>
> for_each_online_node(i) {
> unsigned long pfn;
> for (pfn = node_start_pfn(i); pfn < node_end_pfn(i); pfn++) {
> struct page *page;
> if (!pfn_valid(pfn))
> continue;
> page = pfn_to_page(pfn);
> scan_block(page, page+1, NULL);
> }
> }
It seems that node_start_pfn() isn't present on all the architectures. I
ended up with something like below:
+ /* struct page scanning for each node */
+ for_each_online_node(i) {
+ pg_data_t *pgdat = NODE_DATA(i);
+ unsigned long start_pfn = pgdat->node_start_pfn;
+ unsigned long end_pfn = start_pfn +
+ pgdat->node_spanned_pages - 1;
+ unsigned long pfn;
+
+ for (pfn = start_pfn; pfn < end_pfn; pfn++) {
+ struct page *page;
+
+ if (!pfn_valid(pfn))
+ continue;
+ page = pfn_to_page(pfn);
+ /* only scan if page is in use */
+ if (page_count(page) == 0)
+ continue;
+ scan_block(page, page + 1, NULL);
+ }
+ }
Are the pgdat->node_start_pfn and pgdat->node_spanned_pages always
valid? Thanks.
--
Catalin
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Regression - locking (all from 2.6.28)
2009-03-06 16:40 ` Catalin Marinas
@ 2009-03-06 16:52 ` Dave Hansen
2009-03-06 17:18 ` Catalin Marinas
0 siblings, 1 reply; 19+ messages in thread
From: Dave Hansen @ 2009-03-06 16:52 UTC (permalink / raw)
To: Catalin Marinas
Cc: Andrew Morton, jan sonnek, linux-kernel, viro, Peter Zijlstra,
Andy Whitcroft
On Fri, 2009-03-06 at 16:40 +0000, Catalin Marinas wrote:
> It seems that node_start_pfn() isn't present on all the architectures. I
> ended up with something like below:
>
> + /* struct page scanning for each node */
> + for_each_online_node(i) {
> + pg_data_t *pgdat = NODE_DATA(i);
> + unsigned long start_pfn = pgdat->node_start_pfn;
> + unsigned long end_pfn = start_pfn +
> + pgdat->node_spanned_pages - 1;
> + unsigned long pfn;
> +
> + for (pfn = start_pfn; pfn < end_pfn; pfn++) {
> + struct page *page;
> +
> + if (!pfn_valid(pfn))
> + continue;
> + page = pfn_to_page(pfn);
> + /* only scan if page is in use */
> + if (page_count(page) == 0)
> + continue;
> + scan_block(page, page + 1, NULL);
> + }
> + }
What does scan_block() actually scan? Is that second argument
inclusive?
I think you will miss scanning the contents of the last 'struct page' if
you do it this way because of the -1 you do to end_pfn.
> Are the pgdat->node_start_pfn and pgdat->node_spanned_pages always
> valid? Thanks.
The variables themselves? I'm sure there's a window in early boot where
they aren't valid, but other than that they should be OK unless you're
int the middle of a hotplug operation.
See pgdat_resize_(un)lock() in include/linux/memory_hotplug.h.
-- Dave
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Regression - locking (all from 2.6.28)
2009-03-06 16:52 ` Dave Hansen
@ 2009-03-06 17:18 ` Catalin Marinas
2009-03-06 17:26 ` Dave Hansen
0 siblings, 1 reply; 19+ messages in thread
From: Catalin Marinas @ 2009-03-06 17:18 UTC (permalink / raw)
To: Dave Hansen
Cc: Andrew Morton, jan sonnek, linux-kernel, viro, Peter Zijlstra,
Andy Whitcroft
On Fri, 2009-03-06 at 08:52 -0800, Dave Hansen wrote:
> On Fri, 2009-03-06 at 16:40 +0000, Catalin Marinas wrote:
> > It seems that node_start_pfn() isn't present on all the architectures. I
> > ended up with something like below:
> >
> > + /* struct page scanning for each node */
> > + for_each_online_node(i) {
> > + pg_data_t *pgdat = NODE_DATA(i);
> > + unsigned long start_pfn = pgdat->node_start_pfn;
> > + unsigned long end_pfn = start_pfn +
> > + pgdat->node_spanned_pages - 1;
> > + unsigned long pfn;
> > +
> > + for (pfn = start_pfn; pfn < end_pfn; pfn++) {
> > + struct page *page;
> > +
> > + if (!pfn_valid(pfn))
> > + continue;
> > + page = pfn_to_page(pfn);
> > + /* only scan if page is in use */
> > + if (page_count(page) == 0)
> > + continue;
> > + scan_block(page, page + 1, NULL);
> > + }
> > + }
>
> What does scan_block() actually scan? Is that second argument
> inclusive?
No, it's exclusive. In the above case, it just scans a struct page at
the given pointer.
> I think you will miss scanning the contents of the last 'struct page' if
> you do it this way because of the -1 you do to end_pfn.
Fixed.
> > Are the pgdat->node_start_pfn and pgdat->node_spanned_pages always
> > valid? Thanks.
>
> The variables themselves? I'm sure there's a window in early boot where
> they aren't valid, but other than that they should be OK unless you're
> int the middle of a hotplug operation.
>
> See pgdat_resize_(un)lock() in include/linux/memory_hotplug.h.
I wouldn't hold a lock for that long. It's not really critical to scan
all the page structures at a time as there are subsequent scans as well,
so some can be missed.
--
Catalin
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Regression - locking (all from 2.6.28)
2009-03-06 17:18 ` Catalin Marinas
@ 2009-03-06 17:26 ` Dave Hansen
2009-03-06 18:00 ` Catalin Marinas
0 siblings, 1 reply; 19+ messages in thread
From: Dave Hansen @ 2009-03-06 17:26 UTC (permalink / raw)
To: Catalin Marinas
Cc: Andrew Morton, jan sonnek, linux-kernel, viro, Peter Zijlstra,
Andy Whitcroft
On Fri, 2009-03-06 at 17:18 +0000, Catalin Marinas wrote:
> > > Are the pgdat->node_start_pfn and pgdat->node_spanned_pages always
> > > valid? Thanks.
> >
> > The variables themselves? I'm sure there's a window in early boot where
> > they aren't valid, but other than that they should be OK unless you're
> > int the middle of a hotplug operation.
> >
> > See pgdat_resize_(un)lock() in include/linux/memory_hotplug.h.
>
> I wouldn't hold a lock for that long. It's not really critical to scan
> all the page structures at a time as there are subsequent scans as well,
> so some can be missed.
I think you should be more worried about consistency rather than missing
entries. Take these two lines of code:
start_pfn = node->node_start_pfn;
/* hotplug occurs here */
end_pfn = start_pfn + node->node_spanned_pages;
What if someone comes in and adds memory to the node, at the beginning
of the node, after you have calculated start_pfn? Try to think of what
value you'll get for end_pfn and whether it is consistent and was *ever*
valid at all. Would that oops the kernel?
-- Dave
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Regression - locking (all from 2.6.28)
2009-03-06 17:26 ` Dave Hansen
@ 2009-03-06 18:00 ` Catalin Marinas
2009-03-06 19:19 ` Dave Hansen
0 siblings, 1 reply; 19+ messages in thread
From: Catalin Marinas @ 2009-03-06 18:00 UTC (permalink / raw)
To: Dave Hansen
Cc: Andrew Morton, jan sonnek, linux-kernel, viro, Peter Zijlstra,
Andy Whitcroft
On Fri, 2009-03-06 at 09:26 -0800, Dave Hansen wrote:
> On Fri, 2009-03-06 at 17:18 +0000, Catalin Marinas wrote:
> > > > Are the pgdat->node_start_pfn and pgdat->node_spanned_pages always
> > > > valid? Thanks.
> > >
> > > The variables themselves? I'm sure there's a window in early boot where
> > > they aren't valid, but other than that they should be OK unless you're
> > > int the middle of a hotplug operation.
> > >
> > > See pgdat_resize_(un)lock() in include/linux/memory_hotplug.h.
> >
> > I wouldn't hold a lock for that long. It's not really critical to scan
> > all the page structures at a time as there are subsequent scans as well,
> > so some can be missed.
>
> I think you should be more worried about consistency rather than missing
> entries. Take these two lines of code:
>
> start_pfn = node->node_start_pfn;
> /* hotplug occurs here */
> end_pfn = start_pfn + node->node_spanned_pages;
>
> What if someone comes in and adds memory to the node, at the beginning
> of the node, after you have calculated start_pfn? Try to think of what
> value you'll get for end_pfn and whether it is consistent and was *ever*
> valid at all. Would that oops the kernel?
I assume pfn_valid() should handle this and kmemleak wouldn't scan the
page, unless we need locks around pfn_valid as well but I haven't seen
any used in the kernel.
--
Catalin
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Regression - locking (all from 2.6.28)
2009-03-06 18:00 ` Catalin Marinas
@ 2009-03-06 19:19 ` Dave Hansen
2009-03-06 19:28 ` Pavel Machek
2009-03-16 17:12 ` Catalin Marinas
0 siblings, 2 replies; 19+ messages in thread
From: Dave Hansen @ 2009-03-06 19:19 UTC (permalink / raw)
To: Catalin Marinas
Cc: Andrew Morton, jan sonnek, linux-kernel, viro, Peter Zijlstra,
Andy Whitcroft, Pavel Machek
On Fri, 2009-03-06 at 18:00 +0000, Catalin Marinas wrote:
> > I think you should be more worried about consistency rather than missing
> > entries. Take these two lines of code:
> >
> > start_pfn = node->node_start_pfn;
> > /* hotplug occurs here */
> > end_pfn = start_pfn + node->node_spanned_pages;
> >
> > What if someone comes in and adds memory to the node, at the beginning
> > of the node, after you have calculated start_pfn? Try to think of what
> > value you'll get for end_pfn and whether it is consistent and was *ever*
> > valid at all. Would that oops the kernel?
>
> I assume pfn_valid() should handle this and kmemleak wouldn't scan the
> page, unless we need locks around pfn_valid as well but I haven't seen
> any used in the kernel.
You assume incorrectly. :(
Take my above example, and assume that you have two nodes which are
right next to each other. You might run over the end of one node and
into the next one. Your pages will be pfn_valid() but you will be on
the wrong node.
Please take a look at those locks that I mentioned. Notice that they
are lock the pgdat *span*, not the validity of pages inside the pgdat.
Your code deals with what pages the pgdats *span* and thus needs that
lock. Notice that my example also had to do with those two lines of
code incorrectly guessing the pgdat's *span*.
We recently went to some pain to make sure that the software suspend
code (which walks pgdat ranges as well) worked with memory hotplug.
There really isn't that much code around that actually cares at runtime
about which physical areas a particular node or zone spans. Yours is a
rarity and will require some caution.
You could probably also use the memory hotplug mutex found here:
https://lists.linux-foundation.org/pipermail/linux-pm/2008-November/018884.html
But I'm not sure where those patches have gone. Hmmm. Pavel?
-- Dave
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Regression - locking (all from 2.6.28)
2009-03-06 19:19 ` Dave Hansen
@ 2009-03-06 19:28 ` Pavel Machek
2009-03-16 22:04 ` Rafael J. Wysocki
2009-03-16 17:12 ` Catalin Marinas
1 sibling, 1 reply; 19+ messages in thread
From: Pavel Machek @ 2009-03-06 19:28 UTC (permalink / raw)
To: Dave Hansen
Cc: Catalin Marinas, Andrew Morton, jan sonnek, linux-kernel, viro,
Peter Zijlstra, Andy Whitcroft, Rafael J. Wysocki
Hi!
On Fri 2009-03-06 11:19:46, Dave Hansen wrote:
> On Fri, 2009-03-06 at 18:00 +0000, Catalin Marinas wrote:
> > > I think you should be more worried about consistency rather than missing
> > > entries. Take these two lines of code:
> > >
> > > start_pfn = node->node_start_pfn;
> > > /* hotplug occurs here */
> > > end_pfn = start_pfn + node->node_spanned_pages;
> > >
> > > What if someone comes in and adds memory to the node, at the beginning
> > > of the node, after you have calculated start_pfn? Try to think of what
> > > value you'll get for end_pfn and whether it is consistent and was *ever*
> > > valid at all. Would that oops the kernel?
> >
> > I assume pfn_valid() should handle this and kmemleak wouldn't scan the
> > page, unless we need locks around pfn_valid as well but I haven't seen
> > any used in the kernel.
>
> You assume incorrectly. :(
>
> Take my above example, and assume that you have two nodes which are
> right next to each other. You might run over the end of one node and
> into the next one. Your pages will be pfn_valid() but you will be on
> the wrong node.
>
> Please take a look at those locks that I mentioned. Notice that they
> are lock the pgdat *span*, not the validity of pages inside the pgdat.
> Your code deals with what pages the pgdats *span* and thus needs that
> lock. Notice that my example also had to do with those two lines of
> code incorrectly guessing the pgdat's *span*.
>
> We recently went to some pain to make sure that the software suspend
> code (which walks pgdat ranges as well) worked with memory hotplug.
> There really isn't that much code around that actually cares at runtime
> about which physical areas a particular node or zone spans. Yours is a
> rarity and will require some caution.
>
> You could probably also use the memory hotplug mutex found here:
>
> https://lists.linux-foundation.org/pipermail/linux-pm/2008-November/018884.html
>
> But I'm not sure where those patches have gone. Hmmm. Pavel?
I don't think they were applied. They probably should... Rafael was
about to look into that, but he lost the patch pointer.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Regression - locking (all from 2.6.28)
2009-03-17 0:07 ` KAMEZAWA Hiroyuki
@ 2009-03-14 16:24 ` Pavel Machek
0 siblings, 0 replies; 19+ messages in thread
From: Pavel Machek @ 2009-03-14 16:24 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki
Cc: Rafael J. Wysocki, Dave Hansen, Catalin Marinas, Andrew Morton,
jan sonnek, linux-kernel, viro, Peter Zijlstra, Andy Whitcroft
On Tue 2009-03-17 09:07:32, KAMEZAWA Hiroyuki wrote:
> On Mon, 16 Mar 2009 23:04:27 +0100
> "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
>
> > On Friday 06 March 2009, Pavel Machek wrote:
> > > Hi!
> > >
> > > On Fri 2009-03-06 11:19:46, Dave Hansen wrote:
> > > > On Fri, 2009-03-06 at 18:00 +0000, Catalin Marinas wrote:
> > > > > > I think you should be more worried about consistency rather than missing
> > > > > > entries. Take these two lines of code:
> > > > > >
> > > > > > start_pfn = node->node_start_pfn;
> > > > > > /* hotplug occurs here */
> > > > > > end_pfn = start_pfn + node->node_spanned_pages;
> > > > > >
> > > > > > What if someone comes in and adds memory to the node, at the beginning
> > > > > > of the node, after you have calculated start_pfn? Try to think of what
> > > > > > value you'll get for end_pfn and whether it is consistent and was *ever*
> > > > > > valid at all. Would that oops the kernel?
> > > > >
> > > > > I assume pfn_valid() should handle this and kmemleak wouldn't scan the
> > > > > page, unless we need locks around pfn_valid as well but I haven't seen
> > > > > any used in the kernel.
> > > >
> > > > You assume incorrectly. :(
> > > >
> > > > Take my above example, and assume that you have two nodes which are
> > > > right next to each other. You might run over the end of one node and
> > > > into the next one. Your pages will be pfn_valid() but you will be on
> > > > the wrong node.
> > > >
> > > > Please take a look at those locks that I mentioned. Notice that they
> > > > are lock the pgdat *span*, not the validity of pages inside the pgdat.
> > > > Your code deals with what pages the pgdats *span* and thus needs that
> > > > lock. Notice that my example also had to do with those two lines of
> > > > code incorrectly guessing the pgdat's *span*.
> > > >
> > > > We recently went to some pain to make sure that the software suspend
> > > > code (which walks pgdat ranges as well) worked with memory hotplug.
> > > > There really isn't that much code around that actually cares at runtime
> > > > about which physical areas a particular node or zone spans. Yours is a
> > > > rarity and will require some caution.
> > > >
> > > > You could probably also use the memory hotplug mutex found here:
> > > >
> > > > https://lists.linux-foundation.org/pipermail/linux-pm/2008-November/018884.html
> > > >
> > > > But I'm not sure where those patches have gone. Hmmm. Pavel?
> > >
> > > I don't think they were applied. They probably should... Rafael was
> > > about to look into that, but he lost the patch pointer.
> >
> > Yes. In fact, sending the patch again to me would be appreciated.
> >
>
> Ah...but as I wrote, for me, testing this is difficult.
> Could some expert of hibernation update and post this again ? For memoyr hotplug.
> it seems enough that sending the patch witch CC: to linux-mm and including
> "Memory Hotplug" in the subject.
>
Well, testing this is probably difficult for everyone, as
hotplug-capable machines are rare/expensive.
Can youjust retransmit the patch to rafael?
>
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Regression - locking (all from 2.6.28)
2009-03-06 19:19 ` Dave Hansen
2009-03-06 19:28 ` Pavel Machek
@ 2009-03-16 17:12 ` Catalin Marinas
1 sibling, 0 replies; 19+ messages in thread
From: Catalin Marinas @ 2009-03-16 17:12 UTC (permalink / raw)
To: Dave Hansen
Cc: Andrew Morton, jan sonnek, linux-kernel, viro, Peter Zijlstra,
Andy Whitcroft, Pavel Machek
Hi Dave,
> On Fri, 2009-03-06 at 18:00 +0000, Catalin Marinas wrote:
> > > I think you should be more worried about consistency rather than missing
> > > entries. Take these two lines of code:
> > >
> > > start_pfn = node->node_start_pfn;
> > > /* hotplug occurs here */
> > > end_pfn = start_pfn + node->node_spanned_pages;
> > >
> > > What if someone comes in and adds memory to the node, at the beginning
> > > of the node, after you have calculated start_pfn? Try to think of what
> > > value you'll get for end_pfn and whether it is consistent and was *ever*
> > > valid at all. Would that oops the kernel?
> >
> > I assume pfn_valid() should handle this and kmemleak wouldn't scan the
> > page, unless we need locks around pfn_valid as well but I haven't seen
> > any used in the kernel.
>
> You assume incorrectly. :(
>
> Take my above example, and assume that you have two nodes which are
> right next to each other. You might run over the end of one node and
> into the next one. Your pages will be pfn_valid() but you will be on
> the wrong node.
OK, thanks for taking the time to explain this. I currently added a
dependency on !MEMORY_HOTPLUG for kmemleak since holding the lock while
traversing the page structures is not really feasible.
> You could probably also use the memory hotplug mutex found here:
>
> https://lists.linux-foundation.org/pipermail/linux-pm/2008-November/018884.html
That would be a better option for kmemleak as well.
--
Catalin
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Regression - locking (all from 2.6.28)
2009-03-06 19:28 ` Pavel Machek
@ 2009-03-16 22:04 ` Rafael J. Wysocki
2009-03-17 0:07 ` KAMEZAWA Hiroyuki
0 siblings, 1 reply; 19+ messages in thread
From: Rafael J. Wysocki @ 2009-03-16 22:04 UTC (permalink / raw)
To: Pavel Machek
Cc: Dave Hansen, Catalin Marinas, Andrew Morton, jan sonnek,
linux-kernel, viro, Peter Zijlstra, Andy Whitcroft,
KAMEZAWA Hiroyuki
On Friday 06 March 2009, Pavel Machek wrote:
> Hi!
>
> On Fri 2009-03-06 11:19:46, Dave Hansen wrote:
> > On Fri, 2009-03-06 at 18:00 +0000, Catalin Marinas wrote:
> > > > I think you should be more worried about consistency rather than missing
> > > > entries. Take these two lines of code:
> > > >
> > > > start_pfn = node->node_start_pfn;
> > > > /* hotplug occurs here */
> > > > end_pfn = start_pfn + node->node_spanned_pages;
> > > >
> > > > What if someone comes in and adds memory to the node, at the beginning
> > > > of the node, after you have calculated start_pfn? Try to think of what
> > > > value you'll get for end_pfn and whether it is consistent and was *ever*
> > > > valid at all. Would that oops the kernel?
> > >
> > > I assume pfn_valid() should handle this and kmemleak wouldn't scan the
> > > page, unless we need locks around pfn_valid as well but I haven't seen
> > > any used in the kernel.
> >
> > You assume incorrectly. :(
> >
> > Take my above example, and assume that you have two nodes which are
> > right next to each other. You might run over the end of one node and
> > into the next one. Your pages will be pfn_valid() but you will be on
> > the wrong node.
> >
> > Please take a look at those locks that I mentioned. Notice that they
> > are lock the pgdat *span*, not the validity of pages inside the pgdat.
> > Your code deals with what pages the pgdats *span* and thus needs that
> > lock. Notice that my example also had to do with those two lines of
> > code incorrectly guessing the pgdat's *span*.
> >
> > We recently went to some pain to make sure that the software suspend
> > code (which walks pgdat ranges as well) worked with memory hotplug.
> > There really isn't that much code around that actually cares at runtime
> > about which physical areas a particular node or zone spans. Yours is a
> > rarity and will require some caution.
> >
> > You could probably also use the memory hotplug mutex found here:
> >
> > https://lists.linux-foundation.org/pipermail/linux-pm/2008-November/018884.html
> >
> > But I'm not sure where those patches have gone. Hmmm. Pavel?
>
> I don't think they were applied. They probably should... Rafael was
> about to look into that, but he lost the patch pointer.
Yes. In fact, sending the patch again to me would be appreciated.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Regression - locking (all from 2.6.28)
2009-03-16 22:04 ` Rafael J. Wysocki
@ 2009-03-17 0:07 ` KAMEZAWA Hiroyuki
2009-03-14 16:24 ` Pavel Machek
0 siblings, 1 reply; 19+ messages in thread
From: KAMEZAWA Hiroyuki @ 2009-03-17 0:07 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Pavel Machek, Dave Hansen, Catalin Marinas, Andrew Morton,
jan sonnek, linux-kernel, viro, Peter Zijlstra, Andy Whitcroft
On Mon, 16 Mar 2009 23:04:27 +0100
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> On Friday 06 March 2009, Pavel Machek wrote:
> > Hi!
> >
> > On Fri 2009-03-06 11:19:46, Dave Hansen wrote:
> > > On Fri, 2009-03-06 at 18:00 +0000, Catalin Marinas wrote:
> > > > > I think you should be more worried about consistency rather than missing
> > > > > entries. Take these two lines of code:
> > > > >
> > > > > start_pfn = node->node_start_pfn;
> > > > > /* hotplug occurs here */
> > > > > end_pfn = start_pfn + node->node_spanned_pages;
> > > > >
> > > > > What if someone comes in and adds memory to the node, at the beginning
> > > > > of the node, after you have calculated start_pfn? Try to think of what
> > > > > value you'll get for end_pfn and whether it is consistent and was *ever*
> > > > > valid at all. Would that oops the kernel?
> > > >
> > > > I assume pfn_valid() should handle this and kmemleak wouldn't scan the
> > > > page, unless we need locks around pfn_valid as well but I haven't seen
> > > > any used in the kernel.
> > >
> > > You assume incorrectly. :(
> > >
> > > Take my above example, and assume that you have two nodes which are
> > > right next to each other. You might run over the end of one node and
> > > into the next one. Your pages will be pfn_valid() but you will be on
> > > the wrong node.
> > >
> > > Please take a look at those locks that I mentioned. Notice that they
> > > are lock the pgdat *span*, not the validity of pages inside the pgdat.
> > > Your code deals with what pages the pgdats *span* and thus needs that
> > > lock. Notice that my example also had to do with those two lines of
> > > code incorrectly guessing the pgdat's *span*.
> > >
> > > We recently went to some pain to make sure that the software suspend
> > > code (which walks pgdat ranges as well) worked with memory hotplug.
> > > There really isn't that much code around that actually cares at runtime
> > > about which physical areas a particular node or zone spans. Yours is a
> > > rarity and will require some caution.
> > >
> > > You could probably also use the memory hotplug mutex found here:
> > >
> > > https://lists.linux-foundation.org/pipermail/linux-pm/2008-November/018884.html
> > >
> > > But I'm not sure where those patches have gone. Hmmm. Pavel?
> >
> > I don't think they were applied. They probably should... Rafael was
> > about to look into that, but he lost the patch pointer.
>
> Yes. In fact, sending the patch again to me would be appreciated.
>
Ah...but as I wrote, for me, testing this is difficult.
Could some expert of hibernation update and post this again ? For memoyr hotplug.
it seems enough that sending the patch witch CC: to linux-mm and including
"Memory Hotplug" in the subject.
Thanks,
-Kame
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Regression - locking (all from 2.6.28)
@ 2009-03-22 4:45 jan sonnek
0 siblings, 0 replies; 19+ messages in thread
From: jan sonnek @ 2009-03-22 4:45 UTC (permalink / raw)
To: linux-kernel; +Cc: catalin.marinas, linux-scsi, akpm
[-- Attachment #1: Type: text/plain, Size: 4071 bytes --]
I have tried applied some patch (view down) to -mm kernel
(last mmotm 26.29-rc8-mm1), but It was not
successfull. When I have disabled Acceleration
(Option "NoAccel" "true"), I was able to start X system,
but in my dmesg is still error messages (attachement)
Bug page:
http://bugzilla.kernel.org/show_bug.cgi?id=12619
> On Mon, 2009-03-02 at 12:11 -0800, Andrew Morton wrote:
> > > Mar 1 00:06:51 localhost kernel: [ 74.008165] unreferenced
object 0xf6c4daf0 (size 52):
> > > Mar 1 00:06:51 localhost kernel: [ 74.008170] comm
"swapper", pid 1, jiffies 4294893427
> > > Mar 1 00:06:51 localhost kernel: [ 74.008175] backtrace:
> > > Mar 1 00:06:51 localhost kernel: [ 74.008179] [<c018978c>]
kmemleak_alloc+0x17e/0x28e
> > > Mar 1 00:06:51 localhost kernel: [ 74.008185] [<c0186b86>]
kmem_cache_alloc+0xdc/0xe7
> > > Mar 1 00:06:51 localhost kernel: [ 74.008190] [<c01a53bd>]
alloc_buffer_head+0x16/0x71
> > > Mar 1 00:06:51 localhost kernel: [ 74.008196] [<c01a5b91>]
alloc_page_buffers+0x23/0xad
> > > Mar 1 00:06:51 localhost kernel: [ 74.008200] [<c01a5fd4>]
__getblk+0x192/0x26b
> > > Mar 1 00:06:51 localhost kernel: [ 74.008205] [<c01d91f4>]
jread+0x105/0x1de
> > > Mar 1 00:06:51 localhost kernel: [ 74.008209] [<c01d932b>]
do_one_pass+0x5e/0x38c
> > > Mar 1 00:06:51 localhost kernel: [ 74.008213] [<c01d96f8>]
journal_recover+0x41/0x9d
> > > Mar 1 00:06:51 localhost kernel: [ 74.008218] [<c01db8d4>]
journal_load+0x47/0x7b
> > > Mar 1 00:06:51 localhost kernel: [ 74.008221] [<c01d43d1>]
ext3_fill_super+0xe9d/0x144c
> > > Mar 1 00:06:51 localhost kernel: [ 74.008225] [<c018d721>]
get_sb_bdev+0xfa/0x140
> > > Mar 1 00:06:51 localhost kernel: [ 74.008231] [<c01d2070>]
ext3_get_sb+0x18/0x1a
> > > Mar 1 00:06:51 localhost kernel: [ 74.008235] [<c018c71f>]
vfs_kern_mount+0x41/0x7c
> > > Mar 1 00:06:51 localhost kernel: [ 74.008241] [<c018c7a8>]
do_kern_mount+0x37/0xbe
> > > Mar 1 00:06:51 localhost kernel: [ 74.008247] [<c019f0bf>]
do_mount+0x5f7/0x630
> > > Mar 1 00:06:51 localhost kernel: [ 74.008253] [<c019f167>]
sys_mount+0x6f/0xac
> > I suspect kmemleak has gone nuts here.
>
> It seems that the buffer_head structure allocated above is stored in
> page->private. However, the page structures are no longer scanned in
> newer versions of kmemleak. That's the hunk that was removed after
> comments about the contiguity of a node's memory:
>
> + /* mem_map scanning */
> + for_each_online_node(i) {
> + struct page *page, *end;
> +
> + page = NODE_MEM_MAP(i);
> + end = page + NODE_DATA(i)->node_spanned_pages;
> +
> + scan_block(page, end, NULL);
> + }
>
> The alternative is to inform kmemleak about the page structures returned
> from __alloc_pages_internal() but there would be problems with recursive
> calls into kmemleak when it allocates its own data structures.
>
> I'll look at re-adding the hunk above, maybe with some extra checks like
> pfn_valid().
Looking again at this, the node_mem_map is always contiguous and the
code above only scans the node_mem_map, not the memory represented by
the node (which may not be contiguous). So I think it is a valid code
sequence.
If the above gets too deep into the nodes structure, an alternative
would be:
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 09b6fd7..0f17e62 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3552,6 +3552,11 @@ static void __init_refok
alloc_node_mem_map(struct pglist_data *pgdat)
map = alloc_remap(pgdat->node_id, size);
if (!map)
map = alloc_bootmem_node(pgdat, size);
+ /*
+ * Inform kmemleak to scan the node_mem_map arrays as the page
+ * structure may contain pointers to other objects.
+ */
+ kmemleak_alloc(map, size, 1, GFP_ATOMIC);
pgdat->node_mem_map = map + (pgdat->node_start_pfn - start);
}
#ifndef CONFIG_NEED_MULTIPLE_NODES
--
Many thanks,
Jan Sonnek
[-- Attachment #2: dmesg-noaccel --]
[-- Type: text/plain, Size: 13400 bytes --]
[ 1724.965479] =========================================================
[ 1724.965491] [ INFO: possible irq lock inversion dependency detected ]
[ 1724.965501] 2.6.29-rc8-mm1-hanny #23
[ 1724.965507] ---------------------------------------------------------
[ 1724.965516] swapper/0 just changed the state of lock:
[ 1724.965523] (fasync_lock){.-....}, at: [<c019867b>] kill_fasync+0x20/0x3a
[ 1724.965545] but this lock took another, HARDIRQ-unsafe lock in the past:
[ 1724.965552] (&f->f_lock){+.+...}
[ 1724.965560]
[ 1724.965562] and interrupts could create inverse lock ordering between them.
[ 1724.965568]
[ 1724.965574]
[ 1724.965576] other info that might help us debug this:
[ 1724.965584] 3 locks held by swapper/0:
[ 1724.965590] #0: (&dev->event_lock){-.-...}, at: [<c02da4cf>] input_event+0x35/0x6a
[ 1724.965610] #1: (rcu_read_lock){.+.+..}, at: [<c02d9136>] __rcu_read_lock+0x0/0x30
[ 1724.965631] #2: (rcu_read_lock){.+.+..}, at: [<c02dcd6e>] evdev_event+0x0/0xe2
[ 1724.965649]
[ 1724.965651] the first lock's dependencies:
[ 1724.965657] -> (fasync_lock){.-....} ops: 253 {
[ 1724.965672] IN-HARDIRQ-R at:
[ 1724.965681] [<c0147a3a>] __lock_acquire+0x204/0xb4a
[ 1724.965696] [<c0148437>] lock_acquire+0xb7/0xd4
[ 1724.965709] [<c039df42>] _read_lock+0x2d/0x5d
[ 1724.965722] [<c019867b>] kill_fasync+0x20/0x3a
[ 1724.965733] [<c02dcaeb>] evdev_pass_event+0x60/0x66
[ 1724.965745] [<c02dcde1>] evdev_event+0x73/0xe2
[ 1724.965758] [<c02d91fc>] input_pass_event+0x5c/0x7f
[ 1724.965771] [<c02da419>] input_handle_event+0x366/0x36f
[ 1724.965784] [<c02da4ee>] input_event+0x54/0x6a
[ 1724.965796] [<c02f6a5f>] hidinput_hid_event+0x24c/0x279
[ 1724.965811] [<c02f3e49>] hid_process_event+0x8d/0xbc
[ 1724.965824] [<c02f41b4>] hid_report_raw_event+0x33c/0x348
[ 1724.965838] [<c02f426b>] hid_input_report+0xab/0xbc
[ 1724.965851] [<c02fa52c>] hid_irq_in+0x86/0x182
[ 1724.965864] [<c02b3e96>] usb_hcd_giveback_urb+0x68/0x9c
[ 1724.965878] [<c02d3247>] uhci_giveback_urb+0xf6/0x1f1
[ 1724.965891] [<c02d3a3c>] uhci_scan_schedule+0x5f8/0x85f
[ 1724.965903] [<c02d571f>] uhci_irq+0x12b/0x13f
[ 1724.965916] [<c02b3a68>] usb_hcd_irq+0x32/0x81
[ 1724.965928] [<c0157218>] handle_IRQ_event+0xa4/0x121
[ 1724.965942] [<c015843d>] handle_fasteoi_irq+0x77/0xb0
[ 1724.965955] [<ffffffff>] 0xffffffff
[ 1724.965967] INITIAL USE at:
[ 1724.965974] [<c0147b7a>] __lock_acquire+0x344/0xb4a
[ 1724.965986] [<c0148437>] lock_acquire+0xb7/0xd4
[ 1724.965998] [<c039dcdd>] _write_lock_irq+0x33/0x63
[ 1724.966010] [<c01982b6>] fasync_helper+0x44/0xe4
[ 1724.966022] [<c024b0ea>] tty_fasync+0x50/0xea
[ 1724.966037] [<c024d1ca>] tty_release_dev+0x57/0x409
[ 1724.966049] [<c024d593>] tty_release+0x17/0x21
[ 1724.966062] [<c018f5e1>] __fput+0xcf/0x158
[ 1724.966074] [<c018f688>] fput+0x1e/0x20
[ 1724.966085] [<c018cd8a>] filp_close+0x56/0x60
[ 1724.966098] [<c018ce03>] sys_close+0x6f/0xa9
[ 1724.966110] [<c0102b07>] sysenter_do_call+0x12/0x35
[ 1724.966124] [<ffffffff>] 0xffffffff
[ 1724.966133] }
[ 1724.966138] ... key at: [<c04f9324>] fasync_lock+0x10/0x24
[ 1724.966152] -> (&f->f_lock){+.+...} ops: 427 {
[ 1724.966166] HARDIRQ-ON-W at:
[ 1724.966174] [<c0147af8>] __lock_acquire+0x2c2/0xb4a
[ 1724.966188] [<c0148437>] lock_acquire+0xb7/0xd4
[ 1724.966200] [<c039da1c>] _spin_lock+0x2d/0x5d
[ 1724.966213] [<c01989de>] do_fcntl+0x222/0x2bc
[ 1724.966225] [<c0198ad2>] sys_fcntl64+0x5a/0x6e
[ 1724.966238] [<c0102b07>] sysenter_do_call+0x12/0x35
[ 1724.966250] [<ffffffff>] 0xffffffff
[ 1724.966261] SOFTIRQ-ON-W at:
[ 1724.966269] [<c0147b1b>] __lock_acquire+0x2e5/0xb4a
[ 1724.966281] [<c0148437>] lock_acquire+0xb7/0xd4
[ 1724.966293] [<c039da1c>] _spin_lock+0x2d/0x5d
[ 1724.966305] [<c01989de>] do_fcntl+0x222/0x2bc
[ 1724.966318] [<c0198ad2>] sys_fcntl64+0x5a/0x6e
[ 1724.966329] [<c0102b07>] sysenter_do_call+0x12/0x35
[ 1724.966340] [<ffffffff>] 0xffffffff
[ 1724.966351] INITIAL USE at:
[ 1724.966358] [<c0147b7a>] __lock_acquire+0x344/0xb4a
[ 1724.966371] [<c0148437>] lock_acquire+0xb7/0xd4
[ 1724.966383] [<c039da1c>] _spin_lock+0x2d/0x5d
[ 1724.966395] [<c0198326>] fasync_helper+0xb4/0xe4
[ 1724.966407] [<c024b0ea>] tty_fasync+0x50/0xea
[ 1724.966419] [<c024d1ca>] tty_release_dev+0x57/0x409
[ 1724.966431] [<c024d593>] tty_release+0x17/0x21
[ 1724.966444] [<c018f5e1>] __fput+0xcf/0x158
[ 1724.966455] [<c018f688>] fput+0x1e/0x20
[ 1724.966466] [<c018cd8a>] filp_close+0x56/0x60
[ 1724.966480] [<c018ce03>] sys_close+0x6f/0xa9
[ 1724.966492] [<c0102b07>] sysenter_do_call+0x12/0x35
[ 1724.966505] [<ffffffff>] 0xffffffff
[ 1724.966515] }
[ 1724.966519] ... key at: [<c0b7f750>] __key.20487+0x0/0x8
[ 1724.966531] ... acquired at:
[ 1724.966536] [<c01481fd>] __lock_acquire+0x9c7/0xb4a
[ 1724.966547] [<c0148437>] lock_acquire+0xb7/0xd4
[ 1724.966557] [<c039da1c>] _spin_lock+0x2d/0x5d
[ 1724.966566] [<c0198326>] fasync_helper+0xb4/0xe4
[ 1724.966575] [<c024b0ea>] tty_fasync+0x50/0xea
[ 1724.966585] [<c024d1ca>] tty_release_dev+0x57/0x409
[ 1724.966595] [<c024d593>] tty_release+0x17/0x21
[ 1724.966605] [<c018f5e1>] __fput+0xcf/0x158
[ 1724.966614] [<c018f688>] fput+0x1e/0x20
[ 1724.966622] [<c018cd8a>] filp_close+0x56/0x60
[ 1724.966633] [<c018ce03>] sys_close+0x6f/0xa9
[ 1724.966643] [<c0102b07>] sysenter_do_call+0x12/0x35
[ 1724.966653] [<ffffffff>] 0xffffffff
[ 1724.966662]
[ 1724.966666]
[ 1724.966668] the second lock's dependencies:
[ 1724.966675] -> (&f->f_lock){+.+...} ops: 427 {
[ 1724.966689] HARDIRQ-ON-W at:
[ 1724.966696] [<c0147af8>] __lock_acquire+0x2c2/0xb4a
[ 1724.966710] [<c0148437>] lock_acquire+0xb7/0xd4
[ 1724.966722] [<c039da1c>] _spin_lock+0x2d/0x5d
[ 1724.966734] [<c01989de>] do_fcntl+0x222/0x2bc
[ 1724.966746] [<c0198ad2>] sys_fcntl64+0x5a/0x6e
[ 1724.966758] [<c0102b07>] sysenter_do_call+0x12/0x35
[ 1724.966770] [<ffffffff>] 0xffffffff
[ 1724.966781] SOFTIRQ-ON-W at:
[ 1724.966788] [<c0147b1b>] __lock_acquire+0x2e5/0xb4a
[ 1724.966801] [<c0148437>] lock_acquire+0xb7/0xd4
[ 1724.966814] [<c039da1c>] _spin_lock+0x2d/0x5d
[ 1724.966825] [<c01989de>] do_fcntl+0x222/0x2bc
[ 1724.966836] [<c0198ad2>] sys_fcntl64+0x5a/0x6e
[ 1724.966847] [<c0102b07>] sysenter_do_call+0x12/0x35
[ 1724.966860] [<ffffffff>] 0xffffffff
[ 1724.966870] INITIAL USE at:
[ 1724.966877] [<c0147b7a>] __lock_acquire+0x344/0xb4a
[ 1724.966890] [<c0148437>] lock_acquire+0xb7/0xd4
[ 1724.966903] [<c039da1c>] _spin_lock+0x2d/0x5d
[ 1724.966914] [<c0198326>] fasync_helper+0xb4/0xe4
[ 1724.966926] [<c024b0ea>] tty_fasync+0x50/0xea
[ 1724.966939] [<c024d1ca>] tty_release_dev+0x57/0x409
[ 1724.966951] [<c024d593>] tty_release+0x17/0x21
[ 1724.966963] [<c018f5e1>] __fput+0xcf/0x158
[ 1724.966975] [<c018f688>] fput+0x1e/0x20
[ 1724.966987] [<c018cd8a>] filp_close+0x56/0x60
[ 1724.966999] [<c018ce03>] sys_close+0x6f/0xa9
[ 1724.967011] [<c0102b07>] sysenter_do_call+0x12/0x35
[ 1724.967024] [<ffffffff>] 0xffffffff
[ 1724.967034] }
[ 1724.967038] ... key at: [<c0b7f750>] __key.20487+0x0/0x8
[ 1724.967049]
[ 1724.967051] stack backtrace:
[ 1724.967059] Pid: 0, comm: swapper Not tainted 2.6.29-rc8-mm1-hanny #23
[ 1724.967066] Call Trace:
[ 1724.967077] [<c039b4d5>] ? printk+0x14/0x17
[ 1724.967087] [<c01472f0>] print_irq_inversion_bug+0xea/0xf7
[ 1724.967098] [<c0147333>] check_usage_forwards+0x36/0x3f
[ 1724.967107] [<c0146cbb>] mark_lock+0x110/0x1f4
[ 1724.967117] [<c01472fd>] ? check_usage_forwards+0x0/0x3f
[ 1724.967128] [<c0147a3a>] __lock_acquire+0x204/0xb4a
[ 1724.967140] [<c0148437>] lock_acquire+0xb7/0xd4
[ 1724.967150] [<c019867b>] ? kill_fasync+0x20/0x3a
[ 1724.967160] [<c039df42>] _read_lock+0x2d/0x5d
[ 1724.967170] [<c019867b>] ? kill_fasync+0x20/0x3a
[ 1724.967179] [<c019867b>] kill_fasync+0x20/0x3a
[ 1724.967189] [<c02dcaeb>] evdev_pass_event+0x60/0x66
[ 1724.967198] [<c02dcde1>] evdev_event+0x73/0xe2
[ 1724.967210] [<c02d91fc>] input_pass_event+0x5c/0x7f
[ 1724.967220] [<c02da419>] input_handle_event+0x366/0x36f
[ 1724.967232] [<c024ad54>] ? add_timer_randomness+0xee/0x108
[ 1724.967243] [<c02da4ee>] input_event+0x54/0x6a
[ 1724.967254] [<c02f6a5f>] hidinput_hid_event+0x24c/0x279
[ 1724.967265] [<c02f3e49>] hid_process_event+0x8d/0xbc
[ 1724.967276] [<c02f41b4>] hid_report_raw_event+0x33c/0x348
[ 1724.967289] [<c02f426b>] hid_input_report+0xab/0xbc
[ 1724.967299] [<c02fa52c>] hid_irq_in+0x86/0x182
[ 1724.967309] [<c02b59a2>] ? usb_unanchor_urb+0xb/0x85
[ 1724.967320] [<c02b3e96>] usb_hcd_giveback_urb+0x68/0x9c
[ 1724.967331] [<c02d3247>] uhci_giveback_urb+0xf6/0x1f1
[ 1724.967340] [<c039d867>] ? _spin_unlock_irqrestore+0x58/0x65
[ 1724.967352] [<c02d3a3c>] uhci_scan_schedule+0x5f8/0x85f
[ 1724.967363] [<c0145d3a>] ? put_lock_stats+0x1e/0x29
[ 1724.967374] [<c02d571f>] uhci_irq+0x12b/0x13f
[ 1724.967384] [<c02b3a68>] usb_hcd_irq+0x32/0x81
[ 1724.967395] [<c0157218>] handle_IRQ_event+0xa4/0x121
[ 1724.967406] [<c015843d>] handle_fasteoi_irq+0x77/0xb0
[ 1724.967416] [<c01583c6>] ? handle_fasteoi_irq+0x0/0xb0
[ 1724.967423] <IRQ> [<c039e32a>] ? do_IRQ+0x4a/0x8c
[ 1724.967440] [<c010316e>] ? common_interrupt+0x2e/0x34
[ 1724.967452] [<c014007b>] ? timekeeping_suspend+0x61/0x70
[ 1724.967465] [<c023e552>] ? acpi_idle_enter_simple+0x13a/0x165
[ 1724.967477] [<c02f184e>] ? cpuidle_idle_call+0x6a/0x9c
[ 1724.967487] [<c0101d9b>] ? cpu_idle+0x7f/0xb4
[ 1724.967498] [<c0398e34>] ? start_secondary+0x20e/0x24e
[ 1921.835352] unreferenced object 0xf2fa1100 (size 64):
[ 1921.835359] comm "dbus-daemon", pid 2852, jiffies 364589
[ 1921.835363] backtrace:
[ 1921.835372] [<c018ca01>] kmemleak_alloc+0x17e/0x291
[ 1921.835377] [<c0189872>] __kmalloc+0x10f/0x11a
[ 1921.835382] [<c0199e94>] do_sys_poll+0xb8/0x3bf
[ 1921.835386] [<c019a2ea>] sys_poll+0x45/0x8f
[ 1921.835391] [<c0102b07>] sysenter_do_call+0x12/0x35
[ 1921.835396] [<ffffffff>] 0xffffffff
^ permalink raw reply related [flat|nested] 19+ messages in thread
end of thread, other threads:[~2009-03-21 21:45 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <49AC334A.9030800@gmail.com>
2009-03-02 20:11 ` Regression - locking (all from 2.6.28) Andrew Morton
2009-03-03 10:41 ` Catalin Marinas
2009-03-03 15:01 ` Catalin Marinas
2009-03-05 0:54 ` Dave Hansen
2009-03-05 18:04 ` Catalin Marinas
2009-03-05 18:29 ` Peter Zijlstra
2009-03-06 16:40 ` Catalin Marinas
2009-03-06 16:52 ` Dave Hansen
2009-03-06 17:18 ` Catalin Marinas
2009-03-06 17:26 ` Dave Hansen
2009-03-06 18:00 ` Catalin Marinas
2009-03-06 19:19 ` Dave Hansen
2009-03-06 19:28 ` Pavel Machek
2009-03-16 22:04 ` Rafael J. Wysocki
2009-03-17 0:07 ` KAMEZAWA Hiroyuki
2009-03-14 16:24 ` Pavel Machek
2009-03-16 17:12 ` Catalin Marinas
2009-03-03 18:12 ` Peter Zijlstra
2009-03-22 4:45 jan sonnek
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).