On Oct 5, 2019, at 7:29 PM, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
On Fri, 4 Oct 2019 12:42:26 -0400 Qian Cai <cai@xxxxxx> wrote:It is unsafe to call printk() while zone->lock was held, i.e.,
zone->lock --> console_sem
because the console could always allocate some memory in different code
paths and form locking chains in an opposite order,
console_sem --> * --> zone->lock
As the result, it triggers lockdep splats like below and in [1]. It is
fine to take zone->lock after has_unmovable_pages() (which has
dump_stack()) in set_migratetype_isolate(). While at it, remove a
problematic printk() in __offline_isolated_pages() only for debugging as
well which will always disable lockdep on debug kernels.
The problem is probably there forever, but neither many developers will
run memory offline with the lockdep enabled nor admins in the field are
lucky enough yet to hit a perfect timing which required to trigger a
real deadlock. In addition, there aren't many places that call printk()
while zone->lock was held.
WARNING: possible circular locking dependency detected
------------------------------------------------------
test.sh/1724 is trying to acquire lock:
0000000052059ec0 (console_owner){-...}, at: console_unlock+0x
01: 328/0xa30
but task is already holding lock:
000000006ffd89c8 (&(&zone->lock)->rlock){-.-.}, at: start_iso
01: late_page_range+0x216/0x538
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #2 (&(&zone->lock)->rlock){-.-.}:
lock_acquire+0x21a/0x468
_raw_spin_lock+0x54/0x68
get_page_from_freelist+0x8b6/0x2d28
__alloc_pages_nodemask+0x246/0x658
__get_free_pages+0x34/0x78
sclp_init+0x106/0x690
sclp_register+0x2e/0x248
sclp_rw_init+0x4a/0x70
sclp_console_init+0x4a/0x1b8
console_init+0x2c8/0x410
start_kernel+0x530/0x6a0
startup_continue+0x70/0xd0
This appears to be the core of our problem?
No, that is just one of those many places could form the lock chain.
console_lock -> other locks -> zone_lock
Another example is,
It is easier to avoid,
zone_lock -> console_lock
rather than fixing the opposite.
At initialization time,
the sclp driver registers an inappropriate dependency with lockdep. It
does this by calling into the page allocator while holding sclp_lock.
But we don't *want* to teach lockdep that sclp_lock nests outside
zone->lock. We want the opposite.
So can we address this class of problem by declaring "thou shalt not
call the page allocator while holding a lock which can be taken on the
prink path?". And then declare sclp to be defective.
And I think sclp is kinda buggy-but-lucky anyway: if console output is
directed to sclp device #0 and we're then trying to initialize sclp
device #1 then any printk which happens during that initialization will
deadlock. The driver escapes this by only supporting a single device
system-wide but it's not a model which drivers should generally follow.
(And if sclp will only ever support a single device system-wide, why
the heck does it need to take sclp_lock() on the device initialization
path??)
|
