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

[Dave Hansen]

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.