svn commit: r330285 - head/sys/sys

[Pedro Giffuni]

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.