Linux MIPS Architecture development
 help / color / mirror / Atom feed
From: tommy.christensen@eicon.com
To: Ralf Baechle <ralf@oss.sgi.com>
Cc: linux-mips@oss.sgi.com
Subject: Register allocation in copy_to_user
Date: Tue, 25 Sep 2001 20:51:03 +0200	[thread overview]
Message-ID: <3BB0D217.80E313F5@eicon.com> (raw)


For some time, I have seen occasional corruption of tty-output (pty's and
serial). This turned out to be caused by a register collision in read_chan
()
in n_tty.c. In the expansion of copy_to_user, the compiler chose register
"a0" to hold the value of local variable __cu_from. Since this register is
modified in the asm statement, before __cu_from is used, the corruption
occured.

I am not sure, whether this is a compiler-bug (egcs-2.91.66) or the code
should prevent this from happening. Have the semantics about side-effects
of asm statements changed?

Anyway, the attached patch solves this by explicitly building the arguments
to __copy_user in the argument registers ;-) instead of moving them around.
So it actually saves some instructions as well. And the compiler can
generate
better code since it now has more registers for temporary variables ...

Is this OK? It works just fine for me with a 2.4.9 kernel (VR5000).

-Tommy
--- include/asm-mips/uaccess.h.orig      Mon Aug  6 10:35:52 2001
+++ include/asm-mips/uaccess.h     Tue Sep 25 18:51:00 2001
@@ -270,97 +270,81 @@
 extern size_t __copy_user(void *__to, const void *__from, size_t __n);

 #define __copy_to_user(to,from,n) ({ \
-    void *__cu_to; \
-    const void *__cu_from; \
-    long __cu_len; \
+    register void *__cu_to __asm__ ("$4"); \
+    register const void *__cu_from __asm__ ("$5"); \
+    register long __cu_len __asm__ ("$6"); \
     \
     __cu_to = (to); \
     __cu_from = (from); \
     __cu_len = (n); \
     __asm__ __volatile__( \
-         "move\t$4, %1\n\t" \
-         "move\t$5, %2\n\t" \
-         "move\t$6, %3\n\t" \
          __MODULE_JAL(__copy_user) \
-         "move\t%0, $6" \
-         : "=r" (__cu_len) \
-         : "r" (__cu_to), "r" (__cu_from), "r" (__cu_len) \
-         : "$4", "$5", "$6", "$8", "$9", "$10", "$11", "$12", "$15", \
+         : "+r" (__cu_to), "+r" (__cu_from), "+r" (__cu_len) \
+         : \
+         : "$8", "$9", "$10", "$11", "$12", "$15", \
            "$24", "$31","memory"); \
     __cu_len; \
 })

 #define __copy_from_user(to,from,n) ({ \
-    void *__cu_to; \
-    const void *__cu_from; \
-    long __cu_len; \
+    register void *__cu_to __asm__ ("$4"); \
+    register const void *__cu_from __asm__ ("$5"); \
+    register long __cu_len __asm__ ("$6"); \
     \
     __cu_to = (to); \
     __cu_from = (from); \
     __cu_len = (n); \
     __asm__ __volatile__( \
-         "move\t$4, %1\n\t" \
-         "move\t$5, %2\n\t" \
-         "move\t$6, %3\n\t" \
          ".set\tnoreorder\n\t" \
          __MODULE_JAL(__copy_user) \
          ".set\tnoat\n\t" \
-         "addu\t$1, %2, %3\n\t" \
+         "addu\t$1, %1, %2\n\t" \
          ".set\tat\n\t" \
          ".set\treorder\n\t" \
-         "move\t%0, $6" \
-         : "=r" (__cu_len) \
-         : "r" (__cu_to), "r" (__cu_from), "r" (__cu_len) \
-         : "$4", "$5", "$6", "$8", "$9", "$10", "$11", "$12", "$15", \
+         : "+r" (__cu_to), "+r" (__cu_from), "+r" (__cu_len) \
+         : \
+         : "$8", "$9", "$10", "$11", "$12", "$15", \
            "$24", "$31","memory"); \
     __cu_len; \
 })

 #define copy_to_user(to,from,n) ({ \
-    void *__cu_to; \
-    const void *__cu_from; \
-    long __cu_len; \
+    register void *__cu_to __asm__ ("$4"); \
+    register const void *__cu_from __asm__ ("$5"); \
+    register long __cu_len __asm__ ("$6"); \
     \
     __cu_to = (to); \
     __cu_from = (from); \
     __cu_len = (n); \
     if (access_ok(VERIFY_WRITE, __cu_to, __cu_len)) \
          __asm__ __volatile__( \
-              "move\t$4, %1\n\t" \
-              "move\t$5, %2\n\t" \
-              "move\t$6, %3\n\t" \
               __MODULE_JAL(__copy_user) \
-              "move\t%0, $6" \
-              : "=r" (__cu_len) \
-              : "r" (__cu_to), "r" (__cu_from), "r" (__cu_len) \
-              : "$4", "$5", "$6", "$8", "$9", "$10", "$11", "$12", \
+              : "+r" (__cu_to), "+r" (__cu_from), "+r" (__cu_len) \
+              : \
+              : "$8", "$9", "$10", "$11", "$12", \
                 "$15", "$24", "$31","memory"); \
     __cu_len; \
 })

 #define copy_from_user(to,from,n) ({ \
-    void *__cu_to; \
-    const void *__cu_from; \
-    long __cu_len; \
+    register void *__cu_to __asm__ ("$4"); \
+    register const void *__cu_from __asm__ ("$5"); \
+    register long __cu_len __asm__ ("$6"); \
     \
     __cu_to = (to); \
     __cu_from = (from); \
     __cu_len = (n); \
     if (access_ok(VERIFY_READ, __cu_from, __cu_len)) \
          __asm__ __volatile__( \
-              "move\t$4, %1\n\t" \
-              "move\t$5, %2\n\t" \
-              "move\t$6, %3\n\t" \
               ".set\tnoreorder\n\t" \
               __MODULE_JAL(__copy_user) \
               ".set\tnoat\n\t" \
-              "addu\t$1, %2, %3\n\t" \
+              "addu\t$1, %1, %2\n\t" \
               ".set\tat\n\t" \
               ".set\treorder\n\t" \
-              "move\t%0, $6" \
-              : "=r" (__cu_len) \
-              : "r" (__cu_to), "r" (__cu_from), "r" (__cu_len) \
-              : "$4", "$5", "$6", "$8", "$9", "$10", "$11", "$12", \
+              : "+r" (__cu_to), "+r" (__cu_from), "+r" (__cu_len) \
+              : \
+              : "$8", "$9", "$10", "$11", "$12", \
                 "$15", "$24", "$31","memory"); \
     __cu_len; \
 })

             reply	other threads:[~2001-09-25 18:49 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-09-25 18:51 tommy.christensen [this message]
2001-09-25 22:11 ` Register allocation in copy_to_user Ralf Baechle
2001-10-04 15:11 ` tommy.christensen

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

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

  git send-email \
    --in-reply-to=3BB0D217.80E313F5@eicon.com \
    --to=tommy.christensen@eicon.com \
    --cc=linux-mips@oss.sgi.com \
    --cc=ralf@oss.sgi.com \
    /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