On Thu, 16 Feb 2012 09:41:34 -0500 (EST) Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > Folks: > > I recently uncovered a bug in the block layer. It uses a workqueue to > periodically probe removable drives for media or other state changes, > and the workqueue it uses is system_nrt_wq. > > The bug is that system_nrt_wq is not freezable, so it keeps on running > even while the system is in the process of suspending or hibernating. > Doing I/O to a suspended drive doesn't work well and in some cases > causes nasty problems. Obviously these polls need to stop during a > suspend transition. > > A search through the kernel shows that system_nrt_wq is also used in a > few other subsystems: > > ./fs/cifs/cifssmb.c: queue_work(system_nrt_wq, &rdata->work); > ./fs/cifs/cifssmb.c: queue_work(system_nrt_wq, &wdata->work); > ./fs/cifs/misc.c: queue_work(system_nrt_wq, > ./fs/cifs/connect.c: queue_delayed_work(system_nrt_wq, &server->echo, SMB_ECHO_INTERVAL); > ./fs/cifs/connect.c: queue_delayed_work(system_nrt_wq, &tcp_ses->echo, SMB_ECHO_INTERVAL); > ./fs/cifs/connect.c: queue_delayed_work(system_nrt_wq, &cifs_sb->prune_tlinks, > ./fs/cifs/connect.c: queue_delayed_work(system_nrt_wq, &cifs_sb->prune_tlinks, > These should all be freezable and we might even be able to get away with WQ_UNBOUND for some of these. I think we put most of these in system_nrt_wq because Tejun put an earlier job into that queue when he converted it from slow_work and we just cargo-cult copied that... I'll spend some time looking at this in the next day or two, but I suspect that the right answer is to just move these off of the "public" workqueues altogether. > ./drivers/mmc/core/host.c: queue_work(system_nrt_wq, &host->clk_gate_work); > > ./drivers/gpu/drm/drm_crtc_helper.c: queue_delayed_work(system_nrt_wq, delayed_work, DRM_OUTPUT_POLL_PERIOD); > ./drivers/gpu/drm/drm_crtc_helper.c: queue_delayed_work(system_nrt_wq, &dev->mode_config.output_poll_work, DRM_OUTPUT_POLL_PERIOD); > ./drivers/gpu/drm/drm_crtc_helper.c: queue_delayed_work(system_nrt_wq, &dev->mode_config.output_poll_work, 0); > > ./security/keys/gc.c: queue_work(system_nrt_wq, &key_gc_work); > ./security/keys/gc.c: queue_work(system_nrt_wq, &key_gc_work); > ./security/keys/gc.c: queue_work(system_nrt_wq, &key_gc_work); > ./security/keys/gc.c: queue_work(system_nrt_wq, &key_gc_work); > ./security/keys/key.c: queue_work(system_nrt_wq, &key_gc_work); > > My question to all of you: Should system_nrt_wq be made freezable, or > should I create a new workqueue that is both freezable and > non-reentrant? And if I do, which of the usages above should be > converted to the new workqueue? > > Thanks, > > Alan Stern > > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Jeff Layton <jlayton@xxxxxxxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
