Messages in this thread | | | Date | Thu, 25 Oct 2018 16:21:25 -0400 | From | "Theodore Y. Ts'o" <> | Subject | Re: [PATCH 3/3] kernel/workqueue: Suppress a false positive lockdep complaint |
| |
On Thu, Oct 25, 2018 at 09:59:38PM +0200, Johannes Berg wrote: > > So, thinking about this more, can you guarantee (somehow) that the > > workqueue is empty at this point? > > (I hadn't looked at the code then - obviously that's guaranteed)
We can guarantee it from someone who is looking at the code path. In dio_set_defer_completion:
if (!sb->s_dio_done_wq) return sb_init_dio_done_wq(sb);
And then sb_init_dio_done_wq:
int sb_init_dio_done_wq(struct super_block *sb) { struct workqueue_struct *old; struct workqueue_struct *wq = alloc_workqueue("dio/%s", WQ_MEM_RECLAIM, 0, sb->s_id); if (!wq) return -ENOMEM; /* * This has to be atomic as more DIOs can race to create the workqueue */ old = cmpxchg(&sb->s_dio_done_wq, NULL, wq); /* Someone created workqueue before us? Free ours... */ if (old) destroy_workqueue(wq); return 0; }
The race found in the syzbot reproducer has multiple threads all running DIO writes at the same time. So we have multiple threads calling sb_init_dio_done_wq, but all but one will lose the race, and then call destry_workqueue on the freshly created (but never used) workqueue.
We could replace the destroy_workqueue(wq) with a "I_solemnly_swear_this_workqueue_has_never_been_used_please_destroy(wq)".
Or, as Tejun suggested, "destroy_workqueue_skip_drain(wq)", but there is no way for the workqueue code to know whether the caller was using the interface correctly. So this basically becomes a philosophical question about whether or not we trust the caller to be correct or not.
I don't see an obvious way that we can test to make sure the workqueue is never used without actually taking a performance. Am I correct that we would need to take the wq->mutex before we can mess with the wq->flags field?
- Ted
| |