On 03/03/2018 05:21, Konstantin Belousov wrote:
On Sat, Mar 03, 2018 at 01:47:41PM +1100, Bruce Evans wrote:
On Fri, 2 Mar 2018, Konstantin Belousov wrote:

On Fri, Mar 02, 2018 at 12:43:34PM -0500, Pedro Giffuni wrote:
...
I think use of _Nonnull attributes in the threading functions may also
be a waste (I introduced them mostly to be compatible with Android).
FWIW, Dragonfly sprinkled some restrict there recently:

http://gitweb.dragonflybsd.org/dragonfly.git/commit/d33005aaee6af52c80428b59b52aee522c002492

Just in case someone is considering more cleanups.
This is not a cleanup for me, but a needed change. Right now x86
copyouts are implemented in asm, so whatever damage is done to the
prototypes, only effect is at the caller side. In my work, i386 copyouts
are done in C, so it starts matter.
That seems slow, especially for small sizes as are common for syscall args
(in 1 of my versions, copyin() of args is optimized to fuword() in a loop,
and fuword() is optimized to not use pcb_onfault, so it is not much more
than 1 memory access.  However, in your i386 version this optimization
would be negative since the slow part is switching the map, so fuword()
should never be used to access multiple words).
Yes. I already explained it in private, the current choice for i386 is
either to be neglected very fast, or to get this change to still be a
Tier 1 32 bit platform. The change is to make 4/4g split for UVA/KVA.
In particular, the change ensures that it is possible to self-host i386
for forthcoming years, which is not practical for armv7 now and would be
less so with clang grow.

In other news, my system already boots single-user on SMP machine and
I have torture tests like setting invalid %ss segment by sigreturn(2),
work.  There is (much) more to come, but I am happy how the patch
progressed so far.
Very nice.

However, copyinstr() and
copystr() should never have been "optimized" by writing them in asm.  On
x86, their asm is badly written so they are slower than simple C versions
except on 8088's and maybe 8086's and maybe on the original i386.  (8088's
were limited mainly by instruction bandwidth and the original i386 wasn't
much better, so short CISC instructions like lodsb and stosb tended to be
faster than larger separate instructions despite their large setup overheads.
Sure, copyinstr() is rewritten in C.  The current version of copyout stuff
is there:
https://kib.kiev.ua/git/gitweb.cgi?p=deviant2.git;a=blob;f=sys/i386/i386/copyout.c;h=9747c06a84d7d2b5faac946f5de57f6a34d96c8c;hb=refs/heads/pcid

Also I looked at the dragonfly commit because I become curious what do you
mean by threading functions.  The first example was
int    pthread_attr_getguardsize(const pthread_attr_t * __restrict,
-                       size_t *);
+           size_t * __restrict);
POSIX agrees with the dragonfly change, but I do not understand it.
Aliasing rules already disallow the first and second arguments to point
to the same memory, because they have different types.
(1) thread_attr_t is opaque, so the types might be the same.
(2) pthread_attr_t might be a pointer to a struct/union containing a size_t.
(3) perhaps other reasons.  I'm not sure how 'restrict interacts with global
      variables or even it it prevents the interaction in (2).  A previous
      discussion showed that const doesn't make types different enough to
      prevent aliasing.  Similarly for volatile.

Similarly for other pointers to {opaque, struct/union, or even integer} types.
size_t can't be aliased to int, but it can be aliased to any unsigned type
in C and to any unsigned type not smaller than uint16_t in POSIX (POSIX
but not C requires u_char == uint8_t, so size_t can't be u_char in POSIX
but it can be u_char in C).
I can only summarize it as 'there is no use to have restrict on the
pthread_attr_getguardsize() arguments'.


Well, I'll admit I don't understand well the advantages and that's why I brought up a pointer to the changes instead of working on them. Usually, standards compliance is reason enough for such change though.

Pedro.
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to