From owner-freebsd-arch@freebsd.org Wed Jul 8 06:13:08 2015 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 67FCD995C26 for ; Wed, 8 Jul 2015 06:13:08 +0000 (UTC) (envelope-from yaneurabeya@gmail.com) Received: from mail-pa0-x22f.google.com (mail-pa0-x22f.google.com [IPv6:2607:f8b0:400e:c03::22f]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 33E1D1AD4; Wed, 8 Jul 2015 06:13:08 +0000 (UTC) (envelope-from yaneurabeya@gmail.com) Received: by pacgz10 with SMTP id gz10so52329360pac.3; Tue, 07 Jul 2015 23:13:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:mime-version:content-type:from:in-reply-to:date:cc :message-id:references:to; bh=V4TqllXYjC426J5lqMhQ2dW9bc3wU/rY2WPajvzghis=; b=PHC5PawE6iLSdrmqapQ//hhIl1Vuk68YMfaJeyfCXOQcoDp2y1Vzk5aCHUYcnlygPn +aWaN8UadFiLaSQqIUmKigbW0iZ7eue9pirBhGu5uHNU7l4qi/ptaALhHa8yzgak+6Ci D0Xw+OmvPF8Q/pRwZeYEoDqZwdq2oO0Wpo9MkPImUgk4nVXqK3OWd2I1M2Gz6tmg/utL ggRcJ0jpovh4/CYxXVe5aIR8/yT8oIawR4hXJ88TBrqmoWRII64qBDRZTylO0ok3Kp8n Tl5upqNY1armpacvj1hlU8O1E6e9kokhV/7aDuvlIQCR+pVUawa/GecftUznC/mXPYON UFBw== X-Received: by 10.67.4.201 with SMTP id cg9mr17004751pad.53.1436335987528; Tue, 07 Jul 2015 23:13:07 -0700 (PDT) Received: from ?IPv6:2601:602:8001:6c87:6089:bf26:e463:3672? ([2601:602:8001:6c87:6089:bf26:e463:3672]) by smtp.gmail.com with ESMTPSA id r9sm393370pds.82.2015.07.07.23.13.06 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 07 Jul 2015 23:13:06 -0700 (PDT) Subject: Re: CFT/CFR: NUMA policy branch Mime-Version: 1.0 (Mac OS X Mail 7.3 \(1878.6\)) Content-Type: multipart/signed; boundary="Apple-Mail=_4C725582-4821-4CB1-93CB-7BA046F0986F"; protocol="application/pgp-signature"; micalg=pgp-sha512 X-Pgp-Agent: GPGMail 2.5 From: Garrett Cooper In-Reply-To: Date: Tue, 7 Jul 2015 23:13:09 -0700 Cc: "freebsd-arch@freebsd.org" Message-Id: References: To: Adrian Chadd X-Mailer: Apple Mail (2.1878.6) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Jul 2015 06:13:08 -0000 --Apple-Mail=_4C725582-4821-4CB1-93CB-7BA046F0986F Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=windows-1252 On Jul 5, 2015, at 19:06, Adrian Chadd wrote: > Hi, >=20 > I've done another update. kib@ has been beating me with the clue stick = a bit. >=20 > = https://github.com/freebsd/freebsd/compare/master...erikarn:local/adrian_n= uma_policy >=20 > * (kib) (numactl.c) fix up sorting of include files > * (kib) (numactl.c) consistent use of values when calling err() > * (kib) (numactl.c) consistently wrap lines at 78 characters, don't > prematurely wrap lines > * (kib) don't use the old-style BSD licence mentioning "regents", use > the updated one > * (kib) (vm_domain.c) don't break out after iterating a few times and > have the API be unpredictable - so now the API will always succeed in > reading a vm_policy >=20 > I've tested the policies (first-touch, fixed-domain, round-robin) and > they all still work as advertised, both on threads and processes. >=20 > I'd appreciate more reviews and some further testing. Please create a dummy pull request or post the code up on Phabricator. - Please put some of the items like policy_to_str and str_to_policy in a = library. - Please use that code in the kernel as well for = sysctl_vm_default_policy to reduce duplication. - Please note reasoning for why `options MAXMEMDOM=3D16` in numa(4). - Why are checking for `if (p)` before calling PROC_UNLOCK(p) in = sys_numa_getaffinity, but not sys_numa_setaffinity? - sys_numa_setaffinity and sys_numa_getaffinity look similar. Could = something be implemented like sysctl(3) for handling getting/setting of = affinities all in one shot? - `if (p)` should be `if (p !=3D NULL)`, etc per style(9). - Is there a way that the affinity could be inherited/not inherited = across threads, similar to what ktrace -i does? If so, how does one do = that? - In vm_domain_rr_selectdomain, should this use atomic(9), i.e. can = multiple threads access/mangle the value of td_dom_rr_idx in parallel? - In vm_domain_policy_validate, couldn=92t you remove all of the = intermediary `return (-1)`=92s as long as you put `break;`s in the = switch/case statements? - Should vm_domain_policy_cleanup/vm_domain_policy_init be implemented? = If so, what should they have in there? - Would it make sense for `struct vm_domain_iterator` to be a = queue(9)-like type (just based on the name alone)? If not, what data = structure do you anticipate it having, e.g. tree, queue, directed graph, = etc? - In vm_domain_iterator_run, vi->n is always decremented after the vi->n = <=3D 0 run is done =97 why not move that outside the switch-case = statement? - Why did you hand roll `vm_domain_iterator_isdone` in = `vm_domain_iterator_run` up at the top? - The parameter names for functions/syscalls can be omitted in the = declarations. - The change to vm_page.c seems like it could be committed separate from = the NUMA changes. - `However without it'll kernel panic below - the code didn=92t` -> = `However without the following check, the kernel will panic below; the = code didn=92t` - vm_domain_iterator_run seems like it could use one of the queue(9) = data structures. - Why is OPT_TID 1001? - `int error;` should come after `cpuset_t set;` per alignment and = style(9). - `atoi` parsing is better handled via strtoll, etc. Thanks! -NGie --Apple-Mail=_4C725582-4821-4CB1-93CB-7BA046F0986F Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=signature.asc Content-Type: application/pgp-signature; name=signature.asc Content-Description: Message signed with OpenPGP using GPGMail -----BEGIN PGP SIGNATURE----- Comment: GPGTools - https://gpgtools.org iQEcBAEBCgAGBQJVnL91AAoJEMZr5QU6S73eao8H/idxsdPeoqIHaHR7TaYtM/7o FiwKrWYusUuUDXQfEdM8EgMk1deCM2LIfBQl4gMx+QiSnZSOMmGWCUUr1csRx8GR mfYVJ9t6M+YMhX7nG7wNQA1RDRQIT3hxC8/VA7iRMfcHGXoglPljruxpFi+q17bH TLcW7uLTi00D/FDoCjdNPfjycFMpG6JpPnxa/QY1x6RdGJKLP5lkOWE5NfJ5st87 GIqipOJWyKq+LZNf75hI4KTmO6LwRWUKAV2/UG3IscdZoO037jyb5MNKgreDAwtq lP8tMk9WN02+90wchgZo2iw7WcOewr3Yc1uN3LP+ITqhUJY3a7xrpeXDfK7hVaY= =2v/h -----END PGP SIGNATURE----- --Apple-Mail=_4C725582-4821-4CB1-93CB-7BA046F0986F--