des at ofug.org
Sat Mar 29 07:13:50 PST 2003
Bruce Evans <bde at 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
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 len);
> % +extern int (*copyout_vector)(const void *kaddr, void *udaddr, size_t len);
> 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.
Dag-Erling Smørgrav - des at ofug.org
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 6332 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/cvs-src/attachments/20030329/e0c1c824/page.bin
More information about the cvs-src