Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 3 Mar 2018 23:09:20 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        Pedro Giffuni <pfg@freebsd.org>, src-committers@freebsd.org,  svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r330285 - head/sys/sys
Message-ID:  <20180303224422.U3460@besplex.bde.org>
In-Reply-To: <20180303102106.GG3194@kib.kiev.ua>
References:  <201803021647.w22Gl2t7092316@repo.freebsd.org> <dba3b880-58d0-e6df-8f99-4d27919134c9@FreeBSD.org> <20180302181934.GF3194@kib.kiev.ua> <20180303130511.N1283@besplex.bde.org> <20180303102106.GG3194@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 3 Mar 2018, 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.

I use i386 since it is 10-20% faster than amd64 for my applications,
and don't like changes that fix this by slowing down the fast case.  My
applications don't include clang.

>> ...
>>> 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'.

No, the summary is 'POSIX is correct to declare almost all arg pointers
as restrict, since it is not useful for new APIs to allow aliases, and
restrict must be used so that implementations can take advantage of this
if they want'.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180303224422.U3460>