svn commit: r327354 - head/sys/vm

[Rodney W. Grimes]

> 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 at freebsd.org