Messages in this thread | | | Subject | Re: [PATCH] io-wq: fix handling of NUMA node IDs | From | Jens Axboe <> | Date | Tue, 26 Nov 2019 11:17:06 -0700 |
| |
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@google.com> > --- > > 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.
-- Jens Axboe
| |