unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Florian Weimer <fw@deneb.enyo.de>
To: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Cc: libc-alpha@sourceware.org
Subject: Re: [PATCH 03/15] sparc: Use Linux kABI for syscall return
Date: Tue, 11 Feb 2020 20:24:37 +0100	[thread overview]
Message-ID: <874kvxnczu.fsf@mid.deneb.enyo.de> (raw)
In-Reply-To: <10c5636d-6ea7-e95c-3a3c-67be298472fd@linaro.org> (Adhemerval Zanella's message of "Tue, 11 Feb 2020 15:55:09 -0300")

* Adhemerval Zanella:

> On 11/02/2020 08:15, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> +	if (__glibc_unlikely (__g1 != 0)) 				\
>> 
>> This change is inconsistent with the other updates, which use __g1 ==
>> -1.  Is this deliberate?
>> 
>> Thanks,
>> Florian
>> 
>
> In fact __SYSCALL_STRING already sets the 'o0' to a negative value if
> the 'xcc' condition is set (indicating that the syscall has failed).
> The 'g1' check is superfluous, it will be always true since 'g1' will
> be either 0 or 1. 
>
> And both the set and check of 'g1' result is also superfluous, since 'o0' 
> will already hold all the required information.
>
> Below is an updated patch, checked on sparc64-linux-gnu and 
> sparcv9-linux-gnu.

I see, nice additional cleanup.

> It changes the sparc internal_syscall* macros to return a negative
> value instead the 'g1' register value on 'err' macro argument.
               ^ of                     ^ in the?


> The __SYSCALL_STRING macro is also changed to no set the 'g1'
                                                ^ not
> value, since 'o1' already holds all the required information
> to check if syscall has failed.
>
> The macro INTERNAL_SYSCALL_DECL is no longer required, and the
> INTERNAL_SYSCALL_ERROR_P follows the other Linux kABIS.  The
                          ^ macro?                 ^ kABIs
                (or drop the “the“ on the preceding line)

> redefinition of INTERNAL_VSYSCALL_CALL is also no longer
> required.
>
> Checked on sparc64-linux-gnu and sparcv9-linux-gnu. It fixes
> the sporadic issues on sparc32 where clock_nanosleep does not
> act as cancellation entrypoint.

I double-checked this against the kernel sources, and entry.S has
this:

ret_sys_call:
        ld      [%curptr + TI_FLAGS], %l6
        cmp     %o0, -ERESTART_RESTARTBLOCK
        ld      [%sp + STACKFRAME_SZ + PT_PSR], %g3
        set     PSR_C, %g2
        bgeu    1f

But ERESTART_RESTARTBLOCK is not 4095, so glibc with this change will
now treat certain internal kernel error codes as errors, while they
were previously reported as success.  This looks like a kernel bug, in
that ERESTART_RESTARTBLOCK was not updated when more error codes were
added.  On the other hand, these error codes should never leak into
userspace.

To me, your patch looks good.

  reply	other threads:[~2020-02-11 19:26 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-10 19:20 [PATCH 01/15] powerpc: Consolidate Linux syscall definition Adhemerval Zanella
2020-02-10 19:20 ` [PATCH 02/15] powerpc: Use Linux kABI for syscall return Adhemerval Zanella
2020-02-11 11:18   ` Florian Weimer
2020-02-11 12:14     ` Adhemerval Zanella
2020-02-11 12:31       ` Florian Weimer
2020-02-11 13:31         ` Adhemerval Zanella
2020-02-11 19:45           ` Florian Weimer
2020-02-11 19:45   ` Florian Weimer
2020-02-12 13:24     ` Adhemerval Zanella
2020-02-10 19:20 ` [PATCH 03/15] sparc: " Adhemerval Zanella
2020-02-11 11:15   ` Florian Weimer
2020-02-11 18:55     ` Adhemerval Zanella
2020-02-11 19:24       ` Florian Weimer [this message]
2020-02-11 20:29         ` Adhemerval Zanella
2020-02-11 21:15           ` Florian Weimer
2020-02-12 12:35             ` Adhemerval Zanella
2020-02-12 12:38               ` Florian Weimer
2020-02-12 12:56                 ` Adhemerval Zanella
2020-02-10 19:20 ` [PATCH 04/15] alpha: Refactor syscall and " Adhemerval Zanella
2020-02-10 19:20 ` [PATCH 05/15] ia64: " Adhemerval Zanella
2020-02-10 19:20 ` [PATCH 06/15] mips64: Consolidate Linux sysdep.h Adhemerval Zanella
2020-02-10 22:48   ` Joseph Myers
2020-02-11 19:05     ` Adhemerval Zanella
2020-02-10 19:20 ` [PATCH 07/15] mips: Use Linux kABI for syscall return Adhemerval Zanella
2020-02-10 19:20 ` [PATCH 08/15] nios2: " Adhemerval Zanella
2020-02-11 11:20   ` Florian Weimer
2020-02-11 19:09     ` Adhemerval Zanella
2020-02-11 11:50   ` Andreas Schwab
2020-02-19 21:40   ` Vineet Gupta
2020-02-20 13:14     ` Adhemerval Zanella
2020-02-20 20:39       ` Vineet Gupta
2020-02-20 21:04         ` Vineet Gupta
2020-02-27 17:49           ` Adhemerval Zanella
2020-02-10 19:20 ` [PATCH 09/15] microblaze: Avoid clobbering register parameters in syscall Adhemerval Zanella
2020-02-11 11:21   ` Florian Weimer
2020-02-11 19:10     ` Adhemerval Zanella
2020-02-10 19:20 ` [PATCH 10/15] riscv: " Adhemerval Zanella
2020-02-10 19:51   ` DJ Delorie
2020-02-10 21:27     ` Palmer Dabbelt
2020-02-10 21:55       ` Adhemerval Zanella
2020-02-10 19:20 ` [PATCH 11/15] s390: Consolidate Linux syscall definition Adhemerval Zanella
2020-02-10 19:20 ` [PATCH 12/15] sparc: Avoid clobbering register parameters in syscall Adhemerval Zanella
2020-02-11 11:22   ` Florian Weimer
2020-02-11 19:17     ` Adhemerval Zanella
2020-02-10 19:20 ` [PATCH 13/15] linux: Consolidate INLINE_SYSCALL Adhemerval Zanella
2020-02-11 12:03   ` Florian Weimer
2020-02-11 20:53     ` Adhemerval Zanella
2020-02-11 21:00       ` Florian Weimer
2020-02-10 19:20 ` [PATCH 14/15] nptl: Remove ununsed pthread-errnos.h rule Adhemerval Zanella
2020-02-11 11:23   ` Florian Weimer
2020-02-11 11:51     ` Florian Weimer
2020-02-11 21:01       ` Adhemerval Zanella
2020-02-10 19:20 ` [PATCH 15/15] linux: Remove INTERNAL_SYSCALL_DECL Adhemerval Zanella
2020-02-11 12:34   ` Florian Weimer
2020-02-11 12:36   ` Florian Weimer
2020-02-11 20:57     ` Adhemerval Zanella
2020-02-11 12:48   ` Florian Weimer
2020-02-11 20:55     ` Adhemerval Zanella
2020-02-15  7:51   ` Andreas Schwab
2020-02-15  9:32     ` [PATCH] arm: fix use of INTERNAL_SYSCALL_CALL Andreas Schwab
2020-02-15  9:55       ` Florian Weimer
2020-02-11 19:43 ` [PATCH 01/15] powerpc: Consolidate Linux syscall definition Florian Weimer
2020-02-12 13:19   ` Adhemerval Zanella

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/libc/involved.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=874kvxnczu.fsf@mid.deneb.enyo.de \
    --to=fw@deneb.enyo.de \
    --cc=adhemerval.zanella@linaro.org \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).