bug-guix
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#46981: Severe Emacs shell mode performance regression in recent Linu


From: Mark H Weaver
Subject: bug#46981: Severe Emacs shell mode performance regression in recent Linux-libre
Date: Sat, 06 Mar 2021 21:56:28 -0500

Mark H Weaver <mhw@netris.org> writes:

> I've since confirmed that reverting the two upstream commits mentioned
> in <https://bugs.gnu.org/46978> fixes the performance regression.  I've
> attached a preliminary patch for Guix that reverts those upstream
> commits for linux-libre@5.10, and which I've tested on my system.

Here's the patch.

      Mark

>From 9522f18387673de5bad51d783e25c7b05dbab7d7 Mon Sep 17 00:00:00 2001
From: Mark H Weaver <mhw@netris.org>
Date: Sat, 6 Mar 2021 18:56:10 -0500
Subject: [PATCH] gnu: linux-libre@5.10: Revert recent changes to TTY
 subsystem.

This fixes a performance regression in Emacs shell mode.

* gnu/packages/patches/linux-libre-revert-tty-changes-1.patch,
gnu/packages/patches/linux-libre-revert-tty-changes-2.patch: New files.
* gnu/local.mk (dist_patch_DATA): Add them.
* gnu/packages/linux.scm (%linux-libre-revert-tty-changes-1-patch)
(%linux-libre-revert-tty-changes-2-patch): New variables.
(linux-libre-5.10-source): Add the patches.
---
 gnu/local.mk                                  |   2 +
 gnu/packages/linux.scm                        |  10 +-
 .../linux-libre-revert-tty-changes-1.patch    | 144 ++++
 .../linux-libre-revert-tty-changes-2.patch    | 667 ++++++++++++++++++
 4 files changed, 822 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/linux-libre-revert-tty-changes-1.patch
 create mode 100644 gnu/packages/patches/linux-libre-revert-tty-changes-2.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 477853cd53..9b4f3395c7 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1318,6 +1318,8 @@ dist_patch_DATA =                                         
\
   %D%/packages/patches/linbox-fix-pkgconfig.patch              \
   %D%/packages/patches/linkchecker-tests-require-network.patch \
   %D%/packages/patches/linphoneqt-tabbutton.patch              \
+  %D%/packages/patches/linux-libre-revert-tty-changes-1.patch  \
+  %D%/packages/patches/linux-libre-revert-tty-changes-2.patch  \
   %D%/packages/patches/linux-libre-support-for-Pinebook-Pro.patch \
   %D%/packages/patches/linux-pam-no-setfsuid.patch             \
   %D%/packages/patches/lirc-localstatedir.patch                        \
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 5b5cb8ea8d..da1afe4648 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -473,6 +473,12 @@ corresponding UPSTREAM-SOURCE (an origin), using the given 
DEBLOB-SCRIPTS."
     (sha256
      (base32 "1ifnfhpakzffn4b8n7x7w5cps9mzjxlkcfz9zqak2vaw8nzvl39f"))))
 
