Skip to content

Commit fe4f794

Browse files
author
Andreas Gruenbacher
committed
gfs2: Fix asynchronous thread destruction
The kernel threads are currently stopped and destroyed synchronously by gfs2_make_fs_ro() and gfs2_put_super(), and asynchronously by signal_our_withdraw(), with no synchronization, so the synchronous and asynchronous contexts can race with each other. First, when creating the kernel threads, take an extra task struct reference so that the task struct won't go away immediately when they terminate. This allows those kthreads to terminate immediately when they're done rather than hanging around as zombies until they are reaped by kthread_stop(). When kthread_stop() is called on a terminated kthread, it will return immediately. Second, in signal_our_withdraw(), once the SDF_JOURNAL_LIVE flag has been cleared, wake up the logd and quotad wait queues instead of stopping the logd and quotad kthreads. The kthreads are then expected to terminate automatically within short time, but if they cannot, they will not block the withdraw. For example, if a user process and one of the kthread decide to withdraw at the same time, only one of them will perform the actual withdraw and the other will wait for it to be done. If the kthread ends up being the one to wait, the withdrawing user process won't be able to stop it. Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
1 parent f66af88 commit fe4f794

4 files changed

Lines changed: 32 additions & 33 deletions

File tree

fs/gfs2/ops_fstype.c

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,29 +1103,49 @@ static int init_threads(struct gfs2_sbd *sdp)
11031103
struct task_struct *p;
11041104
int error = 0;
11051105

1106-
p = kthread_run(gfs2_logd, sdp, "gfs2_logd");
1106+
p = kthread_create(gfs2_logd, sdp, "gfs2_logd");
11071107
if (IS_ERR(p)) {
11081108
error = PTR_ERR(p);
1109-
fs_err(sdp, "can't start logd thread: %d\n", error);
1109+
fs_err(sdp, "can't create logd thread: %d\n", error);
11101110
return error;
11111111
}
1112+
get_task_struct(p);
11121113
sdp->sd_logd_process = p;
11131114

1114-
p = kthread_run(gfs2_quotad, sdp, "gfs2_quotad");
1115+
p = kthread_create(gfs2_quotad, sdp, "gfs2_quotad");
11151116
if (IS_ERR(p)) {
11161117
error = PTR_ERR(p);
1117-
fs_err(sdp, "can't start quotad thread: %d\n", error);
1118+
fs_err(sdp, "can't create quotad thread: %d\n", error);
11181119
goto fail;
11191120
}
1121+
get_task_struct(p);
11201122
sdp->sd_quotad_process = p;
1123+
1124+
wake_up_process(sdp->sd_logd_process);
1125+
wake_up_process(sdp->sd_quotad_process);
11211126
return 0;
11221127

11231128
fail:
11241129
kthread_stop(sdp->sd_logd_process);
1130+
put_task_struct(sdp->sd_logd_process);
11251131
sdp->sd_logd_process = NULL;
11261132
return error;
11271133
}
11281134

1135+
void gfs2_destroy_threads(struct gfs2_sbd *sdp)
1136+
{
1137+
if (sdp->sd_logd_process) {
1138+
kthread_stop(sdp->sd_logd_process);
1139+
put_task_struct(sdp->sd_logd_process);
1140+
sdp->sd_logd_process = NULL;
1141+
}
1142+
if (sdp->sd_quotad_process) {
1143+
kthread_stop(sdp->sd_quotad_process);
1144+
put_task_struct(sdp->sd_quotad_process);
1145+
sdp->sd_quotad_process = NULL;
1146+
}
1147+
}
1148+
11291149
/**
11301150
* gfs2_fill_super - Read in superblock
11311151
* @sb: The VFS superblock
@@ -1276,12 +1296,7 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
12761296

12771297
if (error) {
12781298
gfs2_freeze_unlock(&sdp->sd_freeze_gh);
1279-
if (sdp->sd_quotad_process)
1280-
kthread_stop(sdp->sd_quotad_process);
1281-
sdp->sd_quotad_process = NULL;
1282-
if (sdp->sd_logd_process)
1283-
kthread_stop(sdp->sd_logd_process);
1284-
sdp->sd_logd_process = NULL;
1299+
gfs2_destroy_threads(sdp);
12851300
fs_err(sdp, "can't make FS RW: %d\n", error);
12861301
goto fail_per_node;
12871302
}

fs/gfs2/super.c

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -549,17 +549,7 @@ void gfs2_make_fs_ro(struct gfs2_sbd *sdp)
549549
if (!test_bit(SDF_KILL, &sdp->sd_flags))
550550
gfs2_flush_delete_work(sdp);
551551

552-
if (!log_write_allowed && current == sdp->sd_quotad_process)
553-
fs_warn(sdp, "The quotad daemon is withdrawing.\n");
554-
else if (sdp->sd_quotad_process)
555-
kthread_stop(sdp->sd_quotad_process);
556-
sdp->sd_quotad_process = NULL;
557-
558-
if (!log_write_allowed && current == sdp->sd_logd_process)
559-
fs_warn(sdp, "The logd daemon is withdrawing.\n");
560-
else if (sdp->sd_logd_process)
561-
kthread_stop(sdp->sd_logd_process);
562-
sdp->sd_logd_process = NULL;
552+
gfs2_destroy_threads(sdp);
563553

564554
if (log_write_allowed) {
565555
gfs2_quota_sync(sdp->sd_vfs, 0);
@@ -615,8 +605,10 @@ static void gfs2_put_super(struct super_block *sb)
615605
if (!sb_rdonly(sb)) {
616606
gfs2_make_fs_ro(sdp);
617607
}
618-
if (gfs2_withdrawn(sdp))
608+
if (gfs2_withdrawn(sdp)) {
609+
gfs2_destroy_threads(sdp);
619610
gfs2_quota_cleanup(sdp);
611+
}
620612
WARN_ON(gfs2_withdrawing(sdp));
621613

622614
/* At this point, we're through modifying the disk */

fs/gfs2/super.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ extern int gfs2_lookup_in_master_dir(struct gfs2_sbd *sdp, char *filename,
3636
extern int gfs2_make_fs_rw(struct gfs2_sbd *sdp);
3737
extern void gfs2_make_fs_ro(struct gfs2_sbd *sdp);
3838
extern void gfs2_online_uevent(struct gfs2_sbd *sdp);
39+
extern void gfs2_destroy_threads(struct gfs2_sbd *sdp);
3940
extern int gfs2_statfs_init(struct gfs2_sbd *sdp);
4041
extern void gfs2_statfs_change(struct gfs2_sbd *sdp, s64 total, s64 free,
4142
s64 dinodes);

fs/gfs2/util.c

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -151,17 +151,8 @@ static void signal_our_withdraw(struct gfs2_sbd *sdp)
151151
if (!sb_rdonly(sdp->sd_vfs)) {
152152
bool locked = mutex_trylock(&sdp->sd_freeze_mutex);
153153

154-
if (sdp->sd_quotad_process &&
155-
current != sdp->sd_quotad_process) {
156-
kthread_stop(sdp->sd_quotad_process);
157-
sdp->sd_quotad_process = NULL;
158-
}
159-
160-
if (sdp->sd_logd_process &&
161-
current != sdp->sd_logd_process) {
162-
kthread_stop(sdp->sd_logd_process);
163-
sdp->sd_logd_process = NULL;
164-
}
154+
wake_up(&sdp->sd_logd_waitq);
155+
wake_up(&sdp->sd_quota_wait);
165156

166157
wait_event_timeout(sdp->sd_log_waitq,
167158
gfs2_log_is_empty(sdp),

0 commit comments

Comments
 (0)