ext4: fix panic on module unload when stopping lazyinit thread
PATCH 1/1 ext4: fix panic on module unload when stopping lazyinit thread
This is for the latest reported module unload oops in kernel.org bugzilla #27652
If the lazyinit thread is running, the teardown function ext4_destroy_lazyinit_thread() has problems:
ext4_clear_request_list();
while (ext4_li_info->li_task) {
wake_up(&ext4_li_info->li_wait_daemon);
wait_event(ext4_li_info->li_wait_task,
ext4_li_info->li_task == NULL);
}clearing the request list will cause the thread to exit and free ext4_li_info, so then we're waiting on something which is getting freed.
Fix this up by making the thread respond to kthread_stop, and exit, without the need to wait for that exit in some other homegrown way.
---
fs/ext4/super.c +13 -7
@@ -77,6 +77,7 @@ static struct dentry *ext4_mount(struct const char *dev_name, void *data);static void ext4_destroy_lazyinit_thread (void );static void ext4_unregister_li_request (struct super_block *sb); +static void ext4_clear_request_list (void );#if !defined(CONFIG_EXT3_FS ) && !defined(CONFIG_EXT3_FS_MODULE ) && defined(CONFIG_EXT4_USE_FOR_EXT23 ) static struct file_system_type ext3_fs_type = {
@@ -2704,6 +2705,8 @@ static void ext4_unregister_li_request(smutex_unlock (&ext4_li_info->li_list_mtx); } +static struct task_struct *ext4_lazyinit_task; +/* * This is the function where ext4lazyinit thread lives. It walks * through the request list searching for next scheduled filesystem.
@@ -2772,6 +2775,10 @@ cont_thread:if (time_before (jiffies, next_wakeup))schedule ();finish_wait (&eli->li_wait_daemon, &wait); +if (kthread_should_stop ()) { +ext4_clear_request_list (); + goto exit_thread; + } } exit_thread:
@@ -2796,6 +2803,7 @@ exit_thread:wake_up (&eli->li_wait_task);kfree (ext4_li_info); + ext4_lazyinit_task =NULL ; ext4_li_info =NULL ;mutex_unlock (&ext4_li_mtx);
@@ -2818,11 +2826,10 @@ static void ext4_clear_request_list(voidstatic int ext4_run_lazyinit_thread (void ) { -struct task_struct *t; - - t =kthread_run (ext4_lazyinit_thread, ext4_li_info,"ext4lazyinit" ); -if (IS_ERR (t)) { -int err =PTR_ERR (t); + ext4_lazyinit_task =kthread_run (ext4_lazyinit_thread, + ext4_li_info,"ext4lazyinit" ); +if (IS_ERR (ext4_lazyinit_task)) { +int err =PTR_ERR (ext4_lazyinit_task);ext4_clear_request_list ();del_timer_sync (&ext4_li_info->li_timer);kfree (ext4_li_info);
@@ -2973,16 +2980,10 @@ static void ext4_destroy_lazyinit_thread* If thread exited earlier * there's nothing to be done. */ -if (!ext4_li_info) +if (!ext4_li_info || !ext4_lazyinit_task)return ; -ext4_clear_request_list ();
Quoted 17 lines
This is for the latest reported module unload oops in kernel.org bugzilla #27652
If the lazyinit thread is running, the teardown function
ext4_destroy_lazyinit_thread()has problems:ext4_clear_request_list(); while (ext4_li_info->li_task) { wake_up(&ext4_li_info->li_wait_daemon); wait_event(ext4_li_info->li_wait_task, ext4_li_info->li_task == NULL); }clearing the request list will cause the thread to exit and free
ext4_li_info, so then we're waiting on something which is getting freed.
Quoted 5 lines
Fix this up by making the thread respond to
kthread_stop, and exit, without the need to wait for that exit in some other homegrown way.Signed-off-by Eric Sandeen <[email protected]>
It works now. Thanks.
Quoted 85 lines
---
Index:
linux-2.6/fs/ext4/super.c
===================================================================fs/ext4/super.c +1
-0@@ -77,6 +77,7 @@ static struct dentry *ext4_mount(struct const char *dev_name, void *data);static void ext4_destroy_lazyinit_thread (void );static void ext4_unregister_li_request (struct super_block *sb); +static void ext4_clear_request_list (void );#if !defined(CONFIG_EXT3_FS ) && !defined(CONFIG_EXT3_FS_MODULE ) && defined(CONFIG_EXT4_USE_FOR_EXT23 )static struct
file_system_typeext3_fs_type= {fs/ext4/super.c +13
-13@@ -2704,6 +2705,8 @@ static void ext4_unregister_li_request(smutex_unlock (&ext4_li_info->li_list_mtx); } +static struct task_struct *ext4_lazyinit_task; +/* * This is the function where ext4lazyinit thread lives. It walks * through the request list searching for next scheduled filesystem. @@ -2772,6 +2775,10 @@ cont_thread:if (time_before (jiffies, next_wakeup))schedule ();finish_wait (&eli->li_wait_daemon, &wait); +if (kthread_should_stop ()) { +ext4_clear_request_list (); + goto exit_thread; + } } exit_thread:@@ -2796,6 +2803,7 @@ exit_thread:wake_up (&eli->li_wait_task);kfree (ext4_li_info); + ext4_lazyinit_task =NULL ; ext4_li_info =NULL ;mutex_unlock (&ext4_li_mtx);@@ -2818,11 +2826,10 @@ static void ext4_clear_request_list(voidstatic int ext4_run_lazyinit_thread (void ) { -struct task_struct *t; - - t =kthread_run (ext4_lazyinit_thread, ext4_li_info,"ext4lazyinit" ); -if (IS_ERR (t)) { -int err =PTR_ERR (t); + ext4_lazyinit_task =kthread_run (ext4_lazyinit_thread, + ext4_li_info,"ext4lazyinit" ); +if (IS_ERR (ext4_lazyinit_task)) { +int err =PTR_ERR (ext4_lazyinit_task);ext4_clear_request_list ();del_timer_sync (&ext4_li_info->li_timer);kfree (ext4_li_info);@@ -2973,16 +2980,10 @@ static void ext4_destroy_lazyinit_thread* If thread exited earlier * there's nothing to be done. */ -if (!ext4_li_info) +if (!ext4_li_info || !ext4_lazyinit_task)return ; -ext4_clear_request_list (); - -while (ext4_li_info->li_task) { -wake_up (&ext4_li_info->li_wait_daemon); -wait_event (ext4_li_info->li_wait_task, - ext4_li_info->li_task ==NULL ); - } +kthread_stop (ext4_lazyinit_task); } static int ext4_fill_super(struct super_block *sb, void *data, intsilent)
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 13 lines · Eric Sandeen
This is for the latest reported module unload oops in kernel.org bugzilla #27652
If the lazyinit thread is running, the teardown function
ext4_destroy_lazyinit_thread()has problems:ext4_clear_request_list(); while (ext4_li_info->li_task) { wake_up(&ext4_li_info->li_wait_daemon); wait_event(ext4_li_info->li_wait_task, ext4_li_info->li_task == NULL); }
Quoted 7 lines
clearing the request list will cause the thread to exit and free
ext4_li_info, so then we're waiting on something which is getting freed.Fix this up by making the thread respond to
kthread_stop, and exit, without the need to wait for that exit in some other homegrown way.
It looks good. Thanks for covering that Eric.
-Lukas
Quoted 81 lines
Signed-off-by Eric Sandeen <[email protected]>---
Index:
linux-2.6/fs/ext4/super.c
===================================================================fs/ext4/super.c +14
-13@@ -77,6 +77,7 @@ static struct dentry *ext4_mount(struct const char *dev_name, void *data);static void ext4_destroy_lazyinit_thread (void );static void ext4_unregister_li_request (struct super_block *sb); +static void ext4_clear_request_list (void );#if !defined(CONFIG_EXT3_FS ) && !defined(CONFIG_EXT3_FS_MODULE ) && defined(CONFIG_EXT4_USE_FOR_EXT23 ) static struct file_system_type ext3_fs_type = {@@ -2704,6 +2705,8 @@ static void ext4_unregister_li_request(smutex_unlock (&ext4_li_info->li_list_mtx); } +static struct task_struct *ext4_lazyinit_task; +/* * This is the function where ext4lazyinit thread lives. It walks * through the request list searching for next scheduled filesystem. @@ -2772,6 +2775,10 @@ cont_thread:if (time_before (jiffies, next_wakeup))schedule ();finish_wait (&eli->li_wait_daemon, &wait); +if (kthread_should_stop ()) { +ext4_clear_request_list (); + goto exit_thread; + } } exit_thread:@@ -2796,6 +2803,7 @@ exit_thread:wake_up (&eli->li_wait_task);kfree (ext4_li_info); + ext4_lazyinit_task =NULL ; ext4_li_info =NULL ;mutex_unlock (&ext4_li_mtx);@@ -2818,11 +2826,10 @@ static void ext4_clear_request_list(voidstatic int ext4_run_lazyinit_thread (void ) { -struct task_struct *t; - - t =kthread_run (ext4_lazyinit_thread, ext4_li_info,"ext4lazyinit" ); -if (IS_ERR (t)) { -int err =PTR_ERR (t); + ext4_lazyinit_task =kthread_run (ext4_lazyinit_thread, + ext4_li_info,"ext4lazyinit" ); +if (IS_ERR (ext4_lazyinit_task)) { +int err =PTR_ERR (ext4_lazyinit_task);ext4_clear_request_list ();del_timer_sync (&ext4_li_info->li_timer);kfree (ext4_li_info);@@ -2973,16 +2980,10 @@ static void ext4_destroy_lazyinit_thread* If thread exited earlier * there's nothing to be done. */ -if (!ext4_li_info) +if (!ext4_li_info || !ext4_lazyinit_task)return ; -ext4_clear_request_list (); - -while (ext4_li_info->li_task) { -wake_up (&ext4_li_info->li_wait_daemon); -wait_event (ext4_li_info->li_wait_task, - ext4_li_info->li_task ==NULL ); - } +kthread_stop (ext4_lazyinit_task); }static int ext4_fill_super (struct super_block *sb,void *data,int silent)
Ted Ts'o
maintainerQuoted 2 lines · Eric Sandeen
This is for the latest reported module unload oops in kernel.org bugzilla #27652
Thanks, added to the ext4 patch queue.
- Ted