This is the mail archive of the
mailing list for the glibc project.
Re: [PATCH] mips/o32: fix internal_syscall5/6/7
On 17/08/2017 17:34, Maciej W. Rozycki wrote:
> On Thu, 17 Aug 2017, Adhemerval Zanella wrote:
>> My point is I think we should aim for compiler optimization safeness
>> (to avoid code breakage over compiler defined default flags) and taking
>> as base current approach to *avoid* VLA on GLIBC I do not think it is
>> good approach to use it as a bridge to force GCC to generate the expected
> You certainly have a point here overall, although I don't think a VLA
> whose size is always 0 really hurts. And we've used the approach with
> `alloca' since forever with no adverse effects until we added a place
> where the caller invokes the syscall wrapper in a loop. So I wouldn't
> necessarily call it an issue. Mind that this is target-specific code, so
> we can rely on a target-specific execution model rather than limiting
> ourselves to what generic ISO C guarantees.
> Aurelien's figures indicating a clear size reduction certainly count as a
> pro though.
Joseph pointed out another advantage of avoid VLAs (building with
-Werror=alloca -Werror=vla). My main problem here is we are betting that
compiler won't mess with our assumptions and generate the desirable code
without trying to adhere what it is suppose to provide. Target generic
ISO C give us a better guarantee and any deviation indicates a possible
compiler issue, not otherwise (such this case). My another point is we
can optimize if required later if this is the case and imho this is hardly
the case here (at least for latency).
If I understood correctly Aurelien's suggestion of returning err in v1
is not ABI strictly so it will end up calling __libc_do_syscall with a
non-conformant ABI convention (similar to pipe implementation where requires
assembly specific implementation for a lot of architectures to get this
right). Again this is something I would really to avoid.
>> I still thinking trying to optimize for 5/6/7 syscall argument is over
>> engineering in this *specific* case. As I put in my last message,
>> 5/6/7 argument syscalls are used for
>> pread, pwrite, lseek, llseek, ppoll, posix_fadvice, posix_fallocate,
>> sync_file_range, fallocate, preadv, pwritev, preadv2, pwritev2, select,
>> pselect, mmap, readahead, epoll_pwait, splice, recvfrom, sendto, recvmmsg,
>> msgsnd, msgrcv, msgget, msgctl, semop, semget, semctl, semtimedop, shmat,
>> shmdt, shmget, and shmctl.
>> Which are the one generated from C implementation (some are still auto
>> generated). The majority of them are blocking syscalls, so both context
>> switch plus the required work for syscall completion itself will taking
>> proportionally all the required time. So trying to squeeze some cycles
>> don't really pay off comparing to code maintainability (just all this
>> discussion of which C construct would be safe enough to generate the
>> correct stack spill plus the current issue should indicate we should
>> aim for correctness first).
> TBH, I find it questionable whether it's really the approach I proposed
> that requires more engineering (and long-term maintenance) effort rather
> than using a separate handwritten assembly-language call stub. Especially
> if a non-standard calling convention is used.
IMHO I find the VLA suggestion more fragile in long term.
> If everyone but me thinks there's a clear advantage in using such a
> handcoded stub though, then as I previously noted please adjust the
> affected MIPS16 stubs to avoid the extra indirection, i.e. you can call
> `__libc_do_syscall' directly from MIPS16 code as you'd do from regular
> MIPS or microMIPS code, as the lone reason for the existence of the MIPS16
> stubs is the inexistence of a MIPS16 SYSCALL instruction.
Ok, I will try to at least check it on qemu. If you have any points on how
correctly build a mips16 glibc it could be helpful.
> Once you're done with that I can push your proposed change through MIPS16
> regression testing if that helped. I can see if I can run microMIPS
> testing as well, although I'd have to double-check for an available board
> as I don't use one regularly.