From owner-svn-src-user@FreeBSD.ORG Mon May 13 18:55:00 2013 Return-Path: Delivered-To: svn-src-user@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 894E4ECF; Mon, 13 May 2013 18:55:00 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from mail-ie0-x236.google.com (mail-ie0-x236.google.com [IPv6:2607:f8b0:4001:c03::236]) by mx1.freebsd.org (Postfix) with ESMTP id 4D5111FB; Mon, 13 May 2013 18:55:00 +0000 (UTC) Received: by mail-ie0-f182.google.com with SMTP id a14so13070271iee.41 for ; Mon, 13 May 2013 11:55:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:reply-to:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; bh=higxCAUwoV+CQKyUpr0Sq/G/WKV1dUbYg/+hHUDHFto=; b=qogqWHU32Jbb6mx3V3g485s26NvfnTm5SjjoXz5Y4HzaB4PekVowsVuqklNlAo0hYj jC+YdANXWE2eDfj2jBGkaABoKX4aWBI1bFCts8R4qNwfHJIDRphOmubWuHSH2eEqb9yT cDQKEf1mQ6AztI3x4QAvKAy6mJZ8FcO5anxKi05+3RUo4tckcX4N1iWeYGJmy/pCaqJW XQuvxMmzYgnuEInzDQlDaPUtRx57YeOWRCeykzR7SMGyaQbWkAY8HsydVIi6w1eiavaG 91ZH9b1juOQzF8AmzRbGopnsWlGPPBUB1les8C0duHI2xeseYarvOoJXSHJ+Emcoq0IY ZgFA== MIME-Version: 1.0 X-Received: by 10.43.138.196 with SMTP id it4mr5268123icc.3.1368471299985; Mon, 13 May 2013 11:54:59 -0700 (PDT) Sender: asmrookie@gmail.com Received: by 10.42.117.134 with HTTP; Mon, 13 May 2013 11:54:59 -0700 (PDT) In-Reply-To: <51913443.1060600@FreeBSD.org> References: <201305120210.r4C2AFpt076600@svn.freebsd.org> <51912863.6010601@FreeBSD.org> <51913443.1060600@FreeBSD.org> Date: Mon, 13 May 2013 20:54:59 +0200 X-Google-Sender-Auth: wY-hDQBPkH4uka5c4B6w3dTpwmM Message-ID: Subject: Re: svn commit: r250546 - user/attilio/jeff-numa/sys/x86/acpica From: Attilio Rao To: John Baldwin Content-Type: text/plain; charset=UTF-8 Cc: src-committers@freebsd.org, svn-src-user@freebsd.org X-BeenThere: svn-src-user@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: attilio@FreeBSD.org List-Id: "SVN commit messages for the experimental " user" src tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 13 May 2013 18:55:00 -0000 On Mon, May 13, 2013 at 8:43 PM, John Baldwin wrote: > On 5/13/13 2:21 PM, Attilio Rao wrote: >> On Mon, May 13, 2013 at 7:52 PM, John Baldwin wrote: >>> On 5/11/13 10:10 PM, Attilio Rao wrote: >>>> Author: attilio >>>> Date: Sun May 12 02:10:15 2013 >>>> New Revision: 250546 >>>> URL: http://svnweb.freebsd.org/changeset/base/250546 >>>> >>>> Log: >>>> Add the code to setup the correct number of probed memory domains. >>>> >>>> Sponsored by: EMC / Isilon storage division >>>> Obtained from: jeff >>>> >>>> Modified: >>>> user/attilio/jeff-numa/sys/x86/acpica/srat.c >>>> >>>> Modified: user/attilio/jeff-numa/sys/x86/acpica/srat.c >>>> ============================================================================== >>>> --- user/attilio/jeff-numa/sys/x86/acpica/srat.c Sun May 12 01:58:04 2013 (r250545) >>>> +++ user/attilio/jeff-numa/sys/x86/acpica/srat.c Sun May 12 02:10:15 2013 (r250546) >>>> @@ -244,33 +244,34 @@ static int >>>> renumber_domains(void) >>>> { >>>> int domains[VM_PHYSSEG_MAX]; >>>> - int ndomain, i, j, slot; >>>> + int i, j, slot; >>>> >>>> /* Enumerate all the domains. */ >>>> - ndomain = 0; >>>> + vm_ndomain = 0; >>>> for (i = 0; i < num_mem; i++) { >>>> /* See if this domain is already known. */ >>>> - for (j = 0; j < ndomain; j++) { >>>> + for (j = 0; j < vm_ndomain; j++) { >>>> if (domains[j] >= mem_info[i].domain) >>>> break; >>>> } >>>> - if (j < ndomain && domains[j] == mem_info[i].domain) >>>> + if (j < vm_ndomain && domains[j] == mem_info[i].domain) >>>> continue; >>>> >>>> /* Insert the new domain at slot 'j'. */ >>>> slot = j; >>>> - for (j = ndomain; j > slot; j--) >>>> + for (j = vm_ndomain; j > slot; j--) >>>> domains[j] = domains[j - 1]; >>>> domains[slot] = mem_info[i].domain; >>>> - ndomain++; >>>> - if (ndomain > MAXMEMDOM) { >>>> + vm_ndomain++; >>>> + if (vm_ndomain > MAXMEMDOM) { >>>> + vm_ndomain = 1; >>>> printf("SRAT: Too many memory domains\n"); >>>> return (EFBIG); >>>> } >>>> } >>>> >>>> /* Renumber each domain to its index in the sorted 'domains' list. */ >>>> - for (i = 0; i < ndomain; i++) { >>>> + for (i = 0; i < vm_ndomain; i++) { >>>> /* >>>> * If the domain is already the right value, no need >>>> * to renumber. >>>> >>> >>> Why not just set vm_ndomain = ndomain at the end? The current code aims >>> to only set the global variables if it successfully parses the entire >>> table. Setting vm_ndomain at the end would be consistent with this >>> model (and a much smaller diff). >> >> The provided diff is consistent. >> It does reset to 1 (initial value) in case of error, otherwise it is >> just the correct value (see the KASSERT in the end). > > That is not what I was saying. The previous code used a temporary > variable while it built the affinity info and only set 'mem_affinity' at > the end. It never had to back out and clear it back to NULL if there > was an error (which is more error prone). Following the same model here > would mean that you would leave vm_ndomains alone and only set it at the > very end when mem_affinity is set after all checks have passed. I understand what you are saying, and I'm replying that your objection is nosense because the effect will be just the same. Are you seriously thinking that this version is less readable? Attilio -- Peace can only be achieved by understanding - A. Einstein