From owner-freebsd-arch@freebsd.org Mon Jul 6 02:06:28 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 1B5D99FF2 for ; Mon, 6 Jul 2015 02:06:28 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: from mail-ig0-x235.google.com (mail-ig0-x235.google.com [IPv6:2607:f8b0:4001:c05::235]) (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 DC5FC1DAD for ; Mon, 6 Jul 2015 02:06:27 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: by igrv9 with SMTP id v9so144717982igr.1 for ; Sun, 05 Jul 2015 19:06:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:content-type; bh=MgXW751BNWdXhotYPui2BqIW7AWt6aNKyQf8ZNMqyus=; b=J2I48mUfuPbk33T6w7iQWpGjVCDsOAjitW+7EAy1EJOwhoR7U6XYI4CSS06/vkrTb/ E3fDR54WHSIlw4vwtuUnr2WdK2JHvrrYNjTA4+8Iw1f+TxDixW/LS6bz9ZR25e9rLmpj qI5c/kC27+ICwy4BX0EJfLFU/DnkcFIieH9kq3orqPqkG6sWOf3c/mWV1gOKMR5vQNup +NAJynTAl4CeQ4wYkMkTRlHLIOx3LkiV7R4f1weepTLGs0LOZfXPhxJo2dLbaMoIxqC/ 3b2aCD/5/4iu0iqE3OXybteN5iw1o279tXfD6t6BQXXratt1HA6TvhjPsdp2PqL5RjqD iruw== MIME-Version: 1.0 X-Received: by 10.50.111.167 with SMTP id ij7mr64607342igb.49.1436148387259; Sun, 05 Jul 2015 19:06:27 -0700 (PDT) Sender: adrian.chadd@gmail.com Received: by 10.36.38.133 with HTTP; Sun, 5 Jul 2015 19:06:27 -0700 (PDT) In-Reply-To: References: Date: Sun, 5 Jul 2015 19:06:27 -0700 X-Google-Sender-Auth: a17En5YrqDaxatb0mLHe3bEmcI4 Message-ID: Subject: Re: CFT/CFR: NUMA policy branch From: Adrian Chadd To: "freebsd-arch@freebsd.org" Content-Type: text/plain; charset=UTF-8 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: Mon, 06 Jul 2015 02:06:28 -0000 Hi, I've done another update. kib@ has been beating me with the clue stick a bit. https://github.com/freebsd/freebsd/compare/master...erikarn:local/adrian_numa_policy * (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 I've tested the policies (first-touch, fixed-domain, round-robin) and they all still work as advertised, both on threads and processes. I'd appreciate more reviews and some further testing. Thanks! -adrian From owner-freebsd-arch@freebsd.org Tue Jul 7 09:38:00 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 DC8954C87B for ; Tue, 7 Jul 2015 09:38:00 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 5498B1FEA; Tue, 7 Jul 2015 09:38:00 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id t679blsh066509 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Tue, 7 Jul 2015 12:37:47 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.9.2 kib.kiev.ua t679blsh066509 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id t679blPn066508; Tue, 7 Jul 2015 12:37:47 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Tue, 7 Jul 2015 12:37:47 +0300 From: Konstantin Belousov To: Adrian Chadd Cc: "freebsd-arch@freebsd.org" Subject: Re: CFT/CFR: NUMA policy branch Message-ID: <20150707093747.GE2080@kib.kiev.ua> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on tom.home 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: Tue, 07 Jul 2015 09:38:01 -0000 On Sun, Jul 05, 2015 at 07:06:27PM -0700, Adrian Chadd wrote: > Hi, > > I've done another update. kib@ has been beating me with the clue stick a bit. > > https://github.com/freebsd/freebsd/compare/master...erikarn:local/adrian_numa_policy > > * (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 > > I've tested the policies (first-touch, fixed-domain, round-robin) and > they all still work as advertised, both on threads and processes. > > I'd appreciate more reviews and some further testing. I did not found a fix for the wrong locking of seq_t. Am I reading sys_numa_getaffinity() right that it does copyout() while owning the process lock ? The things are still syscalls instead of procctl() commands. I did not read further, the patch is half-done at best. From owner-freebsd-arch@freebsd.org Tue Jul 7 16:37:58 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 B05D4996D9B for ; Tue, 7 Jul 2015 16:37:58 +0000 (UTC) (envelope-from jason.harmening@gmail.com) Received: from mail-la0-x22c.google.com (mail-la0-x22c.google.com [IPv6:2a00:1450:4010:c03::22c]) (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 5186716EF for ; Tue, 7 Jul 2015 16:37:58 +0000 (UTC) (envelope-from jason.harmening@gmail.com) Received: by lagc2 with SMTP id c2so200292609lag.3 for ; Tue, 07 Jul 2015 09:37:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:date:message-id:subject:from:to:content-type; bh=bFkfkGuPOwzYuCuhtUxsKdoAZdOCmM6i1s8mj0qRr6Q=; b=OUrLB13+fPVYqLIi/ufxmzCEOhfHyZq3M1wEQT79L8Oz3w8wU9LRemWxwW+dIvRHkB 7wZNxx9UzhxnY8gpzL0nfKJQnkRXcN4d8nJWwW28KLWFtH6zH6lCdMn8r3USNkuRXsdY WF3Qr+p5Mu/14k1t8BIdEUqoXijTzFflozxgAgkN8u7F3/ym850lgdUsWg7rmhbCP0Ff +9fn+Q5LElU2jk1nlvbt+nSFf+40RUhbd0ntz60v06Er4Q49KW550aeSSjKAcKOiu3OC p9u7PkWKilLvBSitsEXq72JLlvZ4abzSmt9HnI31VQ9jN61xqMZTS4jD/tfrhWdaiqwH DP8w== MIME-Version: 1.0 X-Received: by 10.112.137.164 with SMTP id qj4mr4759695lbb.105.1436287076019; Tue, 07 Jul 2015 09:37:56 -0700 (PDT) Received: by 10.112.42.162 with HTTP; Tue, 7 Jul 2015 09:37:55 -0700 (PDT) Date: Tue, 7 Jul 2015 11:37:55 -0500 Message-ID: Subject: RFC: New KPI for fast temporary single-page KVA mappings From: Jason Harmening To: FreeBSD Arch Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.20 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: Tue, 07 Jul 2015 16:37:58 -0000 Hi everyone, I'd like to propose a couple of new pmap functions: vm_offset_t pmap_quick_enter_page(vm_page_t m) void pmap_quick_remove_page(vm_offset_t kva) These functions will create and destroy a temporary, usually CPU-local mapping of the specified page. Where available, they will use the direct map. Otherwise, they will use a per-CPU pageframe that's allocated at boot. Guarantees: --Will not sleep --Will not fail --Safe to call under a non-spin lock or from an ithread Restrictions: --Not safe to call from interrupt filter or under a spin mutex on all arches --Mappings should be held for as little time as possible; don't do any locking or sleeping while holding a mapping --Current implementation only guarantees a single page of mapping space across all arches. MI code should not make nested calls to pmap_quick_enter_page(). My idea is that the first consumer of this would be busdma. All non-iommu implementations would use this for bounce buffer copies of pages that don't have resident mappings. Currently busdma uses physcopy[in|out] for unmapped buffers, which on most arches uses sf_bufs that can sleep, making bus_dmamap_sync() unsafe to call in a lot of cases. busdma would also use this for virtually-indexed cache maintenance on arm and mips. It currently ignores cache maintenance for buffers that don't have a KVA or resident UVA mapping, which may not be correct for buffers that don't belong to curproc or have cache-resident VAs on other cores. I've created 2 Differential reviews: https://reviews.freebsd.org/D3013: the implementation https://reviews.freebsd.org/D3014: the kmod I've been using to test it I'd like any and all feedback, both on the general approach and the implementation details. Some things to note on the implementation: --I've intentionally avoided touching existing pmap code for the time being. Some of the new code could likely be shared with other pmap KPIs in a lot of cases. --I've structured the KPI to make it easy to extend to guarantee more than one per-CPU page in the future. I could see that being useful for copying between pages, for example --There's no immediate consumer for the sparc64 implementation, since busdma there needs neither bounce buffers nor cache maintenance. --I would very much like feedback and testing from experts on non-x86 arches. I only have hardware to test the i386 and amd64 implementations; I've only cross-compiled it for everything else. Some of the non-x86 details, like the Book E powerpc TLB invalidation code, are a bit scary and probably not quite right. Thanks, Jason From owner-freebsd-arch@freebsd.org Tue Jul 7 22:53:19 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 89D29996AC6 for ; Tue, 7 Jul 2015 22:53:19 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: from mail-ie0-x232.google.com (mail-ie0-x232.google.com [IPv6:2607:f8b0:4001:c03::232]) (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 52B85162D for ; Tue, 7 Jul 2015 22:53:19 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: by iebmu5 with SMTP id mu5so144248903ieb.1 for ; Tue, 07 Jul 2015 15:53:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=BRnnipCwY5jpJuADGfoG3Voa2uSOSF4NX7esZMWCTgw=; b=xpxsdZmHtStdxv/MyGIefhh4zxNUHEtpXdxhSur5pP8b5Ga+xMidKjZdyZJptmJkrq 3hndiEet1X9WFsdjSaZ+iEAZu3BlyxFLFUnO2DqmbCETPCGwvtCwvbwJ2fFxI3mBprcQ qXBJq82CPXMjKktYrckRCDBn6v6lnct6MDlYN00dNhyYLLAuqrt5ye9z4ZJwmK7Ae21V iZT01K6eRM1wo6ZGiAbV7R7yddlFDIvU19b2ZwDMCM5JC6ni9BywHeTICF0zXe0PnfNC YucghI5075VRzyRXtUzzgLrmXpES9n6OgdLmpYqT25J+SWWSj8jU8gKFZk+r+xhpEk4c z2vw== MIME-Version: 1.0 X-Received: by 10.107.5.1 with SMTP id 1mr10483875iof.88.1436309598669; Tue, 07 Jul 2015 15:53:18 -0700 (PDT) Sender: adrian.chadd@gmail.com Received: by 10.36.38.133 with HTTP; Tue, 7 Jul 2015 15:53:18 -0700 (PDT) In-Reply-To: <20150707093747.GE2080@kib.kiev.ua> References: <20150707093747.GE2080@kib.kiev.ua> Date: Tue, 7 Jul 2015 15:53:18 -0700 X-Google-Sender-Auth: lmbB_PhaYq5ajSWIxJdoTEurAb0 Message-ID: Subject: Re: CFT/CFR: NUMA policy branch From: Adrian Chadd To: Konstantin Belousov Cc: "freebsd-arch@freebsd.org" Content-Type: text/plain; charset=UTF-8 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: Tue, 07 Jul 2015 22:53:19 -0000 On 7 July 2015 at 02:37, Konstantin Belousov wrote: > On Sun, Jul 05, 2015 at 07:06:27PM -0700, Adrian Chadd wrote: >> Hi, >> >> I've done another update. kib@ has been beating me with the clue stick a bit. >> >> https://github.com/freebsd/freebsd/compare/master...erikarn:local/adrian_numa_policy >> >> * (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 >> >> I've tested the policies (first-touch, fixed-domain, round-robin) and >> they all still work as advertised, both on threads and processes. >> >> I'd appreciate more reviews and some further testing. > > I did not found a fix for the wrong locking of seq_t. Which locking is wrong? * the global policy is locked via the global mutex; * the process/thread policy is locked via PROC_LOCK. What did I miss? > Am I reading sys_numa_getaffinity() right that it does copyout() while > owning the process lock ? I've fixed and push that, thanks. > The things are still syscalls instead of procctl() commands. I haven't done that yet. procctl() doesn't have a concept of per-TID operations at all, and it currently has a framework for iterating over things that this would have to slot into somehow. I'm open to suggestions on how you would integrate per-TID operations into procctl(). > I did not read further, the patch is half-done at best. That's lovely. Meanwhile, people are actively using this thing. -adrian From owner-freebsd-arch@freebsd.org Wed Jul 8 02:43:27 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 9533D995577 for ; Wed, 8 Jul 2015 02:43:27 +0000 (UTC) (envelope-from rpaulo@me.com) Received: from mr11p00im-asmtp001.me.com (mr11p00im-asmtp001.me.com [17.110.69.252]) (using TLSv1.2 with cipher DHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 6DC6411FF; Wed, 8 Jul 2015 02:43:27 +0000 (UTC) (envelope-from rpaulo@me.com) Received: from me.com (c-73-162-13-215.hsd1.ca.comcast.net [73.162.13.215]) by mr11p00im-asmtp001.me.com (Oracle Communications Messaging Server 7.0.5.35.0 64bit (built Mar 31 2015)) with ESMTPSA id <0NR500H94E86C100@mr11p00im-asmtp001.me.com>; Wed, 08 Jul 2015 02:43:19 +0000 (GMT) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.14.151,1.0.33,0.0.0000 definitions=2015-07-08_02:2015-07-07,2015-07-08,1970-01-01 signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 suspectscore=4 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=7.0.1-1412110000 definitions=main-1507080044 From: Rui Paulo To: freebsd-arch@freebsd.org Cc: Adrian Chadd , Konstantin Belousov Subject: Re: CFT/CFR: NUMA policy branch Date: Tue, 07 Jul 2015 19:43:18 -0700 Message-id: <2926903.YAk7qUEGf9@akita> User-Agent: KMail/4.14.3 (FreeBSD/11.0-CURRENT; KDE/4.14.3; amd64; ; ) In-reply-to: References: <20150707093747.GE2080@kib.kiev.ua> MIME-version: 1.0 Content-transfer-encoding: 7Bit Content-type: text/plain; charset=us-ascii 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 02:43:27 -0000 On Tuesday 07 July 2015 15:53:18 Adrian Chadd wrote: > > I did not read further, the patch is half-done at best. > > That's lovely. Meanwhile, people are actively using this thing. It may not be perfect, but it's way more than half done. You might object to introducing the syscalls, but procctl is still annoyingly limited. -- Rui Paulo From owner-freebsd-arch@freebsd.org Wed Jul 8 05:29:54 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 866AC9952A1 for ; Wed, 8 Jul 2015 05:29:54 +0000 (UTC) (envelope-from alfred@freebsd.org) Received: from elvis.mu.org (elvis.mu.org [IPv6:2001:470:1f05:b76::196]) by mx1.freebsd.org (Postfix) with ESMTP id 75F621679 for ; Wed, 8 Jul 2015 05:29:54 +0000 (UTC) (envelope-from alfred@freebsd.org) Received: from u10-2-32-011.office.norse-data.com (unknown [50.204.88.51]) by elvis.mu.org (Postfix) with ESMTPSA id 80CA7341F841; Tue, 7 Jul 2015 22:29:47 -0700 (PDT) Message-ID: <559CB5DB.7040209@freebsd.org> Date: Tue, 07 Jul 2015 22:32:11 -0700 From: Alfred Perlstein Organization: FreeBSD User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: freebsd-arch@freebsd.org, Konstantin Belousov Subject: Re: CFT/CFR: NUMA policy branch References: <20150707093747.GE2080@kib.kiev.ua> In-Reply-To: <20150707093747.GE2080@kib.kiev.ua> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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 05:29:54 -0000 On 7/7/15 2:37 AM, Konstantin Belousov wrote: > I did not read further, the patch is half-done at best. This is unproductive. People are using Adrian's work and we plan on using it as well. FreeBSD is behind in the NUMA game by at least a decade Please describe in detail what the issues are, do not roadblock based on vagueshedding. JFYI: From http://kernelnewbies.org/LinuxVersions -> 2.5.59 released Janury 17, 2003: NUMA aware scheduler extensions Let's move on and incrementally fix this as opposed to roadblocking. It's 13 years late. -Alfred From owner-freebsd-arch@freebsd.org Wed Jul 8 05:30:55 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 3402C9953A1 for ; Wed, 8 Jul 2015 05:30:55 +0000 (UTC) (envelope-from alfred@freebsd.org) Received: from elvis.mu.org (elvis.mu.org [IPv6:2001:470:1f05:b76::196]) by mx1.freebsd.org (Postfix) with ESMTP id 20B99192D; Wed, 8 Jul 2015 05:30:55 +0000 (UTC) (envelope-from alfred@freebsd.org) Received: from u10-2-32-011.office.norse-data.com (unknown [50.204.88.51]) by elvis.mu.org (Postfix) with ESMTPSA id 0FB3E341F885; Tue, 7 Jul 2015 22:30:55 -0700 (PDT) Message-ID: <559CB61F.2070301@freebsd.org> Date: Tue, 07 Jul 2015 22:33:19 -0700 From: Alfred Perlstein Organization: FreeBSD User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Rui Paulo , freebsd-arch@freebsd.org CC: Konstantin Belousov , Adrian Chadd Subject: Re: CFT/CFR: NUMA policy branch References: <20150707093747.GE2080@kib.kiev.ua> <2926903.YAk7qUEGf9@akita> In-Reply-To: <2926903.YAk7qUEGf9@akita> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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 05:30:55 -0000 On 7/7/15 7:43 PM, Rui Paulo wrote: > On Tuesday 07 July 2015 15:53:18 Adrian Chadd wrote: > >>> I did not read further, the patch is half-done at best. >> That's lovely. Meanwhile, people are actively using this thing. > It may not be perfect, but it's way more than half done. You might object to > introducing the syscalls, but procctl is still annoyingly limited. > (not yelling at you Rui)... but really... Is that the problem?!!? Just write a userland library to abstract the kernel interface! -Alfred 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-- From owner-freebsd-arch@freebsd.org Wed Jul 8 06:38:11 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 08B149960F5 for ; Wed, 8 Jul 2015 06:38:11 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: from mail-ig0-x234.google.com (mail-ig0-x234.google.com [IPv6:2607:f8b0:4001:c05::234]) (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 C5EF81681 for ; Wed, 8 Jul 2015 06:38:10 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: by iggp10 with SMTP id p10so14891183igg.0 for ; Tue, 07 Jul 2015 23:38:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type:content-transfer-encoding; bh=XVsgEyaZ9PDJwwk2GUZvd8dPnm2HDChAO3QcqcaSyCQ=; b=Bph9UZAaCcatZDjG7d1NTJdPCevVyqysEjCbf2CSrJYPyMy9sehOhB0oWOg/NKmStg BrsHe72iFmJ4OJShhViWQg5FhSm89HUCk9ExmiY2hvmIJxsycgOjEvH/JXBbmxU0BPL9 XSJGTdvQWUPwVQOJZEM2C55BSgpIAq1Mecww1iAVSzcsqsHEvhEejwUk/D8lLOVNIM6g Jz7zcFqJopDQGX5tqrmX12hUn7ac3pK2wM7fk9Peur3BAirEpqJFFiDLzhyD59LDBLeF njGGCCEHGZBXadr2Bb8sorPlP99MluMeF4PkdbH7j2Pf1I1a6gWZOeeJlaiU7yqr7WFe 7Ndw== MIME-Version: 1.0 X-Received: by 10.107.155.74 with SMTP id d71mr13796318ioe.29.1436337490210; Tue, 07 Jul 2015 23:38:10 -0700 (PDT) Sender: adrian.chadd@gmail.com Received: by 10.36.38.133 with HTTP; Tue, 7 Jul 2015 23:38:10 -0700 (PDT) In-Reply-To: References: Date: Tue, 7 Jul 2015 23:38:10 -0700 X-Google-Sender-Auth: mUbZFW01ie5RLTqev7tsAyu5KTE Message-ID: Subject: Re: CFT/CFR: NUMA policy branch From: Adrian Chadd To: Garrett Cooper Cc: "freebsd-arch@freebsd.org" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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:38:11 -0000 There's a phabricator review. It's not up to date, because: * it broke for a while, and * kib requested he be sent patches, not a phabricator review. -a On 7 July 2015 at 23:13, Garrett Cooper wrote: > On Jul 5, 2015, at 19:06, Adrian Chadd wrote: > >> Hi, >> >> I've done another update. kib@ has been beating me with the clue stick a= bit. >> >> https://github.com/freebsd/freebsd/compare/master...erikarn:local/adrian= _numa_policy >> >> * (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 >> >> I've tested the policies (first-touch, fixed-domain, round-robin) and >> they all still work as advertised, both on threads and processes. >> >> 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 somet= hing be implemented like sysctl(3) for handling getting/setting of affiniti= es 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 acros= s 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 multi= ple threads access/mangle the value of td_dom_rr_idx in parallel? > - In vm_domain_policy_validate, couldn=E2=80=99t you remove all of the in= termediary `return (-1)`=E2=80=99s as long as you put `break;`s in the swit= ch/case statements? > - Should vm_domain_policy_cleanup/vm_domain_policy_init be implemented? I= f so, what should they have in there? > - Would it make sense for `struct vm_domain_iterator` to be a queue(9)-li= ke 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 =E2=80=94 why not move that outside the switch-case stat= ement? > - Why did you hand roll `vm_domain_iterator_isdone` in `vm_domain_iterato= r_run` up at the top? > - The parameter names for functions/syscalls can be omitted in the declar= ations. > - 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=E2=80=99t` ->= `However without the following check, the kernel will panic below; the cod= e didn=E2=80=99t` > - 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 From owner-freebsd-arch@freebsd.org Wed Jul 8 06:44:28 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 39B75996221 for ; Wed, 8 Jul 2015 06:44:28 +0000 (UTC) (envelope-from rpaulo@me.com) Received: from mr11p00im-asmtp003.me.com (mr11p00im-asmtp003.me.com [17.110.69.254]) (using TLSv1.2 with cipher DHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 0D9C91B1C; Wed, 8 Jul 2015 06:44:28 +0000 (UTC) (envelope-from rpaulo@me.com) Received: from me.com (c-73-162-13-215.hsd1.ca.comcast.net [73.162.13.215]) by mr11p00im-asmtp003.me.com (Oracle Communications Messaging Server 7.0.5.35.0 64bit (built Mar 31 2015)) with ESMTPSA id <0NR500BOSPDQGH20@mr11p00im-asmtp003.me.com>; Wed, 08 Jul 2015 06:44:20 +0000 (GMT) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.14.151,1.0.33,0.0.0000 definitions=2015-07-08_04:2015-07-07,2015-07-08,1970-01-01 signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 suspectscore=2 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=7.0.1-1412110000 definitions=main-1507080118 From: Rui Paulo To: Alfred Perlstein Cc: freebsd-arch@freebsd.org, Konstantin Belousov , Adrian Chadd Subject: Re: CFT/CFR: NUMA policy branch Date: Tue, 07 Jul 2015 23:44:13 -0700 Message-id: <1443707.QHq1OS6BQP@akita> User-Agent: KMail/4.14.3 (FreeBSD/11.0-CURRENT; KDE/4.14.3; amd64; ; ) In-reply-to: <559CB61F.2070301@freebsd.org> References: <2926903.YAk7qUEGf9@akita> <559CB61F.2070301@freebsd.org> MIME-version: 1.0 Content-transfer-encoding: 7Bit Content-type: text/plain; charset=us-ascii 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:44:28 -0000 On Tuesday 07 July 2015 22:33:19 Alfred Perlstein wrote: > On 7/7/15 7:43 PM, Rui Paulo wrote: > > On Tuesday 07 July 2015 15:53:18 Adrian Chadd wrote: > >>> I did not read further, the patch is half-done at best. > >> > >> That's lovely. Meanwhile, people are actively using this thing. > > > > It may not be perfect, but it's way more than half done. You might object > > to introducing the syscalls, but procctl is still annoyingly limited. > (not yelling at you Rui)... but really... Is that the problem?!!? Just > write a userland library to abstract the kernel interface! How can a library help? If you can't tell the kernel to apply a policy per- TID (procctl works by PID), it's useless for multi-threaded applications. -- Rui Paulo From owner-freebsd-arch@freebsd.org Wed Jul 8 07:36:09 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 CDFA49969D6 for ; Wed, 8 Jul 2015 07:36:09 +0000 (UTC) (envelope-from bright@mu.org) Received: from elvis.mu.org (elvis.mu.org [IPv6:2001:470:1f05:b76::196]) by mx1.freebsd.org (Postfix) with ESMTP id BDA261D76; Wed, 8 Jul 2015 07:36:09 +0000 (UTC) (envelope-from bright@mu.org) Received: from [IPv6:2601:645:8004:7515:1da0:325d:7d6:e304] (unknown [IPv6:2601:645:8004:7515:1da0:325d:7d6:e304]) by elvis.mu.org (Postfix) with ESMTPSA id AA062341F841; Wed, 8 Jul 2015 00:36:08 -0700 (PDT) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (1.0) Subject: Re: CFT/CFR: NUMA policy branch From: Alfred Perlstein X-Mailer: iPhone Mail (12H143) In-Reply-To: <1443707.QHq1OS6BQP@akita> Date: Wed, 8 Jul 2015 00:36:07 -0700 Cc: Alfred Perlstein , "freebsd-arch@freebsd.org" , Konstantin Belousov , Adrian Chadd Content-Transfer-Encoding: quoted-printable Message-Id: <295D42F8-6542-4665-B824-B61A7CD48282@mu.org> References: <2926903.YAk7qUEGf9@akita> <559CB61F.2070301@freebsd.org> <1443707.QHq1OS6BQP@akita> To: Rui Paulo 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 07:36:09 -0000 > On Jul 7, 2015, at 11:44 PM, Rui Paulo wrote: >=20 >> On Tuesday 07 July 2015 22:33:19 Alfred Perlstein wrote: >>> On 7/7/15 7:43 PM, Rui Paulo wrote: >>> On Tuesday 07 July 2015 15:53:18 Adrian Chadd wrote: >>>>> I did not read further, the patch is half-done at best. >>>>=20 >>>> That's lovely. Meanwhile, people are actively using this thing. >>>=20 >>> It may not be perfect, but it's way more than half done. You might obje= ct >>> to introducing the syscalls, but procctl is still annoyingly limited. >> (not yelling at you Rui)... but really... Is that the problem?!!? Just >> write a userland library to abstract the kernel interface! >=20 > How can a library help? If you can't tell the kernel to apply a policy pe= r- > TID (procctl works by PID), it's useless for multi-threaded applications. >=20 The library would abstract away the kernel boundary concerns. So let's say w= e did what kib wanted and extended procctl to support tids, well at that poi= nt the syscalls made could be garbage collected and the library updated to c= all the correct kernel entry point.=20= From owner-freebsd-arch@freebsd.org Wed Jul 8 19:16:09 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 C5558996520 for ; Wed, 8 Jul 2015 19:16:09 +0000 (UTC) (envelope-from alfred@freebsd.org) Received: from elvis.mu.org (elvis.mu.org [IPv6:2001:470:1f05:b76::196]) by mx1.freebsd.org (Postfix) with ESMTP id B28E9167D; Wed, 8 Jul 2015 19:16:09 +0000 (UTC) (envelope-from alfred@freebsd.org) Received: from u10-2-32-011.office.norse-data.com (unknown [50.204.88.51]) by elvis.mu.org (Postfix) with ESMTPSA id 4A634341F877; Wed, 8 Jul 2015 12:16:09 -0700 (PDT) Message-ID: <559D778B.5050408@freebsd.org> Date: Wed, 08 Jul 2015 12:18:35 -0700 From: Alfred Perlstein Organization: FreeBSD User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Adrian Chadd , Garrett Cooper CC: "freebsd-arch@freebsd.org" Subject: Re: CFT/CFR: NUMA policy branch References: In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit 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 19:16:10 -0000 On 7/7/15 11:38 PM, Adrian Chadd wrote: > There's a phabricator review. It's not up to date, because: > > * it broke for a while, and > * kib requested he be sent patches, not a phabricator review. > So Kib is complaining that his feedback is getting lost, but refuses to use a review tracker? MFW: http://images5.fanpop.com/image/photos/25000000/No-country-for-old-men-tommy-lee-jones-25069727-450-276.jpg -Alfred > > -a > > > On 7 July 2015 at 23:13, Garrett Cooper wrote: >> On Jul 5, 2015, at 19:06, Adrian Chadd wrote: >> >>> Hi, >>> >>> I've done another update. kib@ has been beating me with the clue stick a bit. >>> >>> https://github.com/freebsd/freebsd/compare/master...erikarn:local/adrian_numa_policy >>> >>> * (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 >>> >>> I've tested the policies (first-touch, fixed-domain, round-robin) and >>> they all still work as advertised, both on threads and processes. >>> >>> 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=16` 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 != 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’t you remove all of the intermediary `return (-1)`’s 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 <= 0 run is done — 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’t` -> `However without the following check, the kernel will panic below; the code didn’t` >> - 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 > _______________________________________________ > freebsd-arch@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-arch > To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" From owner-freebsd-arch@freebsd.org Wed Jul 8 19:34:02 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 3375C9969A7 for ; Wed, 8 Jul 2015 19:34:02 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from mail-pa0-f47.google.com (mail-pa0-f47.google.com [209.85.220.47]) (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 055CA1E0A for ; Wed, 8 Jul 2015 19:34:01 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: by pabvl15 with SMTP id vl15so136540039pab.1 for ; Wed, 08 Jul 2015 12:33:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:subject:mime-version:content-type:from :in-reply-to:date:cc:message-id:references:to; bh=9TfwR0q4RaU9A4zU9k3N+b0dV/+KFKUnfJtNfInRanA=; b=E1NCVoWwTPivW09TVDMvvDyrFH9WnxGVOIzMIkgGLdzeVvr4h++6p5+5Y9MsO27y1i 1BjjPmxe4MIYHuGdHhqQzwFsILUTfPfa9ej98V8YWyozjgAXDi/GQSRR8VaLm5R96euR 8hGnqV2bLa4EWgx1TAE8pnsYQi0OcJnqeby72Ww8ujli8LV1k++tWOl0zgfsRpE3Pla/ TIHxDqSncGL++nTah15AmWCgg+50a7OiG3DlOPXfOetUztNk2EziVpWxoV3OzTRnHr6B KxA1fYh1u6DNae/12orhy2raMSeXetrATITyqsVNTLHgthTu//W/DTNfNplWjJbEzKiU eMDA== X-Gm-Message-State: ALoCoQmLz/yKS3UMOOkwoOUlvN95x6+bUfrcCMyovYDBrF3xpRYNWBLYIrKc4mMeI2XiWdk+4VXG X-Received: by 10.70.102.164 with SMTP id fp4mr23911254pdb.48.1436384035444; Wed, 08 Jul 2015 12:33:55 -0700 (PDT) Received: from [10.64.26.63] ([69.53.236.236]) by smtp.gmail.com with ESMTPSA id om10sm3347774pbb.58.2015.07.08.12.33.53 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 08 Jul 2015 12:33:54 -0700 (PDT) Sender: Warner Losh Subject: Re: CFT/CFR: NUMA policy branch Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2098\)) Content-Type: multipart/signed; boundary="Apple-Mail=_A32F3F2F-3D8E-42E7-903C-EFE2F1DBAF21"; protocol="application/pgp-signature"; micalg=pgp-sha512 X-Pgp-Agent: GPGMail 2.5 From: Warner Losh In-Reply-To: <559D778B.5050408@freebsd.org> Date: Wed, 8 Jul 2015 13:33:40 -0600 Cc: Adrian Chadd , Garrett Cooper , "freebsd-arch@freebsd.org" Message-Id: <168418DA-CCD3-4BA8-A0C9-5F0CF967F2E0@bsdimp.com> References: <559D778B.5050408@freebsd.org> To: Alfred Perlstein X-Mailer: Apple Mail (2.2098) 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 19:34:02 -0000 --Apple-Mail=_A32F3F2F-3D8E-42E7-903C-EFE2F1DBAF21 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On Jul 8, 2015, at 1:18 PM, Alfred Perlstein = wrote: >=20 >=20 >=20 > On 7/7/15 11:38 PM, Adrian Chadd wrote: >> There's a phabricator review. It's not up to date, because: >>=20 >> * it broke for a while, and >> * kib requested he be sent patches, not a phabricator review. >>=20 >=20 >=20 > So Kib is complaining that his feedback is getting lost, but refuses = to use a review tracker? >=20 > MFW: > = http://images5.fanpop.com/image/photos/25000000/No-country-for-old-men-tom= my-lee-jones-25069727-450-276.jpg Do we really need to resort to name calling for a reviewer who is trying to help make things better? kib has provided me good feedback on patches I=E2=80=99ve done in the past, though it sometimes takes me a while to = understand his concerns. It is well worth the while to do that, and to engage him constructively rather than belittling his efforts. And experience has = shown that phabricator is great for small patches, but terrible for large = patches that get revised over and over before going in. This is the 4th or 5th review than I can recall where phabricator=E2=80=99s flaws went from = minor annoyances to major hassles. For really big reviews, I=E2=80=99m = starting to think after 2 or 3 rounds we should close the review and start a new one to help work around the issues. In other words, the right reaction to =E2=80=9CI=E2=80=99m stopping the = review here since it isn=E2=80=99t half done=E2=80=9D isn=E2=80=99t the defensive and belittling one one = I=E2=80=99ve seen, but rather to start a conversation about what he thinks is missing. Maybe he=E2=80=99s = missed something, or maybe you have. While it is cool people are using it, = kib=E2=80=99s concerns generally are looking past the initial glow of partial success = for how to climb the next mountain range and generally are worth the effort to get. Warner > -Alfred >=20 >>=20 >> -a >>=20 >>=20 >> On 7 July 2015 at 23:13, Garrett Cooper = wrote: >>> On Jul 5, 2015, at 19:06, Adrian Chadd wrote: >>>=20 >>>> 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. >>>=20 >>> - 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=E2=80=99t you remove all of = the intermediary `return (-1)`=E2=80=99s 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 =E2=80=94 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=E2=80=99t`= -> `However without the following check, the kernel will panic below; = the code didn=E2=80=99t` >>> - 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. >>>=20 >>> Thanks! >>> -NGie >> _______________________________________________ >> freebsd-arch@freebsd.org mailing list >> http://lists.freebsd.org/mailman/listinfo/freebsd-arch >> To unsubscribe, send any mail to = "freebsd-arch-unsubscribe@freebsd.org" >=20 > _______________________________________________ > freebsd-arch@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-arch > To unsubscribe, send any mail to = "freebsd-arch-unsubscribe@freebsd.org" --Apple-Mail=_A32F3F2F-3D8E-42E7-903C-EFE2F1DBAF21 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 iQIcBAEBCgAGBQJVnXsVAAoJEGwc0Sh9sBEAPlUQANgVqYekRlZgk2KoFLVwoZzW 6rMEKLC0SSW9LmcH2fpx8w0OZ+7DenCzEe7Szcn7sQzEsfjACgOEcziRnuMjlhTw Kay0jLiGviFm2Y6c6CV4JmE9D+vvNMpASUzD6+Mx1UHh7cbB9kWXYvuD4YZfYuEv L6rcipAWar7jsf7rl3Ez/fdMQVH2E1QtNm0Yrg1ZOV09+ENkxof0b/tJytt6iCAs ISKeSuDlXbZ0uj6NawtZ3qTafCjMb1ayccsXFQHizmBjxWqMY1mZwk3tOCLjgsZZ O7Ix/GZgF5ic4CDNXNNf/Ww3FEcYaplK1r3yia54wEwZYWPIotuCVB+eRei3p/sX mCdtAlwJa6XD+1B3uS7pHMnK7Zeg9adySDKZdnLy72RVr2jSPDYrFtEx4mkAoq3h 4jYKfnIVL4bbreTnp1Qik2HuwZkl39AwVGbJpm/49CM3Y4OVO5r1sehWBZjX028i /Yudo6p9tDn9YBpVR47OWnSCvMmyG/4g4HLbhGJjfsOfcO/ZVsgKWyDVgitzBSxx iO4qQwT4UpiUWwgqBxdpVU5YcewPRjM0Bc+eh1Vv8Pqj8JUZQnmZo/0OCxCt6kXO V90/nXYeYoBAF5lFYgG67iO/qiUinnElllqIqbm8sYrAtQl7dBEvKS2gNqUmjtjw DYmxSb0W2EsHbZUEAC3L =qAlQ -----END PGP SIGNATURE----- --Apple-Mail=_A32F3F2F-3D8E-42E7-903C-EFE2F1DBAF21-- From owner-freebsd-arch@freebsd.org Wed Jul 8 19:44:37 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 1A419996C16 for ; Wed, 8 Jul 2015 19:44:37 +0000 (UTC) (envelope-from bright@mu.org) Received: from elvis.mu.org (elvis.mu.org [192.203.228.196]) by mx1.freebsd.org (Postfix) with ESMTP id 079CE165A; Wed, 8 Jul 2015 19:44:36 +0000 (UTC) (envelope-from bright@mu.org) Received: from [10.2.8.86] (unknown [50.204.88.51]) by elvis.mu.org (Postfix) with ESMTPSA id 3C8EE341F877; Wed, 8 Jul 2015 12:44:35 -0700 (PDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (1.0) Subject: Re: CFT/CFR: NUMA policy branch From: Alfred Perlstein X-Mailer: iPhone Mail (12H143) In-Reply-To: <168418DA-CCD3-4BA8-A0C9-5F0CF967F2E0@bsdimp.com> Date: Wed, 8 Jul 2015 12:44:34 -0700 Cc: Alfred Perlstein , Adrian Chadd , Garrett Cooper , "freebsd-arch@freebsd.org" Content-Transfer-Encoding: quoted-printable Message-Id: References: <559D778B.5050408@freebsd.org> <168418DA-CCD3-4BA8-A0C9-5F0CF967F2E0@bsdimp.com> To: Warner Losh 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 19:44:37 -0000 What name calling? Sent from my iPhone > On Jul 8, 2015, at 12:33 PM, Warner Losh wrote: >=20 >=20 >> On Jul 8, 2015, at 1:18 PM, Alfred Perlstein wrote: >>=20 >>=20 >>=20 >>> On 7/7/15 11:38 PM, Adrian Chadd wrote: >>> There's a phabricator review. It's not up to date, because: >>>=20 >>> * it broke for a while, and >>> * kib requested he be sent patches, not a phabricator review. >>=20 >>=20 >> So Kib is complaining that his feedback is getting lost, but refuses to u= se a review tracker? >>=20 >> MFW: >> http://images5.fanpop.com/image/photos/25000000/No-country-for-old-men-to= mmy-lee-jones-25069727-450-276.jpg >=20 > Do we really need to resort to name calling for a reviewer who is trying > to help make things better? kib has provided me good feedback on patches > I=E2=80=99ve done in the past, though it sometimes takes me a while to und= erstand > his concerns. It is well worth the while to do that, and to engage him > constructively rather than belittling his efforts. And experience has show= n > that phabricator is great for small patches, but terrible for large patche= s > that get revised over and over before going in. This is the 4th or 5th > review than I can recall where phabricator=E2=80=99s flaws went from minor= > annoyances to major hassles. For really big reviews, I=E2=80=99m starting t= o think > after 2 or 3 rounds we should close the review and start a new one > to help work around the issues. >=20 > In other words, the right reaction to =E2=80=9CI=E2=80=99m stopping the re= view here since it isn=E2=80=99t > half done=E2=80=9D isn=E2=80=99t the defensive and belittling one one I=E2= =80=99ve seen, but rather to > start a conversation about what he thinks is missing. Maybe he=E2=80=99s m= issed > something, or maybe you have. While it is cool people are using it, kib=E2= =80=99s > concerns generally are looking past the initial glow of partial success fo= r > how to climb the next mountain range and generally are worth the effort > to get. >=20 > Warner >=20 >> -Alfred >>=20 >>>=20 >>> -a >>>=20 >>>=20 >>>> On 7 July 2015 at 23:13, Garrett Cooper wrote: >>>>> On Jul 5, 2015, at 19:06, Adrian Chadd wrote: >>>>>=20 >>>>> 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/adri= an_numa_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. >>>>=20 >>>> - 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_poli= cy 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_nu= ma_getaffinity, but not sys_numa_setaffinity? >>>> - sys_numa_setaffinity and sys_numa_getaffinity look similar. Could som= ething be implemented like sysctl(3) for handling getting/setting of affinit= ies 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 acr= oss 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 mul= tiple threads access/mangle the value of td_dom_rr_idx in parallel? >>>> - In vm_domain_policy_validate, couldn=E2=80=99t you remove all of the i= ntermediary `return (-1)`=E2=80=99s as long as you put `break;`s in the swit= ch/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 =E2=80=94 why not move that outside the switch-case sta= tement? >>>> - Why did you hand roll `vm_domain_iterator_isdone` in `vm_domain_itera= tor_run` up at the top? >>>> - The parameter names for functions/syscalls can be omitted in the decl= arations. >>>> - The change to vm_page.c seems like it could be committed separate fro= m the NUMA changes. >>>> - `However without it'll kernel panic below - the code didn=E2=80=99t` -= > `However without the following check, the kernel will panic below; the cod= e didn=E2=80=99t` >>>> - vm_domain_iterator_run seems like it could use one of the queue(9) da= ta structures. >>>> - Why is OPT_TID 1001? >>>> - `int error;` should come after `cpuset_t set;` per alignment and styl= e(9). >>>> - `atoi` parsing is better handled via strtoll, etc. >>>>=20 >>>> Thanks! >>>> -NGie >>> _______________________________________________ >>> freebsd-arch@freebsd.org mailing list >>> http://lists.freebsd.org/mailman/listinfo/freebsd-arch >>> To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" >>=20 >> _______________________________________________ >> freebsd-arch@freebsd.org mailing list >> http://lists.freebsd.org/mailman/listinfo/freebsd-arch >> To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" >=20 From owner-freebsd-arch@freebsd.org Wed Jul 8 20:44:18 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 ECFB7997617 for ; Wed, 8 Jul 2015 20:44:18 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 7F0AE3780; Wed, 8 Jul 2015 20:44:14 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from porto.starpoint.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id XAA29606; Wed, 08 Jul 2015 23:44:12 +0300 (EEST) (envelope-from avg@FreeBSD.org) Received: from localhost ([127.0.0.1]) by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1ZCwCS-0008R9-AJ; Wed, 08 Jul 2015 23:44:12 +0300 Message-ID: <559D8B78.4020305@FreeBSD.org> Date: Wed, 08 Jul 2015 23:43:36 +0300 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Alfred Perlstein , Adrian Chadd , Garrett Cooper CC: "freebsd-arch@freebsd.org" Subject: Re: CFT/CFR: NUMA policy branch References: <559D778B.5050408@freebsd.org> In-Reply-To: <559D778B.5050408@freebsd.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit 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 20:44:19 -0000 On 08/07/2015 22:18, Alfred Perlstein wrote: > > > On 7/7/15 11:38 PM, Adrian Chadd wrote: >> There's a phabricator review. It's not up to date, because: >> >> * it broke for a while, and >> * kib requested he be sent patches, not a phabricator review. >> > > > So Kib is complaining that his feedback is getting lost, but refuses to use a > review tracker? How about phabricator losing diffs? Would that be a valid complaint? P.S. -- Andriy Gapon From owner-freebsd-arch@freebsd.org Wed Jul 8 20:57:42 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 059EE9978B5 for ; Wed, 8 Jul 2015 20:57:42 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: from mail-ig0-x22e.google.com (mail-ig0-x22e.google.com [IPv6:2607:f8b0:4001:c05::22e]) (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 C2D121231; Wed, 8 Jul 2015 20:57:41 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: by igau2 with SMTP id u2so73656928iga.0; Wed, 08 Jul 2015 13:57:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=8iQ+feOdtntlY0qex75mUj25ZfnCcpHqa0lHKA5yL4A=; b=VZhjR09IGJ8lBdLP2UzWnXnC0vtFh316YF4thB7YSz/0lK0mPU0LibuNB3iic/pKLW QQVTZUmY68n96eCdgVawh5TP5hjSQlIOdAQyTWPXKdkUe5OeJOvFDOh5QTq8c9TqIYYE jWtmx+uzy9jd/RcS9ZSB20CSDd21eLOW5Qj19yOsvQ9AD6vHCARmheupaDk0g1vnkXm8 S5KBy9RId3E1lxaEFE2apkDhj1DBMUJAGxHaP54NyPSFl32mOhAAV318M2wnSTXpk+dR PR4YLN8YCCdct/OHM+v9Uam4fI+5xYQV4Kqif1vSFCPllUOvky8a9QyKl0pzXpr8tH39 ndxw== MIME-Version: 1.0 X-Received: by 10.50.102.7 with SMTP id fk7mr62846621igb.49.1436389061128; Wed, 08 Jul 2015 13:57:41 -0700 (PDT) Sender: adrian.chadd@gmail.com Received: by 10.36.38.133 with HTTP; Wed, 8 Jul 2015 13:57:41 -0700 (PDT) In-Reply-To: <559D8B78.4020305@FreeBSD.org> References: <559D778B.5050408@freebsd.org> <559D8B78.4020305@FreeBSD.org> Date: Wed, 8 Jul 2015 13:57:41 -0700 X-Google-Sender-Auth: 4Dpf-ytf6NRFIqNb_4SGLEM1Gnk Message-ID: Subject: Re: CFT/CFR: NUMA policy branch From: Adrian Chadd To: Andriy Gapon Cc: Alfred Perlstein , Garrett Cooper , "freebsd-arch@freebsd.org" Content-Type: text/plain; charset=UTF-8 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 20:57:42 -0000 On 8 July 2015 at 13:43, Andriy Gapon wrote: > On 08/07/2015 22:18, Alfred Perlstein wrote: >> >> >> On 7/7/15 11:38 PM, Adrian Chadd wrote: >>> There's a phabricator review. It's not up to date, because: >>> >>> * it broke for a while, and >>> * kib requested he be sent patches, not a phabricator review. >>> >> >> >> So Kib is complaining that his feedback is getting lost, but refuses to use a >> review tracker? > > How about phabricator losing diffs? Would that be a valid complaint? Hi, Let's not get side tracked. I've invited a variety of people to review and comment on this stuff. Some people want it in reviews.freebsd.org, some want it via diffs. Different people want work done in different units of work. My plan is to get this into "good enough" state to throw into -HEAD. I don't even care if in 12 months it's completely replaced with an alternate implementation and/or API. What i care about right now is getting the basic pieces in place so further work and experimentation can be done. Right now the entry limit to evaluating any NUMA things on FreeBSD is "you don't, without numa.diff", and that's unacceptable. I completely expect that it'll change over the course of a few years. But the fact we still don't have even the most basic userland exposed API for controlling things is IMHO unacceptable and reflects poorly on us as a community. So, I'm looking for less nit-picking and more "this is wrong, you should do this." A lot of kibs responses have been errors on my part that I hadn't picked up on and weren't exposed during testing. I'm looking for more of those. I haven't yet gone over Garrett's comments in too much depth; I'll look at that tonight if I don't fall asleep first. The important thing here is to try and finally move the default available functionality along a little bit so people can get interested and start using this and contribute their own work. Thanks, -adrian From owner-freebsd-arch@freebsd.org Wed Jul 8 21:53:15 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 BEF03996265 for ; Wed, 8 Jul 2015 21:53:15 +0000 (UTC) (envelope-from bright@mu.org) Received: from elvis.mu.org (elvis.mu.org [192.203.228.196]) by mx1.freebsd.org (Postfix) with ESMTP id AB54915E1; Wed, 8 Jul 2015 21:53:15 +0000 (UTC) (envelope-from bright@mu.org) Received: from [10.2.8.86] (unknown [50.204.88.51]) by elvis.mu.org (Postfix) with ESMTPSA id 45F57341F912; Wed, 8 Jul 2015 14:53:12 -0700 (PDT) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (1.0) Subject: Re: CFT/CFR: NUMA policy branch From: Alfred Perlstein X-Mailer: iPhone Mail (12H143) In-Reply-To: Date: Wed, 8 Jul 2015 14:53:11 -0700 Cc: Andriy Gapon , Alfred Perlstein , Garrett Cooper , "freebsd-arch@freebsd.org" Content-Transfer-Encoding: quoted-printable Message-Id: <7458F68B-E31A-4168-90E1-5327AC0D5746@mu.org> References: <559D778B.5050408@freebsd.org> <559D8B78.4020305@FreeBSD.org> To: Adrian Chadd 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 21:53:15 -0000 > On Jul 8, 2015, at 1:57 PM, Adrian Chadd wrote: >=20 >> On 8 July 2015 at 13:43, Andriy Gapon wrote: >>> On 08/07/2015 22:18, Alfred Perlstein wrote: >>>=20 >>>=20 >>>> On 7/7/15 11:38 PM, Adrian Chadd wrote: >>>> There's a phabricator review. It's not up to date, because: >>>>=20 >>>> * it broke for a while, and >>>> * kib requested he be sent patches, not a phabricator review. >>>=20 >>>=20 >>> So Kib is complaining that his feedback is getting lost, but refuses to u= se a >>> review tracker? >>=20 >> How about phabricator losing diffs? Would that be a valid complaint? >=20 > Hi, >=20 > Let's not get side tracked. I've invited a variety of people to review > and comment on this stuff. Some people want it in reviews.freebsd.org, > some want it via diffs. Different people want work done in different > units of work. >=20 > My plan is to get this into "good enough" state to throw into -HEAD. I > don't even care if in 12 months it's completely replaced with an > alternate implementation and/or API. What i care about right now is > getting the basic pieces in place so further work and experimentation > can be done. Right now the entry limit to evaluating any NUMA things > on FreeBSD is "you don't, without numa.diff", and that's unacceptable. > I completely expect that it'll change over the course of a few years. > But the fact we still don't have even the most basic userland exposed > API for controlling things is IMHO unacceptable and reflects poorly on > us as a community. >=20 > So, I'm looking for less nit-picking and more "this is wrong, you > should do this." A lot of kibs responses have been errors on my part > that I hadn't picked up on and weren't exposed during testing. I'm > looking for more of those. I haven't yet gone over Garrett's comments > in too much depth; I'll look at that tonight if I don't fall asleep > first. >=20 > The important thing here is to try and finally move the default > available functionality along a little bit so people can get > interested and start using this and contribute their own work. Can we just use GitHub and move on already? =20 -Alfred= From owner-freebsd-arch@freebsd.org Thu Jul 9 01:36:32 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 9D788996A13 for ; Thu, 9 Jul 2015 01:36:32 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: from mail-ig0-x22f.google.com (mail-ig0-x22f.google.com [IPv6:2607:f8b0:4001:c05::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 66FAF1EA7 for ; Thu, 9 Jul 2015 01:36:32 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: by igau2 with SMTP id u2so77991916iga.0 for ; Wed, 08 Jul 2015 18:36:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type:content-transfer-encoding; bh=SpUc8rU1VK94iiU8nQLBAc0seOgxtQ/mAgvHAZVsKVg=; b=kLkqKo65mr78mXDfN2tSdC4fMqdxa/gAsCLd/ICg1vMaysVMChOdWQ49fKKNRypGew XVqpshejNF0l5qQKtxi2d1SVhkIVkg+kEFpSvnfUejynU+vnO3l9rZD+lY7DrusHCNQs 3mT8OUmaj7Chrvs+kBbk9MtG+FiTIfKBQ5YP54luYtwb8lcYI65YxZfbjvIhz/M7kwuF hFAMAv+dEt3QzRmETdFp3UCj7Yg5132bVEDQvMLG7rYFTJT9Si6yacMghR8C5PTZ3Up/ vRxs1jZ8j0I9IwGyh2k+08oDs2EAJ+6gSvTjSP6vAAf1orAQe32WQkEm5ZyE2nsklOWb u20Q== MIME-Version: 1.0 X-Received: by 10.50.102.7 with SMTP id fk7mr64334559igb.49.1436405791851; Wed, 08 Jul 2015 18:36:31 -0700 (PDT) Sender: adrian.chadd@gmail.com Received: by 10.36.38.133 with HTTP; Wed, 8 Jul 2015 18:36:31 -0700 (PDT) In-Reply-To: References: Date: Wed, 8 Jul 2015 18:36:31 -0700 X-Google-Sender-Auth: KhSntmwcfPJjtWoAc1UvEwzbNhQ Message-ID: Subject: Re: CFT/CFR: NUMA policy branch From: Adrian Chadd To: Garrett Cooper Cc: "freebsd-arch@freebsd.org" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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: Thu, 09 Jul 2015 01:36:32 -0000 On 7 July 2015 at 23:13, Garrett Cooper wrote: > On Jul 5, 2015, at 19:06, Adrian Chadd wrote: > >> Hi, >> >> I've done another update. kib@ has been beating me with the clue stick a= bit. >> >> https://github.com/freebsd/freebsd/compare/master...erikarn:local/adrian= _numa_policy >> >> * (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 >> >> I've tested the policies (first-touch, fixed-domain, round-robin) and >> they all still work as advertised, both on threads and processes. >> >> 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. I'll sketch out a libnuma after this lands. Yes, this stuff will be part of= it. > - Please use that code in the kernel as well for sysctl_vm_default_policy= to reduce duplication. ok. I'll do it after it lands. > - Please note reasoning for why `options MAXMEMDOM=3D16` in numa(4). There's no reasoning. > - Why are checking for `if (p)` before calling PROC_UNLOCK(p) in sys_numa= _getaffinity, but not sys_numa_setaffinity? I'll check. > - sys_numa_setaffinity and sys_numa_getaffinity look similar. Could somet= hing be implemented like sysctl(3) for handling getting/setting of affiniti= es all in one shot? Nope. Other things are the same (eg cpuset). > - `if (p)` should be `if (p !=3D NULL)`, etc per style(9). > - Is there a way that the affinity could be inherited/not inherited acros= s threads, similar to what ktrace -i does? If so, how does one do that? There isn't, much like how there isn't the same for that in cpuset. > - In vm_domain_rr_selectdomain, should this use atomic(9), i.e. can multi= ple threads access/mangle the value of td_dom_rr_idx in parallel? No - it's curthread. only curthread is modifying that. > - In vm_domain_policy_validate, couldn=E2=80=99t you remove all of the in= termediary `return (-1)`=E2=80=99s as long as you put `break;`s in the swit= ch/case statements? > - Should vm_domain_policy_cleanup/vm_domain_policy_init be implemented? I= f so, what should they have in there? They're placeholders - primarily because I think at some point the policy definition will grow to be bigger and we may want to malloc / reference them in some instances. (Much like cpuset.) This way it's clear that they should be called, even if they don't do anyth= ing. I like defining "object lifecycle", even if part of the create/destroy lifecycle is null. Adding things like create/destroy functions later on has proven to be more buggy. > - Would it make sense for `struct vm_domain_iterator` to be a queue(9)-li= ke type (just based on the name alone)? If not, what data structure do you = anticipate it having, e.g. tree, queue, directed graph, etc? No. It's an iterator - it's designed to abstract away the concept of iterating over a domain policy. It won't have any kind of data structure like you mentioned, as it's an iterator. > - In vm_domain_iterator_run, vi->n is always decremented after the vi->n = <=3D 0 run is done =E2=80=94 why not move that outside the switch-case stat= ement? > - Why did you hand roll `vm_domain_iterator_isdone` in `vm_domain_iterato= r_run` up at the top? vm_domain_iterator_isdone() is (currently) defined as a tail loop condition, not a head loop condition. Ie, you can only call it after you've gone through the loop at least once. Now, I don't necessarily like how the loop constructs work, but it's designed to be a minimal set of changes to the vm_page.c code that was doing this kind of iteration over the round-robin set. I think it's because I wrote iterator_isdone() first before I finished fleshing out iterator_run(). > - The parameter names for functions/syscalls can be omitted in the declar= ations. ok > - The change to vm_page.c seems like it could be committed separate from = the NUMA changes. It will be. It's in there because without it any significant imbalance in page allocations between domains can end with exhaustion in one domain and it all going bad. > - `However without it'll kernel panic below - the code didn=E2=80=99t` ->= `However without the following check, the kernel will panic below; the cod= e didn=E2=80=99t` > - vm_domain_iterator_run seems like it could use one of the queue(9) data= structures. again, no - there's no queue struct underlying it. It's an iterator. > - 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. Il'l go sort those out in numactl.c soon. Thanks! -a