bug#46981: Severe Emacs shell mode performance regression in recent Linux-libre

bug#46981: Severe Emacs shell mode performance regression in recent Linux-libre

[Mark H Weaver]

FYI, for those who use Emacs shell mode (M-x shell), I wanted to give a
heads-up about a severe performance regression in Linux-libre 5.10.20,
and I suspect the same regression exists in 5.11.3.

For details, see the bug report I submitted to the Emacs developers,
here: <https://bugs.gnu.org/46978>.

       Mark

bug#46981: Severe Emacs shell mode performance regression in recent Linux-libre

[Mark H Weaver]

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

> FYI, for those who use Emacs shell mode (M-x shell), I wanted to give a
> heads-up about a severe performance regression in Linux-libre 5.10.20,
> and I suspect the same regression exists in 5.11.3.
>
> For details, see the bug report I submitted to the Emacs developers,
> here: <https://bugs.gnu.org/46978>.

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.

I guess that the same fix is also needed for linux-libre@5.11, but I've
not checked.  I'll leave it to others to try applying the same reverts
to 5.11.3 if they wish, and to decide what (if anything) should be done
in Guix about this issue.

      Mark

bug#46981: Severe Emacs shell mode performance regression in recent Linux-libre

[Mark H Weaver]

[-- Attachment #1: Type: text/plain, Size: 355 bytes --]

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: [PATCH] gnu: linux-libre@5.10: Revert recent changes to TTY subsystem. --]
[-- Type: text/x-patch, Size: 28612 bytes --]

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

bug#46981: Severe Emacs shell mode performance regression in recent Linux-libre

[Bengt Richter]

Hi Mark, et?

On +2021-03-06 21:54:42 -0500, Mark H Weaver wrote:
> Mark H Weaver <mhw@netris.org> writes:
> 
> > FYI, for those who use Emacs shell mode (M-x shell), I wanted to give a
> > heads-up about a severe performance regression in Linux-libre 5.10.20,
> > and I suspect the same regression exists in 5.11.3.
> >
> > For details, see the bug report I submitted to the Emacs developers,
> > here: <https://bugs.gnu.org/46978>.
> 
> 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.
> 
> I guess that the same fix is also needed for linux-libre@5.11, but I've
> not checked.  I'll leave it to others to try applying the same reverts
> to 5.11.3 if they wish, and to decide what (if anything) should be done
> in Guix about this issue.
> 
>       Mark
> 
> 
> 

Is the vt infrastructure that emacs uses to do Mx shell well virtualized
so that e.g. a native wayland bash repl displaying tile could be plugged in?
(if not, why not? :)

Seems like an html-textarea-like wayland tile could be implemented to
present the bash-stdout for emacs, and also capture it in a shm-pool
for access by emacs as open file or mmap, to do its mode-dependent
indenting and highlighting, then sending that to another text-area vt tile.

Excuse the handwaving, but I think it could be done, from what I've read
about wayland clients and servers.

-- 
Regards,
Bengt Richter
PS. BTW: anybody thought about defining a wayland server to act as guix daemon?
     It already has async threadsafe event loop and manages access-isolated
     pools of memory, and you can define extended protocol for access.

     And on a wayland-based box, this would be available early.
     Early enough for hurd initialization to make use of, I think.
     Daydream? :)
     

bug#46981: Severe Emacs shell mode performance regression in recent Linux-libre

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 1153 bytes --]


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

> Mark H Weaver <mhw@netris.org> writes:
>
>> FYI, for those who use Emacs shell mode (M-x shell), I wanted to give a
>> heads-up about a severe performance regression in Linux-libre 5.10.20,
>> and I suspect the same regression exists in 5.11.3.
>>
>> For details, see the bug report I submitted to the Emacs developers,
>> here: <https://bugs.gnu.org/46978>.
>
> 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.
>
> I guess that the same fix is also needed for linux-libre@5.11, but I've
> not checked.  I'll leave it to others to try applying the same reverts
> to 5.11.3 if they wish, and to decide what (if anything) should be done
> in Guix about this issue.

Ah, thanks so much for investigating this Mark! I only got as far as
asking around on #emacs.

I can confirm that 5.11.3 is affected, and that reverting to 5.10.19
avoids the issue. I haven't tried your patch yet.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

bug#46981: Severe Emacs shell mode performance regression in recent Linux-libre

[Mark H Weaver]

The problem was fixed upstream in Linux-libre 5.10.21, and presumably
also in 5.11.4, so I'm closing this bug now.

      Mark

bug#46981: Severe Emacs shell mode performance regression in recent Linux-libre

[Ludovic Courtès]

Hi Mark,

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

> The problem was fixed upstream in Linux-libre 5.10.21, and presumably
> also in 5.11.4, so I'm closing this bug now.

Woow, that was far from an obvious fix.  Thanks a lot!

Ludo’.