svn commit: r272800 - head/sys/x86/acpica

[John Baldwin]

On Thursday, October 09, 2014 2:23:10 pm Konstantin Belousov wrote:
> On Thu, Oct 09, 2014 at 05:34:29AM +0000, Adrian Chadd wrote:
> > Author: adrian
> > Date: Thu Oct  9 05:34:28 2014
> > New Revision: 272800
> > URL: https://svnweb.freebsd.org/changeset/base/272800
> > 
> > Log:
> >   Missing from previous commit - keep the VM domain -> PXM mapping
> >   array and use it to map PXM -> VM domain when needed.
> >   
> >   Differential Revision:	D906
> >   Reviewed by:	jhb
> > 
> > Modified:
> >   head/sys/x86/acpica/srat.c
> > 
> > Modified: head/sys/x86/acpica/srat.c
> > 
==============================================================================
> > --- head/sys/x86/acpica/srat.c	Thu Oct  9 05:33:25 2014	(r272799)
> > +++ head/sys/x86/acpica/srat.c	Thu Oct  9 05:34:28 2014	(r272800)
> > @@ -62,6 +62,8 @@ int num_mem;
> >  static ACPI_TABLE_SRAT *srat;
> >  static vm_paddr_t srat_physaddr;
> >  
> > +static int vm_domains[VM_PHYSSEG_MAX];
> > +
> >  static void	srat_walk_table(acpi_subtable_handler *handler, void 
*arg);
> >  
> >  /*
> > @@ -247,7 +249,6 @@ check_phys_avail(void)
> >  static int
> >  renumber_domains(void)
> >  {
> > -	int domains[VM_PHYSSEG_MAX];
> >  	int i, j, slot;
> >  
> >  	/* Enumerate all the domains. */
> > @@ -255,17 +256,17 @@ renumber_domains(void)
> >  	for (i = 0; i < num_mem; i++) {
> >  		/* See if this domain is already known. */
> >  		for (j = 0; j < vm_ndomains; j++) {
> > -			if (domains[j] >= mem_info[i].domain)
> > +			if (vm_domains[j] >= mem_info[i].domain)
> >  				break;
> >  		}
> > -		if (j < vm_ndomains && domains[j] == mem_info[i].domain)
> > +		if (j < vm_ndomains && vm_domains[j] == mem_info[i].domain)
> >  			continue;
> >  
> >  		/* Insert the new domain at slot 'j'. */
> >  		slot = j;
> >  		for (j = vm_ndomains; j > slot; j--)
> > -			domains[j] = domains[j - 1];
> > -		domains[slot] = mem_info[i].domain;
> > +			vm_domains[j] = vm_domains[j - 1];
> > +		vm_domains[slot] = mem_info[i].domain;
> >  		vm_ndomains++;
> >  		if (vm_ndomains > MAXMEMDOM) {
> >  			vm_ndomains = 1;
> > @@ -280,15 +281,15 @@ renumber_domains(void)
> >  		 * If the domain is already the right value, no need
> >  		 * to renumber.
> >  		 */
> > -		if (domains[i] == i)
> > +		if (vm_domains[i] == i)
> >  			continue;
> >  
> >  		/* Walk the cpu[] and mem_info[] arrays to renumber. */
> >  		for (j = 0; j < num_mem; j++)
> > -			if (mem_info[j].domain == domains[i])
> > +			if (mem_info[j].domain == vm_domains[i])
> >  				mem_info[j].domain = i;
> >  		for (j = 0; j <= MAX_APIC_ID; j++)
> > -			if (cpus[j].enabled && cpus[j].domain == domains[i])
> > +			if (cpus[j].enabled && cpus[j].domain == vm_domains[i])
> >  				cpus[j].domain = i;
> >  	}
> >  	KASSERT(vm_ndomains > 0,
> > @@ -368,4 +369,23 @@ srat_set_cpus(void *dummy)
> >  	}
> >  }
> >  SYSINIT(srat_set_cpus, SI_SUB_CPU, SI_ORDER_ANY, srat_set_cpus, NULL);
> > +
> > +/*
> > + * Map a _PXM value to a VM domain ID.
> > + *
> > + * Returns the domain ID, or -1 if no domain ID was found.
> > + */
> > +int
> > +acpi_map_pxm_to_vm_domainid(int pxm)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < vm_ndomains; i++) {
> > +		if (vm_domains[i] == pxm)
> > +			return (i);
> > +	}
> > +
> > +	return (-1);
> > +}
> > +
> >  #endif /* MAXMEMDOM > 1 */
> 
> I do not like it.  Sorry for not looking at the web thing, I have very
> little time.
> 
> It never was an intention that one proximity domain reported by ACPI
> was mapped to single VM domain.  VM could split domains (in terms of
> vm_domains) further for other reasons.  Main motivation is that there
> is 1:1 relations between domain/page queues/page queues locks/pagedaemons.
> 
> I have patches in WIP stage which split firmware proximity domains
> further, to decrease congestion on the page queue locks.  I wrote about
> this in the pgsql performance report.
> 
> The short version is that there is/will be N:1 relation between VM domains
> and proximity domains (which is reported by ACPI for devices).

_PXM is also defined to be what SRAT reports for memory, and is what will be 
used to do NUMA-aware memory allocations.  While the VM system may decide to 
split a given NUMA domain into multiple some-other-things, those some-other-
things won't be a NUMA domain anymore.  At that point, you will need to 
divorce them from 'domain' and use another term as the domain index into 
mem_info[] will still be needed so that NUMA allocations do the correct thing.

-- 
John Baldwin