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

[Jarkko Sakkinen <jarkko@kernel.org>]

On Tue, Feb 23, 2021 at 11:14:10AM -0800, Dave Hansen wrote:
> 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.

I would start with what I have with minimal changes, and then continue
with a patch that completely wipes the section list.

Then fallback can just take a page from a flat FIFO of EPC pages.

/Jarkko