From owner-svn-src-all@freebsd.org Fri Jan 19 17:05:03 2018 Return-Path: Delivered-To: svn-src-all@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 3E376EBCFB7; Fri, 19 Jan 2018 17:05:03 +0000 (UTC) (envelope-from freebsd@pdx.rh.CN85.dnsmgr.net) Received: from pdx.rh.CN85.dnsmgr.net (br1.CN84in.dnsmgr.net [69.59.192.140]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 10CA569E2D; Fri, 19 Jan 2018 17:05:02 +0000 (UTC) (envelope-from freebsd@pdx.rh.CN85.dnsmgr.net) Received: from pdx.rh.CN85.dnsmgr.net (localhost [127.0.0.1]) by pdx.rh.CN85.dnsmgr.net (8.13.3/8.13.3) with ESMTP id w0JH4suV072968; Fri, 19 Jan 2018 09:04:54 -0800 (PST) (envelope-from freebsd@pdx.rh.CN85.dnsmgr.net) Received: (from freebsd@localhost) by pdx.rh.CN85.dnsmgr.net (8.13.3/8.13.3/Submit) id w0JH4rgT072967; Fri, 19 Jan 2018 09:04:53 -0800 (PST) (envelope-from freebsd) From: "Rodney W. Grimes" Message-Id: <201801191704.w0JH4rgT072967@pdx.rh.CN85.dnsmgr.net> Subject: Re: svn commit: r327354 - head/sys/vm In-Reply-To: <601ee1a2-8f4e-518d-4c86-89871cd652af@vangyzen.net> To: Eric van Gyzen Date: Fri, 19 Jan 2018 09:04:53 -0800 (PST) CC: Gleb Smirnoff , Konstantin Belousov , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Reply-To: rgrimes@freebsd.org X-Mailer: ELM [version 2.4ME+ PL121h (25)] MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Jan 2018 17:05:03 -0000 > On 01/17/2018 16:40, Gleb Smirnoff wrote: > > On Fri, Dec 29, 2017 at 07:05:07PM +0000, Konstantin Belousov wrote: > > K> Author: kib > > K> Date: Fri Dec 29 19:05:07 2017 > > K> New Revision: 327354 > > K> URL: https://svnweb.freebsd.org/changeset/base/327354 > > K> > > K> Log: > > K> Style. > > K> > > K> Reviewed by: alc > > K> Sponsored by: The FreeBSD Foundation > > K> MFC after: 1 week > > K> Differential revision: https://reviews.freebsd.org/D13678 > > K> > > K> Modified: > > K> head/sys/vm/vm_swapout.c > > K> > > K> Modified: head/sys/vm/vm_swapout.c > > K> ============================================================================== > > K> --- head/sys/vm/vm_swapout.c Fri Dec 29 18:42:55 2017 (r327353) > > K> +++ head/sys/vm/vm_swapout.c Fri Dec 29 19:05:07 2017 (r327354) > > K> @@ -556,16 +556,14 @@ vm_thread_swapin(struct thread *td) > > K> { > > K> vm_object_t ksobj; > > K> vm_page_t ma[KSTACK_MAX_PAGES]; > > K> - int pages; > > K> + int a, count, i, j, pages, rv; > > K> > > K> pages = td->td_kstack_pages; > > K> ksobj = td->td_kstack_obj; > > K> VM_OBJECT_WLOCK(ksobj); > > K> (void)vm_page_grab_pages(ksobj, 0, VM_ALLOC_NORMAL | VM_ALLOC_WIRED, ma, > > K> pages); > > K> - for (int i = 0; i < pages;) { > > K> - int j, a, count, rv; > > K> - > > K> + for (i = 0; i < pages;) { > > K> vm_page_assert_xbusied(ma[i]); > > K> if (ma[i]->valid == VM_PAGE_BITS_ALL) { > > K> vm_page_xunbusy(ma[i]); > > > > Yeah, style is sacred, but is there a single person on Earth who would > > not agree that moving variables from smaller blocks to function block > > reduces readability of the code? > > I agree that it reduces the readability. Not only that, it also > encourages real bugs by allowing access to the variable when it does not > make sense. I am mixed on if it changes readability or not, in the current style case it unclutters the code of declarations scattered about, in the other case it may make it further away to find a declaration. BUT I do not believe this bit of "style" has anything to do with readability of code, and has more to do with how code runs on a processor and stack frames. If you defer the declaration of "int i" in the above code does that compiler emmit code to allocate a new stack frame, or does it just add space to the function stack frame for this? What happens if you do for (int i = 0; i < pages;) { } for (int i = 1; i < pages;) { } as 2 seperate loops, do we allocate 2 i's on the stack at function startup, or do we defer and allacte each one only when that basic block runs, or do we allocate 1 i and reuse it, I know that the compiler makes functional code but how is that functionality done? The current style leaves no doubt about how things are done from that perspective. Just my $0.05, inflation and all.... > Eric -- Rod Grimes rgrimes@freebsd.org