Re: [PATCH] x86/sgx: Add a basic NUMA allocation scheme to sgx_alloc_epc_page()

Dave Hansen
2021-02-23T19:15:21Z

On 2/21/21 4:54 PM, Dave Hansen wrote:
> Instead of having a for-each-section loop, I'd make it for-each-node ->
> for-each-section. Something like:
>
> for (i = 0; i < num_possible_nodes(); i++) {
> node = (numa_node_id() + i) % num_possible_nodes()
>
> if (!node_isset(nid, sgx_numa_mask))
> continue;
>
> list_for_each_entry(section, &sgx_numa_nodes[nid],
> section_list) {
> __sgx_alloc_epc_page_from_section(section)
> }
> }

OK, here's an almost completely fleshed-out loop:

page = NULL;
node = numa_node_id();
start_node = node;
while (1) {
list_for_each_entry(section, &sgx_numa_nodes[nid],
section_list) {
page = __sgx_alloc_epc(section);
if (page)
break;
}
if (page)
break;

/*
* EPC allocation failed on 'node'. Fall
* back with round-robin to other nodes with
* EPC:
*/
node = next_node_in(node, sgx_numa_mask);

/* Give up if allocation wraps back to the start: */
if (node == start_node)
break;
}

This will:
1. Always start close to the CPU that started the allocation
2. Always spread the allocations out among nodes evenly, never
concentrating allocations on node 0, for instance. (This could also
be node_random() and get a similar effect, but this probably has
slightly better default NUMA behavior).
3. Efficiently look among all nodes because of 'sgx_numa_mask'
4. Have no special case for the first allocation. All allocations will
be satisfied from this unified loop.
5. Compile down to no loop on CONFIG_NUMA=y systems.
6. Be guaranteed to make forward progress even if preempted and
numa_node_id() changes in the loop.

BTW, I think the name of __sgx_alloc_epc_page_from_section() can be
shortened down. It's passed a section and returns a page, so both of
those could be removed from the name.