On 11/26/19 11:17 AM, Jens Axboe wrote: > On 11/26/19 11:10 AM, Jann Horn wrote: >> There are several things that can go wrong in the current code on NUMA >> systems, especially if not all nodes are online all the time: >> >> - If the identifiers of the online nodes do not form a single contiguous >> block starting at zero, wq->wqes will be too small, and OOB memory >> accesses will occur e.g. in the loop in io_wq_create(). >> - If a node comes online between the call to num_online_nodes() and the >> for_each_node() loop in io_wq_create(), an OOB write will occur. >> - If a node comes online between io_wq_create() and io_wq_enqueue(), a >> lookup is performed for an element that doesn't exist, and an OOB read >> will probably occur. >> >> Fix it by: >> >> - using nr_node_ids instead of num_online_nodes() for the allocation size; >> nr_node_ids is calculated by setup_nr_node_ids() to be bigger than the >> highest node ID that could possibly come online at some point, even if >> those nodes' identifiers are not a contiguous block >> - creating workers for all possible CPUs, not just all online ones >> >> This is basically what the normal workqueue code also does, as far as I can >> tell. >> >> Signed-off-by: Jann Horn <jannh@xxxxxxxxxx> >> --- >> >> Notes: >> compile-tested only. >> >> While I think I probably got this stuff right, it might be good if >> someone more familiar with the NUMA logic could give an opinion on this. >> >> An alternative might be to only allocate workers for online nodes, but >> then we'd have to either fiddle together logic to create more workers >> on demand or punt requests on newly-onlined nodes over to older nodes. >> Both of those don't seem very nice to me. > > I don't think caring about not-online nodes in terms of savings is worth > the trouble. I'll run this through the regular testing I have with no > and 2 nodes, thanks. Tests fine for me in all configurations, applied. -- Jens Axboe
