Date: Sat, 29 Mar 2003 16:13:41 +0100 From: des@ofug.org (Dag-Erling =?iso-8859-1?q?Sm=F8rgrav?=) To: Bruce Evans <bde@zeta.org.au> Cc: cvs-all@freebsd.org Subject: Re: Checksum/copy Message-ID: <xzpznneyzmi.fsf@flood.ping.uio.no> In-Reply-To: <20030329235643.H11074@gamplex.bde.org> (Bruce Evans's message of "Sun, 30 Mar 2003 00:27:29 %2B1100 (EST)") References: <Pine.BSF.4.21.0303260956250.27748-100000@root.org> <20030327180247.D1825@gamplex.bde.org> <20030327232742.GA80113@wantadilla.lemis.com> <20030328161552.L5953@gamplex.bde.org> <20030328073513.GA20464@cirb503493.alcatel.com.au> <xzp3cl61hxs.fsf@flood.ping.uio.no> <20030329235643.H11074@gamplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--=-=-=
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: quoted-printable
Bruce Evans <bde@zeta.org.au> 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
--=-=-=--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?xzpznneyzmi.fsf>