Staging: zram: Turn lockdep off during zram_init_device()
PATCH 1/2 Staging: zram: Turn lockdep off during zram_init_device()
zram->init_lock can be hold over an allocation that may try to reclaim memory in zram_init_device(). The same lock can later be taken from a reclaim context in zram_make_request(), thus triggering a lockdep warning. However, memory can not be reclaimed to an uninitialized zram device. Therefore, this warning is a false positive.
To prevent the warning to occur, we turn lockdep off during while the device is initialized.
--- zram_drv.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
drivers/staging/zram/zram_drv.c +14 -0
@@ -637,10 +637,22 @@ int zram_init_device(struct zram *zram)int ret;size_t num_pages; +/* +* Here we hold init_lock over allocations that may try to reclaim +* memory. The same lock is also taken from a reclaim context in +* zram_make_request(). That end up in a lockdep warning. +* Moreover the table can be quite big an it wouldn't be reasonable to +* use kmalloc(GFP_NOFS,...) to allocate it. +* Fortunately, memory can not be reclaimed to an uninitialized zram +* device, so this warning is a false positive. Therefore, to prevent +* the warning, we turn lockdep off here. +*/ +lockdep_off ();down_write (&zram->init_lock);if (zram->init_done) {up_write (&zram->init_lock); +lockdep_on ();return 0 ; }
@@ -682,6 +694,7 @@ int zram_init_device(struct zram *zram) zram->init_done =1 ;up_write (&zram->init_lock); +lockdep_on ();pr_debug ("Initialization done!\n" );return 0 ;
@@ -692,6 +705,7 @@ fail_no_table: fail:__zram_reset_device (zram);up_write (&zram->init_lock); +lockdep_on ();pr_err ("Initialization failed: err=%d\n" , ret);return ret; }
PATCH 2/2 Staging: zram: Add a missing GFP_KERNEL specifier in zram_init_device()
The allocation of zram->compress_buffer is misssing a GFP_* specifier. This is equivalent to GFP_NOWAIT but it is more likely a omission. Since the allocation just above it uses GFP_KERNEL, there is no reason to use GFP_NOWAIT here. Therefore, add GFP_KERNEL.
--- zram_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/zram/zram_drv.c +2 -1
@@ -665,7 +665,8 @@ int zram_init_device(struct zram *zram) goto fail_no_table; } - zram->compress_buffer = (void *)__get_free_pages (__GFP_ZERO,1 ); + zram->compress_buffer = + (void *)__get_free_pages (GFP_KERNEL | __GFP_ZERO,1 );if (!zram->compress_buffer) {pr_err ("Error allocating compressor buffer space\n" ); ret = -ENOMEM ;
Quoted 7 lines · Jerome Marchand
zram->init_lockcan be hold over an allocation that may try to reclaim memory inzram_init_device(). The same lock can later be taken from a reclaim context inzram_make_request(), thus triggering a lockdep warning. However, memory can not be reclaimed to an uninitialized zram device. Therefore, this warning is a false positive.
To prevent the warning to occur, we turn lockdep off during while the device is initialized.
fuck no! why do you even remotely consider this a sane thing to do?
There's tons of lockdep annotations, try one of those. If you have, your changelog utterly fails to explain why non of those are suitable and how your problem is special.
Quoted 9 lines · Peter Zijlstra
Quoted 7 lines · Jerome Marchand
zram->init_lockcan be hold over an allocation that may try to reclaim memory inzram_init_device(). The same lock can later be taken from a reclaim context inzram_make_request(), thus triggering a lockdep warning. However, memory can not be reclaimed to an uninitialized zram device. Therefore, this warning is a false positive.
To prevent the warning to occur, we turn lockdep off during while the device is initialized.
Quoted 6 lines
fuck no! why do you even remotely consider this a sane thing to do?
There's tons of lockdep annotations, try one of those. If you have, your changelog utterly fails to explain why non of those are suitable and how your problem is special.
I must agree. Propsed patch is a hack, and actually can make more trouble to other subsystems. It is better to live with warning than perform temporary lockdep off/on.
Signature · 1 line
Witold Baryluk