Hi, Al,

On Sun, Oct 3, 2021 at 10:50 AM Al Viro [off-list ref] wrote:

On Mon, Sep 27, 2021 at 02:42:48PM +0800, Huacai Chen wrote:

quoted

+/*
+ * Does the process account for user or for system time?
+ */
+#define user_mode(regs) (((regs)->csr_prmd & PLV_MASK) == PLV_USER)
+
+static inline int is_syscall_success(struct pt_regs *regs)
+{
+     return !regs->regs[7];
+}

+static inline long regs_return_value(struct pt_regs *regs)
+{
+     if (is_syscall_success(regs) || !user_mode(regs))
+             return regs->regs[4];
+     else
+             return -regs->regs[4];
+}

Huh???  That looks like you've copied those from MIPS, but on MIPS we have
things like
        li      t0, -EMAXERRNO - 1      # error?
        sltu    t0, t0, v0
        sd      t0, PT_R7(sp)           # set error flag
        beqz    t0, 1f

        ld      t1, PT_R2(sp)           # syscall number
        dnegu   v0                      # error
        sd      t1, PT_R0(sp)           # save it for syscall restarting
1:      sd      v0, PT_R2(sp)           # result
right after the call of sys_...(), along with the restart logics
looking like
        if (regs->regs[0]) {
                switch(regs->regs[2]) {
                case ERESTART_RESTARTBLOCK:
                case ERESTARTNOHAND:
IOW, syscall return values from -EMAXERRNO to -1 are negated, with
regs[7] set accordingly.  Nothing of that sort is done in your
patchset after syscall, and if it had been, your restart logics in
signal handling would've been wrong anyway.

What's going on there?

Sorry, the code derived from MIPS is wrong here, regs_return_value
should simply return regs->regs[4].

Huacai