From owner-cvs-all@FreeBSD.ORG Sat Mar 29 07:13:46 2003 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id DD96A37B401; Sat, 29 Mar 2003 07:13:46 -0800 (PST) Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by mx1.FreeBSD.org (Postfix) with ESMTP id 34BCE43F93; Sat, 29 Mar 2003 07:13:45 -0800 (PST) (envelope-from des@ofug.org) Received: by flood.ping.uio.no (Postfix, from userid 2602) id 2EE0E5308; Sat, 29 Mar 2003 16:13:42 +0100 (CET) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: Bruce Evans From: des@ofug.org (Dag-Erling =?iso-8859-1?q?Sm=F8rgrav?=) Date: Sat, 29 Mar 2003 16:13:41 +0100 In-Reply-To: <20030329235643.H11074@gamplex.bde.org> (Bruce Evans's message of "Sun, 30 Mar 2003 00:27:29 +1100 (EST)") Message-ID: User-Agent: Gnus/5.090015 (Oort Gnus v0.15) Emacs/21.2 References: <20030327180247.D1825@gamplex.bde.org> <20030327232742.GA80113@wantadilla.lemis.com> <20030328161552.L5953@gamplex.bde.org> <20030328073513.GA20464@cirb503493.alcatel.com.au> <20030329235643.H11074@gamplex.bde.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" cc: src-committers@freebsd.org cc: cvs-src@freebsd.org cc: Greg 'groggy' Lehey cc: Mike Silbersack cc: Nate Lawson cc: cvs-all@freebsd.org Subject: Re: Checksum/copy X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 29 Mar 2003 15:13:50 -0000 --=-=-= Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable Bruce Evans writes: > > On a different note, support.s is a bloody mess. Once the dust has > > settled, I'd like to go through it and reorder its contents a little. > There is very little wrong with its order. I placed generic_page*() next to i686_pagezero(), which is right below the various bzero() implementations. That's fine in the sense that it is logically related to bzero(), but it's certainly not alphabetical. There's a comment right before generic_bzero() that says "bcopy family", but bcopy() is miles away from that comment > The microoptimization of making bzero a function pointer wasn't such a > good idea. The main problem with undoing it is that this breaks binary > compatibility. It looked completely bogus to me. I realize it breaks binary compatibility, but I'd rather break it now than after 5.x goes -STABLE. Also, I don't think there are too many third-party modules for 5.x yet, and I'm almost certain there will be more ABI breakage before 5.x goes -STABLE. > % +void pagecopy(const void *from, void *to); > % +void pagezero(void *buf); > > Er, they can't be identical. pmap_zero_page_area() handles partial > pages. i686_pagezero() was only used for the special case where the > partial page is actually the whole page. OK, I was a little too quick there. > I'd prefer not to have more interfaces to combinatorially explode. > The versions of these in the patch are only generic, so using these > interfaces instead of bcopy() and bzero() bypasses non-generic > versions of the latter. We don't currently have any viable non-generic bcopy() or bzero(), but if you prefer, I can make generic_page*() wrappers for b*(). IMHO, if specialized pagezero() and pagecopy() functions result in measurably improved performance, they're worth the added complexity, even if we're just talking about a few percent. > % > % void *memcpy(void *to, const void *from, size_t len); > > The declarations of page*() are disordered ('p' > 'm'). The declarations in systm.h were not sorted alphabetically to start with. They seem to be grouped roughly by purpose, and sometimes sorted alphabetically within the group. > % +extern void (*bcopy_vector)(const void *from, void *to, size_t len); > % +extern void (*ovbcopy_vector)(const void *from, void *to, size_t len); > % +extern void (*bzero_vector)(void *buf, size_t len); > % +extern void (*pagecopy_vector)(const void *from, void *to); > % +extern void (*pagezero_vector)(void *buf); > % +extern int (*copyin_vector)(const void *udaddr, void *kaddr, size_t le= n); > % +extern int (*copyout_vector)(const void *kaddr, void *udaddr, size_t l= en); > > This file was mostly sorted and consistently formatted. Obviously not. The inconsistencies were there to start with, I didn't introduce them. Also, *_vector are variables, not prototypes, and they are listed here only so identcpu.c and npx.c can assign to them. The misalphabetization was not intentional, I just placed page*() next to b*() because they are logically related. New patch attached. DES --=20 Dag-Erling Sm=F8rgrav - des@ofug.org --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=page.diff Index: sys/sys/systm.h =================================================================== RCS file: /home/ncvs/src/sys/sys/systm.h,v retrieving revision 1.191 diff -u -r1.191 systm.h --- sys/sys/systm.h 24 Mar 2003 21:15:35 -0000 1.191 +++ sys/sys/systm.h 29 Mar 2003 12:10:28 -0000 @@ -171,12 +171,9 @@ void bcopy(const void *from, void *to, size_t len); void ovbcopy(const void *from, void *to, size_t len); - -#ifdef __i386__ -extern void (*bzero)(void *buf, size_t len); -#else void bzero(void *buf, size_t len); -#endif +void pagecopy(const void *from, void *to); +void pagezero(void *buf); void *memcpy(void *to, const void *from, size_t len); Index: sys/vm/vm_zeroidle.c =================================================================== RCS file: /home/ncvs/src/sys/vm/vm_zeroidle.c,v retrieving revision 1.18 diff -u -r1.18 vm_zeroidle.c --- sys/vm/vm_zeroidle.c 12 Oct 2002 05:32:24 -0000 1.18 +++ sys/vm/vm_zeroidle.c 29 Mar 2003 11:15:28 -0000 @@ -143,10 +143,10 @@ } } -static struct proc *pagezero; +static struct proc *pagezero_proc; static struct kproc_desc pagezero_kp = { "pagezero", vm_pagezero, - &pagezero + &pagezero_proc }; SYSINIT(pagezero, SI_SUB_KTHREAD_VM, SI_ORDER_ANY, kproc_start, &pagezero_kp) Index: sys/i386/i386/identcpu.c =================================================================== RCS file: /home/ncvs/src/sys/i386/i386/identcpu.c,v retrieving revision 1.119 diff -u -r1.119 identcpu.c --- sys/i386/i386/identcpu.c 18 Mar 2003 08:45:22 -0000 1.119 +++ sys/i386/i386/identcpu.c 29 Mar 2003 12:07:25 -0000 @@ -563,7 +563,7 @@ #if defined(I486_CPU) case CPUCLASS_486: printf("486"); - bzero = i486_bzero; + bzero_vector = i486_bzero; break; #endif #if defined(I586_CPU) @@ -580,6 +580,7 @@ (intmax_t)(tsc_freq + 4999) / 1000000, (u_int)((tsc_freq + 4999) / 10000) % 100); printf("686"); + pagezero_vector = i686_pagezero; break; #endif default: Index: sys/i386/i386/pmap.c =================================================================== RCS file: /home/ncvs/src/sys/i386/i386/pmap.c,v retrieving revision 1.398 diff -u -r1.398 pmap.c --- sys/i386/i386/pmap.c 25 Mar 2003 00:07:02 -0000 1.398 +++ sys/i386/i386/pmap.c 29 Mar 2003 15:06:23 -0000 @@ -2754,12 +2754,7 @@ #endif invlpg((u_int)CADDR2); #endif -#if defined(I686_CPU) - if (cpu_class == CPUCLASS_686) - i686_pagezero(CADDR2); - else -#endif - bzero(CADDR2, PAGE_SIZE); + pagezero(CADDR2); #ifdef SMP curthread->td_switchin = NULL; #endif @@ -2789,11 +2784,9 @@ #endif invlpg((u_int)CADDR2); #endif -#if defined(I686_CPU) - if (cpu_class == CPUCLASS_686 && off == 0 && size == PAGE_SIZE) - i686_pagezero(CADDR2); + if (off == 0 && size == PAGE_SIZE) + pagezero(CADDR2); else -#endif bzero((char *)CADDR2 + off, size); #ifdef SMP curthread->td_switchin = NULL; @@ -2823,12 +2816,7 @@ #endif invlpg((u_int)CADDR3); #endif -#if defined(I686_CPU) - if (cpu_class == CPUCLASS_686) - i686_pagezero(CADDR3); - else -#endif - bzero(CADDR3, PAGE_SIZE); + pagezero(CADDR3); #ifdef SMP curthread->td_switchin = NULL; #endif @@ -2861,7 +2849,7 @@ invlpg((u_int)CADDR1); invlpg((u_int)CADDR2); #endif - bcopy(CADDR1, CADDR2, PAGE_SIZE); + pagecopy(CADDR1, CADDR2); #ifdef SMP curthread->td_switchin = NULL; #endif Index: sys/i386/i386/support.s =================================================================== RCS file: /home/ncvs/src/sys/i386/i386/support.s,v retrieving revision 1.93 diff -u -r1.93 support.s --- sys/i386/i386/support.s 22 Sep 2002 04:45:20 -0000 1.93 +++ sys/i386/i386/support.s 29 Mar 2003 14:57:59 -0000 @@ -48,8 +48,8 @@ .globl bcopy_vector bcopy_vector: .long generic_bcopy - .globl bzero -bzero: + .globl bzero_vector +bzero_vector: .long generic_bzero .globl copyin_vector copyin_vector: @@ -60,6 +60,12 @@ .globl ovbcopy_vector ovbcopy_vector: .long generic_bcopy + .globl pagecopy_vector +pagecopy_vector: + .long generic_pagecopy + .globl pagezero_vector +pagezero_vector: + .long generic_pagezero #if defined(I586_CPU) && defined(DEV_NPX) kernel_fpu_lock: .byte 0xfe @@ -73,6 +79,10 @@ * void bzero(void *buf, u_int len) */ +ENTRY(bzero) + MEXITCOUNT + jmp *bzero_vector + ENTRY(generic_bzero) pushl %edi movl 8(%esp),%edi @@ -349,6 +359,39 @@ ret #endif /* I586_CPU && defined(DEV_NPX) */ +ENTRY(pagecopy) + MEXITCOUNT + jmp *pagecopy_vector + +ENTRY(generic_pagecopy) + pushl %esi + pushl %edi + movl 8(%esp),%esi + movl 12(%esp),%edi + movl $1024,%ecx + xorl %eax,%eax + cld + rep + movsl + popl %edi + popl %esi + ret + +ENTRY(pagezero) + MEXITCOUNT + jmp *pagezero_vector + +ENTRY(generic_pagezero) + pushl %edi + movl 8(%esp),%edi + movl $1024,%ecx + xorl %eax,%eax + cld + rep + stosl + popl %edi + ret + ENTRY(i686_pagezero) pushl %edi pushl %ebx @@ -361,7 +404,7 @@ 1: xorl %eax, %eax repe - scasl + scasl jnz 2f popl %ebx Index: sys/i386/include/md_var.h =================================================================== RCS file: /home/ncvs/src/sys/i386/include/md_var.h,v retrieving revision 1.60 diff -u -r1.60 md_var.h --- sys/i386/include/md_var.h 25 Mar 2003 00:07:03 -0000 1.60 +++ sys/i386/include/md_var.h 29 Mar 2003 15:11:32 -0000 @@ -36,12 +36,17 @@ * Miscellaneous machine-dependent declarations. */ -extern long Maxmem; -extern u_int atdevbase; /* offset in virtual memory of ISA io mem */ extern void (*bcopy_vector)(const void *from, void *to, size_t len); -extern int busdma_swi_pending; +extern void (*bzero_vector)(void *buf, size_t len); extern int (*copyin_vector)(const void *udaddr, void *kaddr, size_t len); extern int (*copyout_vector)(const void *kaddr, void *udaddr, size_t len); +extern void (*ovbcopy_vector)(const void *from, void *to, size_t len); +extern void (*pagecopy_vector)(const void *from, void *to); +extern void (*pagezero_vector)(void *addr); + +extern long Maxmem; +extern u_int atdevbase; /* offset in virtual memory of ISA io mem */ +extern int busdma_swi_pending; extern u_int cpu_exthigh; extern u_int cpu_feature; extern u_int cpu_fxsr; @@ -56,7 +61,6 @@ extern int need_pre_dma_flush; extern int need_post_dma_flush; #endif -extern void (*ovbcopy_vector)(const void *from, void *to, size_t len); extern char sigcode[]; extern int szsigcode; #ifdef COMPAT_FREEBSD4 --=-=-=--