Checksum/copy

Dag-ErlingSmørgrav 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
> 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 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.

DES
-- 
Dag-Erling Smørgrav - des at ofug.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: page.diff
Type: text/x-patch
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 mailing list