+(define %linux-libre-revert-tty-changes-1-patch
+  (search-patch "linux-libre-revert-tty-changes-1.patch"))
+
+(define %linux-libre-revert-tty-changes-2-patch
+  (search-patch "linux-libre-revert-tty-changes-2.patch"))
+
 (define (source-with-patches source patches)
   (origin
     (inherit source)
@@ -487,7 +493,9 @@ corresponding UPSTREAM-SOURCE (an origin), using the given 
DEBLOB-SCRIPTS."
 (define-public linux-libre-5.10-source
   (source-with-patches linux-libre-5.10-pristine-source
                        (list %boot-logo-patch
-                             
%linux-libre-arm-export-__sync_icache_dcache-patch)))
+                             %linux-libre-arm-export-__sync_icache_dcache-patch
+                             %linux-libre-revert-tty-changes-1-patch
+                             %linux-libre-revert-tty-changes-2-patch)))
 
 (define-public linux-libre-5.4-source
   (source-with-patches linux-libre-5.4-pristine-source
diff --git a/gnu/packages/patches/linux-libre-revert-tty-changes-1.patch 
b/gnu/packages/patches/linux-libre-revert-tty-changes-1.patch
new file mode 100644
index 0000000000..76c0974f87
--- /dev/null
+++ b/gnu/packages/patches/linux-libre-revert-tty-changes-1.patch
@@ -0,0 +1,144 @@
+Revert the following upstream patch, which dramatically decreased the
+performance of shell buffers in Emacs.
+
+
+From 41c6f6b926d0e712d0321f8a8f6511fea748e814 Mon Sep 17 00:00:00 2001
+From: Linus Torvalds <torvalds@linux-foundation.org>
+Date: Tue, 19 Jan 2021 10:49:19 -0800
+Subject: tty: implement read_iter
+
+[ Upstream commit dd78b0c483e33225e0e0782b0ed887129b00f956 ]
+
+Now that the ldisc read() function takes kernel pointers, it's fairly
+straightforward to make the tty file operations use .read_iter() instead
+of .read().
+
+That automatically gives us vread() and friends, and also makes it
+possible to do .splice_read() on ttys again.
+
+Fixes: 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops")
+Reported-by: Oliver Giles <ohw.giles@gmail.com>
+Cc: Christoph Hellwig <hch@lst.de>
+Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+Cc: Al Viro <viro@zeniv.linux.org.uk>
+Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
+Signed-off-by: Sasha Levin <sashal@kernel.org>
+---
+ drivers/tty/tty_io.c | 36 ++++++++++++++++++------------------
+ 1 file changed, 18 insertions(+), 18 deletions(-)
+
+diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
+index a50c8a4318228..3f55fe7293f31 100644
+--- b/drivers/tty/tty_io.c
++++ a/drivers/tty/tty_io.c
+@@ -142,7 +142,7 @@
+ /* Mutex to protect creating and releasing a tty */
+ DEFINE_MUTEX(tty_mutex);
+ 
+-static ssize_t tty_read(struct kiocb *, struct iov_iter *);
++static ssize_t tty_read(struct file *, char __user *, size_t, loff_t *);
+ static ssize_t tty_write(struct kiocb *, struct iov_iter *);
+ static __poll_t tty_poll(struct file *, poll_table *);
+ static int tty_open(struct inode *, struct file *);
+@@ -473,9 +473,8 @@
+ 
+ static const struct file_operations tty_fops = {
+       .llseek         = no_llseek,
+-      .read_iter      = tty_read,
++      .read           = tty_read,
+       .write_iter     = tty_write,
+-      .splice_read    = generic_file_splice_read,
+       .splice_write   = iter_file_splice_write,
+       .poll           = tty_poll,
+       .unlocked_ioctl = tty_ioctl,
+@@ -488,9 +487,8 @@
+ 
+ static const struct file_operations console_fops = {
+       .llseek         = no_llseek,
+-      .read_iter      = tty_read,
++      .read           = tty_read,
+       .write_iter     = redirected_tty_write,
+-      .splice_read    = generic_file_splice_read,
+       .splice_write   = iter_file_splice_write,
+       .poll           = tty_poll,
+       .unlocked_ioctl = tty_ioctl,
+@@ -843,17 +841,16 @@
+  * data or clears the cookie. The cookie may be something that the
+  * ldisc maintains state for and needs to free.
+  */
+-static int iterate_tty_read(struct tty_ldisc *ld, struct tty_struct *tty,
+-              struct file *file, struct iov_iter *to)
++static int iterate_tty_read(struct tty_ldisc *ld, struct tty_struct *tty, 
struct file *file,
++              char __user *buf, size_t count)
+ {
+       int retval = 0;
+       void *cookie = NULL;
+       unsigned long offset = 0;
+       char kernel_buf[64];
+-      size_t count = iov_iter_count(to);
+ 
+       do {
+-              int size, copied;
++              int size, uncopied;
+ 
+               size = count > sizeof(kernel_buf) ? sizeof(kernel_buf) : count;
+               size = ld->ops->read(tty, file, kernel_buf, size, &cookie, 
offset);
+@@ -869,9 +866,10 @@
+                       return size;
+               }
+ 
+-              copied = copy_to_iter(kernel_buf, size, to);
+-              offset += copied;
+-              count -= copied;
++              uncopied = copy_to_user(buf+offset, kernel_buf, size);
++              size -= uncopied;
++              offset += size;
++              count -= size;
+ 
+               /*
+                * If the user copy failed, we still need to do another ->read()
+@@ -879,7 +877,7 @@
+                *
+                * But make sure size is zeroed.
+                */
+-              if (unlikely(copied != size)) {
++              if (unlikely(uncopied)) {
+                       count = 0;
+                       retval = -EFAULT;
+               }
+@@ -906,10 +904,10 @@
+  *    read calls may be outstanding in parallel.
+  */
+ 
+-static ssize_t tty_read(struct kiocb *iocb, struct iov_iter *to)
++static ssize_t tty_read(struct file *file, char __user *buf, size_t count,
++                      loff_t *ppos)
+ {
+       int i;
+-      struct file *file = iocb->ki_filp;
+       struct inode *inode = file_inode(file);
+       struct tty_struct *tty = file_tty(file);
+       struct tty_ldisc *ld;
+@@ -922,9 +920,11 @@
+       /* We want to wait for the line discipline to sort out in this
+          situation */
+       ld = tty_ldisc_ref_wait(tty);
++      if (!ld)
++              return hung_up_tty_read(file, buf, count, ppos);
+       i = -EIO;
+-      if (ld && ld->ops->read)
+-              i = iterate_tty_read(ld, tty, file, to);
++      if (ld->ops->read)
++              i = iterate_tty_read(ld, tty, file, buf, count);
+       tty_ldisc_deref(ld);
+ 
+       if (i > 0)
+@@ -2943,7 +2943,7 @@
+ 
+ static int this_tty(const void *t, struct file *file, unsigned fd)
+ {
+-      if (likely(file->f_op->read_iter != tty_read))
++      if (likely(file->f_op->read != tty_read))
+               return 0;
+       return file_tty(file) != t ? 0 : fd + 1;
+ }
diff --git a/gnu/packages/patches/linux-libre-revert-tty-changes-2.patch 
b/gnu/packages/patches/linux-libre-revert-tty-changes-2.patch
new file mode 100644
index 0000000000..9ef0ff477e
--- /dev/null
+++ b/gnu/packages/patches/linux-libre-revert-tty-changes-2.patch
@@ -0,0 +1,667 @@
+Revert the following upstream patch, which dramatically decreased the
+performance of shell buffers in Emacs.
+
+
+From 279e54536ddbb4dbd337fca74926b68651160043 Mon Sep 17 00:00:00 2001
+From: Linus Torvalds <torvalds@linux-foundation.org>
+Date: Mon, 18 Jan 2021 13:31:30 -0800
+Subject: tty: convert tty_ldisc_ops 'read()' function to take a kernel pointer
+
+[ Upstream commit 3b830a9c34d5897be07176ce4e6f2d75e2c8cfd7 ]
+
+The tty line discipline .read() function was passed the final user
+pointer destination as an argument, which doesn't match the 'write()'
+function, and makes it very inconvenient to do a splice method for
+ttys.
+
+This is a conversion to use a kernel buffer instead.
+
+NOTE! It does this by passing the tty line discipline ->read() function
+an additional "cookie" to fill in, and an offset into the cookie data.
+
+The line discipline can fill in the cookie data with its own private
+information, and then the reader will repeat the read until either the
+cookie is cleared or it runs out of data.
+
+The only real user of this is N_HDLC, which can use this to handle big
+packets, even if the kernel buffer is smaller than the whole packet.
+
+Cc: Christoph Hellwig <hch@lst.de>
+Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+Cc: Al Viro <viro@zeniv.linux.org.uk>
+Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
+Signed-off-by: Sasha Levin <sashal@kernel.org>
+---
+ drivers/bluetooth/hci_ldisc.c | 34 +++++++++---------
+ drivers/input/serio/serport.c |  4 ++-
+ drivers/net/ppp/ppp_async.c   |  3 +-
+ drivers/net/ppp/ppp_synctty.c |  3 +-
+ drivers/tty/n_gsm.c           |  3 +-
+ drivers/tty/n_hdlc.c          | 60 +++++++++++++++++++++----------
+ drivers/tty/n_null.c          |  3 +-
+ drivers/tty/n_r3964.c         | 10 +++---
+ drivers/tty/n_tracerouter.c   |  4 ++-
+ drivers/tty/n_tracesink.c     |  4 ++-
+ drivers/tty/n_tty.c           | 82 ++++++++++++++++++-------------------------
+ drivers/tty/tty_io.c          | 64 +++++++++++++++++++++++++++++++--
+ include/linux/tty_ldisc.h     |  3 +-
+ net/nfc/nci/uart.c            |  3 +-
+ 14 files changed, 178 insertions(+), 102 deletions(-)
+
+diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
+index 8be4d807d1370..637c5b8c2aa1a 100644
+--- b/drivers/bluetooth/hci_ldisc.c
++++ a/drivers/bluetooth/hci_ldisc.c
+@@ -801,8 +801,7 @@
+  * We don't provide read/write/poll interface for user space.
+  */
+ static ssize_t hci_uart_tty_read(struct tty_struct *tty, struct file *file,
+-                               unsigned char *buf, size_t nr,
+-                               void **cookie, unsigned long offset)
++                               unsigned char __user *buf, size_t nr)
+ {
+       return 0;
+ }
+@@ -819,28 +818,29 @@
+       return 0;
+ }
+ 
+-static struct tty_ldisc_ops hci_uart_ldisc = {
+-      .owner          = THIS_MODULE,
+-      .magic          = TTY_LDISC_MAGIC,
+-      .name           = "n_hci",
+-      .open           = hci_uart_tty_open,
+-      .close          = hci_uart_tty_close,
+-      .read           = hci_uart_tty_read,
+-      .write          = hci_uart_tty_write,
+-      .ioctl          = hci_uart_tty_ioctl,
+-      .compat_ioctl   = hci_uart_tty_ioctl,
+-      .poll           = hci_uart_tty_poll,
+-      .receive_buf    = hci_uart_tty_receive,
+-      .write_wakeup   = hci_uart_tty_wakeup,
+-};
+-
+ static int __init hci_uart_init(void)
+ {
++      static struct tty_ldisc_ops hci_uart_ldisc;
+       int err;
+ 
+       BT_INFO("HCI UART driver ver %s", VERSION);
+ 
+       /* Register the tty discipline */
++
++      memset(&hci_uart_ldisc, 0, sizeof(hci_uart_ldisc));
++      hci_uart_ldisc.magic            = TTY_LDISC_MAGIC;
++      hci_uart_ldisc.name             = "n_hci";
++      hci_uart_ldisc.open             = hci_uart_tty_open;
++      hci_uart_ldisc.close            = hci_uart_tty_close;
++      hci_uart_ldisc.read             = hci_uart_tty_read;
++      hci_uart_ldisc.write            = hci_uart_tty_write;
++      hci_uart_ldisc.ioctl            = hci_uart_tty_ioctl;
++      hci_uart_ldisc.compat_ioctl     = hci_uart_tty_ioctl;
++      hci_uart_ldisc.poll             = hci_uart_tty_poll;
++      hci_uart_ldisc.receive_buf      = hci_uart_tty_receive;
++      hci_uart_ldisc.write_wakeup     = hci_uart_tty_wakeup;
++      hci_uart_ldisc.owner            = THIS_MODULE;
++
+       err = tty_register_ldisc(N_HCI, &hci_uart_ldisc);
+       if (err) {
+               BT_ERR("HCI line discipline registration failed. (%d)", err);
+diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
+index 8ac970a423de6..33e9d9bfd036f 100644
+--- b/drivers/input/serio/serport.c
++++ a/drivers/input/serio/serport.c
+@@ -156,9 +156,7 @@
+  * returning 0 characters.
+  */
+ 
+-static ssize_t serport_ldisc_read(struct tty_struct * tty, struct file * file,
+-                                unsigned char *kbuf, size_t nr,
+-                                void **cookie, unsigned long offset)
++static ssize_t serport_ldisc_read(struct tty_struct * tty, struct file * 
file, unsigned char __user * buf, size_t nr)
+ {
+       struct serport *serport = (struct serport*) tty->disc_data;
+       struct serio *serio;
+diff --git a/drivers/net/ppp/ppp_async.c b/drivers/net/ppp/ppp_async.c
+index 29a0917a81e60..f14a9d190de91 100644
+--- b/drivers/net/ppp/ppp_async.c
++++ a/drivers/net/ppp/ppp_async.c
+@@ -259,8 +259,7 @@
+  */
+ static ssize_t
+ ppp_asynctty_read(struct tty_struct *tty, struct file *file,
+-                unsigned char *buf, size_t count,
+-                void **cookie, unsigned long offset)
++                unsigned char __user *buf, size_t count)
+ {
+       return -EAGAIN;
+ }
+diff --git a/drivers/net/ppp/ppp_synctty.c b/drivers/net/ppp/ppp_synctty.c
+index 0f338752c38b9..f774b7e52da44 100644
+--- b/drivers/net/ppp/ppp_synctty.c
++++ a/drivers/net/ppp/ppp_synctty.c
+@@ -257,8 +257,7 @@
+  */
+ static ssize_t
+ ppp_sync_read(struct tty_struct *tty, struct file *file,
+-            unsigned char *buf, size_t count,
+-            void **cookie, unsigned long offset)
++             unsigned char __user *buf, size_t count)
+ {
+       return -EAGAIN;
+ }
+diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
+index 25f3152089c2a..fea1eeac5b907 100644
+--- b/drivers/tty/n_gsm.c
++++ a/drivers/tty/n_gsm.c
+@@ -2557,8 +2557,7 @@
+  */
+ 
+ static ssize_t gsmld_read(struct tty_struct *tty, struct file *file,
+-                        unsigned char *buf, size_t nr,
+-                        void **cookie, unsigned long offset)
++                       unsigned char __user *buf, size_t nr)
+ {
+       return -EOPNOTSUPP;
+ }
+diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
+index 12557ee1edb68..1363e659dc1db 100644
+--- b/drivers/tty/n_hdlc.c
++++ a/drivers/tty/n_hdlc.c
+@@ -416,19 +416,13 @@
+  * Returns the number of bytes returned or error code.
+  */
+ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
+-                         __u8 *kbuf, size_t nr,
+-                         void **cookie, unsigned long offset)
++                         __u8 __user *buf, size_t nr)
+ {
+       struct n_hdlc *n_hdlc = tty->disc_data;
+       int ret = 0;
+       struct n_hdlc_buf *rbuf;
+       DECLARE_WAITQUEUE(wait, current);
+ 
+-      /* Is this a repeated call for an rbuf we already found earlier? */
+-      rbuf = *cookie;
+-      if (rbuf)
+-              goto have_rbuf;
+-
+       add_wait_queue(&tty->read_wait, &wait);
+ 
+       for (;;) {
+@@ -442,8 +436,25 @@
+               set_current_state(TASK_INTERRUPTIBLE);
+ 
+               rbuf = n_hdlc_buf_get(&n_hdlc->rx_buf_list);
+-              if (rbuf)
++              if (rbuf) {
++                      if (rbuf->count > nr) {
++                              /* too large for caller's buffer */
++                              ret = -EOVERFLOW;
++                      } else {
++                              __set_current_state(TASK_RUNNING);
++                              if (copy_to_user(buf, rbuf->buf, rbuf->count))
++                                      ret = -EFAULT;
++                              else
++                                      ret = rbuf->count;
++                      }
++
++                      if (n_hdlc->rx_free_buf_list.count >
++                          DEFAULT_RX_BUF_COUNT)
++                              kfree(rbuf);
++                      else
++                              n_hdlc_buf_put(&n_hdlc->rx_free_buf_list, rbuf);
+                       break;
++              }
+ 
+               /* no data */
+               if (tty_io_nonblock(tty, file)) {
+@@ -462,39 +473,6 @@
+       remove_wait_queue(&tty->read_wait, &wait);
+       __set_current_state(TASK_RUNNING);
+ 
+-      if (!rbuf)
+-              return ret;
+-      *cookie = rbuf;
+-
+-have_rbuf:
+-      /* Have we used it up entirely? */
+-      if (offset >= rbuf->count)
+-              goto done_with_rbuf;
+-
+-      /* More data to go, but can't copy any more? EOVERFLOW */
+-      ret = -EOVERFLOW;
+-      if (!nr)
+-              goto done_with_rbuf;
+-
+-      /* Copy as much data as possible */
+-      ret = rbuf->count - offset;
+-      if (ret > nr)
+-              ret = nr;
+-      memcpy(kbuf, rbuf->buf+offset, ret);
+-      offset += ret;
+-
+-      /* If we still have data left, we leave the rbuf in the cookie */
+-      if (offset < rbuf->count)
+-              return ret;
+-
+-done_with_rbuf:
+-      *cookie = NULL;
+-
+-      if (n_hdlc->rx_free_buf_list.count > DEFAULT_RX_BUF_COUNT)
+-              kfree(rbuf);
+-      else
+-              n_hdlc_buf_put(&n_hdlc->rx_free_buf_list, rbuf);
+-
+       return ret;
+ 
+ }     /* end of n_hdlc_tty_read() */
+diff --git a/drivers/tty/n_null.c b/drivers/tty/n_null.c
+index 96feabae47407..ce03ae78f5c6a 100644
+--- b/drivers/tty/n_null.c
++++ a/drivers/tty/n_null.c
+@@ -20,8 +20,7 @@
+ }
+ 
+ static ssize_t n_null_read(struct tty_struct *tty, struct file *file,
+-                         unsigned char *buf, size_t nr,
+-                         void **cookie, unsigned long offset)
++                         unsigned char __user * buf, size_t nr)
+ {
+       return -EOPNOTSUPP;
+ }
+diff --git a/drivers/tty/n_r3964.c b/drivers/tty/n_r3964.c
+index 934dd2fb2ec80..3161f0a535e37 100644
+--- b/drivers/tty/n_r3964.c
++++ a/drivers/tty/n_r3964.c
+@@ -129,7 +129,7 @@
+ static int r3964_open(struct tty_struct *tty);
+ static void r3964_close(struct tty_struct *tty);
+ static ssize_t r3964_read(struct tty_struct *tty, struct file *file,
+-              void *cookie, unsigned char *buf, size_t nr);
++              unsigned char __user * buf, size_t nr);
+ static ssize_t r3964_write(struct tty_struct *tty, struct file *file,
+               const unsigned char *buf, size_t nr);
+ static int r3964_ioctl(struct tty_struct *tty, struct file *file,
+@@ -1058,8 +1058,7 @@
+ }
+ 
+ static ssize_t r3964_read(struct tty_struct *tty, struct file *file,
+-                        unsigned char *kbuf, size_t nr,
+-                        void **cookie, unsigned long offset)
++                        unsigned char __user * buf, size_t nr)
+ {
+       struct r3964_info *pInfo = tty->disc_data;
+       struct r3964_client_info *pClient;
+@@ -1110,7 +1109,10 @@
+               kfree(pMsg);
+               TRACE_M("r3964_read - msg kfree %p", pMsg);
+ 
+-              memcpy(kbuf, &theMsg, ret);
++              if (copy_to_user(buf, &theMsg, ret)) {
++                      ret = -EFAULT;
++                      goto unlock;
++              }
+ 
+               TRACE_PS("read - return %d", ret);
+               goto unlock;
+diff --git a/drivers/tty/n_tracerouter.c b/drivers/tty/n_tracerouter.c
+index 4479af4d2fa5c..3490ed51b1a3c 100644
+--- b/drivers/tty/n_tracerouter.c
++++ a/drivers/tty/n_tracerouter.c
+@@ -118,9 +118,7 @@
+  *     -EINVAL
+  */
+ static ssize_t n_tracerouter_read(struct tty_struct *tty, struct file *file,
+-                                unsigned char *buf, size_t nr,
+-                                void **cookie, unsigned long offset)
+-{
++                                unsigned char __user *buf, size_t nr) {
+       return -EINVAL;
+ }
+ 
+diff --git a/drivers/tty/n_tracesink.c b/drivers/tty/n_tracesink.c
+index d96ba82cc3569..1d9931041fd8b 100644
+--- b/drivers/tty/n_tracesink.c
++++ a/drivers/tty/n_tracesink.c
+@@ -115,9 +115,7 @@
+  *     -EINVAL
+  */
+ static ssize_t n_tracesink_read(struct tty_struct *tty, struct file *file,
+-                              unsigned char *buf, size_t nr,
+-                              void **cookie, unsigned long offset)
+-{
++                              unsigned char __user *buf, size_t nr) {
+       return -EINVAL;
+ }
+ 
+diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
+index c2869489ba681..e8963165082ee 100644
+--- b/drivers/tty/n_tty.c
++++ a/drivers/tty/n_tty.c
+@@ -164,24 +164,29 @@
+               memset(buffer, 0x00, size);
+ }
+ 
+-static void tty_copy(struct tty_struct *tty, void *to, size_t tail, size_t n)
++static int tty_copy_to_user(struct tty_struct *tty, void __user *to,
++                          size_t tail, size_t n)
+ {
+       struct n_tty_data *ldata = tty->disc_data;
+       size_t size = N_TTY_BUF_SIZE - tail;
+       void *from = read_buf_addr(ldata, tail);
++      int uncopied;
+ 
+       if (n > size) {
+               tty_audit_add_data(tty, from, size);
+-              memcpy(to, from, size);
+-              zero_buffer(tty, from, size);
++              uncopied = copy_to_user(to, from, size);
++              zero_buffer(tty, from, size - uncopied);
++              if (uncopied)
++                      return uncopied;
+               to += size;
+               n -= size;
+               from = ldata->read_buf;
+       }
+ 
+       tty_audit_add_data(tty, from, n);
+-      memcpy(to, from, n);
+-      zero_buffer(tty, from, n);
++      uncopied = copy_to_user(to, from, n);
++      zero_buffer(tty, from, n - uncopied);
++      return uncopied;
+ }
+ 
+ /**
+@@ -1937,16 +1942,15 @@
+ /**
+  *    copy_from_read_buf      -       copy read data directly
+  *    @tty: terminal device
+- *    @kbp: data
++ *    @b: user data
+  *    @nr: size of data
+  *
+  *    Helper function to speed up n_tty_read.  It is only called when
+- *    ICANON is off; it copies characters straight from the tty queue.
+- *
+- *    It can be profitably called twice; once to drain the space from
+- *    the tail pointer to the (physical) end of the buffer, and once
+- *    to drain the space from the (physical) beginning of the buffer
+- *    to head pointer.
++ *    ICANON is off; it copies characters straight from the tty queue to
++ *    user space directly.  It can be profitably called twice; once to
++ *    drain the space from the tail pointer to the (physical) end of the
++ *    buffer, and once to drain the space from the (physical) beginning of
++ *    the buffer to head pointer.
+  *
+  *    Called under the ldata->atomic_read_lock sem
+  *
+@@ -1956,7 +1960,7 @@
+  */
+ 
+ static int copy_from_read_buf(struct tty_struct *tty,
+-                                    unsigned char **kbp,
++                                    unsigned char __user **b,
+                                     size_t *nr)
+ 
+ {
+@@ -1972,7 +1976,8 @@
+       n = min(*nr, n);
+       if (n) {
+               unsigned char *from = read_buf_addr(ldata, tail);
+-              memcpy(*kbp, from, n);
++              retval = copy_to_user(*b, from, n);
++              n -= retval;
+               is_eof = n == 1 && *from == EOF_CHAR(tty);
+               tty_audit_add_data(tty, from, n);
+               zero_buffer(tty, from, n);
+@@ -1981,7 +1986,7 @@
+               if (L_EXTPROC(tty) && ldata->icanon && is_eof &&
+                   (head == ldata->read_tail))
+                       n = 0;
+-              *kbp += n;
++              *b += n;
+               *nr -= n;
+       }
+       return retval;
+@@ -1990,12 +1995,12 @@
+ /**
+  *    canon_copy_from_read_buf        -       copy read data in canonical mode
+  *    @tty: terminal device
+- *    @kbp: data
++ *    @b: user data
+  *    @nr: size of data
+  *
+  *    Helper function for n_tty_read.  It is only called when ICANON is on;
+  *    it copies one line of input up to and including the line-delimiting
+- *    character into the result buffer.
++ *    character into the user-space buffer.
+  *
+  *    NB: When termios is changed from non-canonical to canonical mode and
+  *    the read buffer contains data, n_tty_set_termios() simulates an EOF
+@@ -2011,14 +2016,14 @@
+  */
+ 
+ static int canon_copy_from_read_buf(struct tty_struct *tty,
+-                                  unsigned char **kbp,
++                                  unsigned char __user **b,
+                                   size_t *nr)
+ {
+       struct n_tty_data *ldata = tty->disc_data;
+       size_t n, size, more, c;
+       size_t eol;
+       size_t tail;
+-      int found = 0;
++      int ret, found = 0;
+ 
+       /* N.B. avoid overrun if nr == 0 */
+       if (!*nr)
+@@ -2054,8 +2059,10 @@
+       n_tty_trace("%s: eol:%zu found:%d n:%zu c:%zu tail:%zu more:%zu\n",
+                   __func__, eol, found, n, c, tail, more);
+ 
+-      tty_copy(tty, *kbp, tail, n);
+-      *kbp += n;
++      ret = tty_copy_to_user(tty, *b, tail, n);
++      if (ret)
++              return -EFAULT;
++      *b += n;
+       *nr -= n;
+ 
+       if (found)
+@@ -2120,11 +2127,10 @@
+  */
+ 
+ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
+-                        unsigned char *kbuf, size_t nr,
+-                        void **cookie, unsigned long offset)
++                       unsigned char __user *buf, size_t nr)
+ {
+       struct n_tty_data *ldata = tty->disc_data;
+-      unsigned char *kb = kbuf;
++      unsigned char __user *b = buf;
+       DEFINE_WAIT_FUNC(wait, woken_wake_function);
+       int c;
+       int minimum, time;
+@@ -2170,13 +2176,17 @@
+               /* First test for status change. */
+               if (packet && tty->link->ctrl_status) {
+                       unsigned char cs;
+-                      if (kb != kbuf)
++                      if (b != buf)
+                               break;
+                       spin_lock_irq(&tty->link->ctrl_lock);
+                       cs = tty->link->ctrl_status;
+                       tty->link->ctrl_status = 0;
+                       spin_unlock_irq(&tty->link->ctrl_lock);
+-                      *kb++ = cs;
++                      if (put_user(cs, b)) {
++                              retval = -EFAULT;
++                              break;
++                      }
++                      b++;
+                       nr--;
+                       break;
+               }
+@@ -2219,20 +2229,24 @@
+               }
+ 
+               if (ldata->icanon && !L_EXTPROC(tty)) {
+-                      retval = canon_copy_from_read_buf(tty, &kb, &nr);
++                      retval = canon_copy_from_read_buf(tty, &b, &nr);
+                       if (retval)
+                               break;
+               } else {
+                       int uncopied;
+ 
+                       /* Deal with packet mode. */
+-                      if (packet && kb == kbuf) {
+-                              *kb++ = TIOCPKT_DATA;
++                      if (packet && b == buf) {
++                              if (put_user(TIOCPKT_DATA, b)) {
++                                      retval = -EFAULT;
++                                      break;
++                              }
++                              b++;
+                               nr--;
+                       }
+ 
+-                      uncopied = copy_from_read_buf(tty, &kb, &nr);
+-                      uncopied += copy_from_read_buf(tty, &kb, &nr);
++                      uncopied = copy_from_read_buf(tty, &b, &nr);
++                      uncopied += copy_from_read_buf(tty, &b, &nr);
+                       if (uncopied) {
+                               retval = -EFAULT;
+                               break;
+@@ -2241,7 +2255,7 @@
+ 
+               n_tty_check_unthrottle(tty);
+ 
+-              if (kb - kbuf >= minimum)
++              if (b - buf >= minimum)
+                       break;
+               if (time)
+                       timeout = time;
+@@ -2253,8 +2267,8 @@
+       remove_wait_queue(&tty->read_wait, &wait);
+       mutex_unlock(&ldata->atomic_read_lock);
+ 
+-      if (kb - kbuf)
+-              retval = kb - kbuf;
++      if (b - buf)
++              retval = b - buf;
+ 
+       return retval;
+ }
+diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
+index 21cd5ac6ca8b5..a50c8a4318228 100644
+--- b/drivers/tty/tty_io.c
++++ a/drivers/tty/tty_io.c
+@@ -830,65 +830,6 @@
+               time->tv_sec = sec;
+ }
+ 
+-/*
+- * Iterate on the ldisc ->read() function until we've gotten all
+- * the data the ldisc has for us.
+- *
+- * The "cookie" is something that the ldisc read function can fill
+- * in to let us know that there is more data to be had.
+- *
+- * We promise to continue to call the ldisc until it stops returning
+- * data or clears the cookie. The cookie may be something that the
+- * ldisc maintains state for and needs to free.
+- */
+-static int iterate_tty_read(struct tty_ldisc *ld, struct tty_struct *tty, 
struct file *file,
+-              char __user *buf, size_t count)
+-{
+-      int retval = 0;
+-      void *cookie = NULL;
+-      unsigned long offset = 0;
+-      char kernel_buf[64];
+-
+-      do {
+-              int size, uncopied;
+-
+-              size = count > sizeof(kernel_buf) ? sizeof(kernel_buf) : count;
+-              size = ld->ops->read(tty, file, kernel_buf, size, &cookie, 
offset);
+-              if (!size)
+-                      break;
+-
+-              /*
+-               * A ldisc read error return will override any previously copied
+-               * data (eg -EOVERFLOW from HDLC)
+-               */
+-              if (size < 0) {
+-                      memzero_explicit(kernel_buf, sizeof(kernel_buf));
+-                      return size;
+-              }
+-
+-              uncopied = copy_to_user(buf+offset, kernel_buf, size);
+-              size -= uncopied;
+-              offset += size;
+-              count -= size;
+-
+-              /*
+-               * If the user copy failed, we still need to do another ->read()
+-               * call if we had a cookie to let the ldisc clear up.
+-               *
+-               * But make sure size is zeroed.
+-               */
+-              if (unlikely(uncopied)) {
+-                      count = 0;
+-                      retval = -EFAULT;
+-              }
+-      } while (cookie);
+-
+-      /* We always clear tty buffer in case they contained passwords */
+-      memzero_explicit(kernel_buf, sizeof(kernel_buf));
+-      return offset ? offset : retval;
+-}
+-
+-
+ /**
+  *    tty_read        -       read method for tty device files
+  *    @file: pointer to tty file
+@@ -922,9 +863,10 @@
+       ld = tty_ldisc_ref_wait(tty);
+       if (!ld)
+               return hung_up_tty_read(file, buf, count, ppos);
+-      i = -EIO;
+       if (ld->ops->read)
+-              i = iterate_tty_read(ld, tty, file, buf, count);
++              i = ld->ops->read(tty, file, buf, count);
++      else
++              i = -EIO;
+       tty_ldisc_deref(ld);
+ 
+       if (i > 0)
+diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
+index b1e6043e99175..572a079761165 100644
+--- b/include/linux/tty_ldisc.h
++++ a/include/linux/tty_ldisc.h
+@@ -185,8 +185,7 @@
+       void    (*close)(struct tty_struct *);
+       void    (*flush_buffer)(struct tty_struct *tty);
+       ssize_t (*read)(struct tty_struct *tty, struct file *file,
+-                      unsigned char *buf, size_t nr,
+-                      void **cookie, unsigned long offset);
++                      unsigned char __user *buf, size_t nr);
+       ssize_t (*write)(struct tty_struct *tty, struct file *file,
+                        const unsigned char *buf, size_t nr);
+       int     (*ioctl)(struct tty_struct *tty, struct file *file,
+diff --git a/net/nfc/nci/uart.c b/net/nfc/nci/uart.c
+index 11b554ce07ffc..1204c438e87dc 100644
+--- b/net/nfc/nci/uart.c
++++ a/net/nfc/nci/uart.c
+@@ -292,8 +292,7 @@
+ 
+ /* We don't provide read/write/poll interface for user space. */
+ static ssize_t nci_uart_tty_read(struct tty_struct *tty, struct file *file,
+-                               unsigned char *buf, size_t nr,
+-                               void **cookie, unsigned long offset)
++                               unsigned char __user *buf, size_t nr)
+ {
+       return 0;
+ }
-- 
2.30.1


reply via email to

[Prev in Thread] Current Thread [Next in Thread]