linux-stable.git/drivers/tty/n_tty.c, branch linux-5.15.y Linux kernel stable tree tty: fix hang on tty device with no_room set 2023-08-03T08:22:29+00:00 Hui Li caelli@tencent.com 2023-04-06T02:44:50+00:00 c556573e4bb16bb6d152b521caa2b29ef65b88bb [ Upstream commit 4903fde8047a28299d1fc79c1a0dcc255e928f12 ] It is possible to hang pty devices in this case, the reader was blocking at epoll on master side, the writer was sleeping at wait_woken inside n_tty_write on slave side, and the write buffer on tty_port was full, we found that the reader and writer would never be woken again and blocked forever. The problem was caused by a race between reader and kworker: n_tty_read(reader): n_tty_receive_buf_common(kworker): copy_from_read_buf()| |room = N_TTY_BUF_SIZE - (ldata->read_head - tail) |room <= 0 n_tty_kick_worker() | |ldata->no_room = true After writing to slave device, writer wakes up kworker to flush data on tty_port to reader, and the kworker finds that reader has no room to store data so room <= 0 is met. At this moment, reader consumes all the data on reader buffer and calls n_tty_kick_worker to check ldata->no_room which is false and reader quits reading. Then kworker sets ldata->no_room=true and quits too. If write buffer is not full, writer will wake kworker to flush data again after following writes, but if write buffer is full and writer goes to sleep, kworker will never be woken again and tty device is blocked. This problem can be solved with a check for read buffer size inside n_tty_receive_buf_common, if read buffer is empty and ldata->no_room is true, a call to n_tty_kick_worker is necessary to keep flushing data to reader. Cc: <stable@vger.kernel.org> Fixes: 42458f41d08f ("n_tty: Ensure reader restarts worker for next reader") Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Signed-off-by: Hui Li <caelli@tencent.com> Message-ID: <1680749090-14106-1-git-send-email-caelli@tencent.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Sasha Levin <sashal@kernel.org> 
[ Upstream commit 4903fde8047a28299d1fc79c1a0dcc255e928f12 ]

It is possible to hang pty devices in this case, the reader was
blocking at epoll on master side, the writer was sleeping at
wait_woken inside n_tty_write on slave side, and the write buffer
on tty_port was full, we found that the reader and writer would
never be woken again and blocked forever.

The problem was caused by a race between reader and kworker:
n_tty_read(reader):  n_tty_receive_buf_common(kworker):
copy_from_read_buf()|
                    |room = N_TTY_BUF_SIZE - (ldata->read_head - tail)
                    |room <= 0
n_tty_kick_worker() |
                    |ldata->no_room = true

After writing to slave device, writer wakes up kworker to flush
data on tty_port to reader, and the kworker finds that reader
has no room to store data so room <= 0 is met. At this moment,
reader consumes all the data on reader buffer and calls
n_tty_kick_worker to check ldata->no_room which is false and
reader quits reading. Then kworker sets ldata->no_room=true
and quits too.

If write buffer is not full, writer will wake kworker to flush data
again after following writes, but if write buffer is full and writer
goes to sleep, kworker will never be woken again and tty device is
blocked.

This problem can be solved with a check for read buffer size inside
n_tty_receive_buf_common, if read buffer is empty and ldata->no_room
is true, a call to n_tty_kick_worker is necessary to keep flushing
data to reader.

Cc: <stable@vger.kernel.org>
Fixes: 42458f41d08f ("n_tty: Ensure reader restarts worker for next reader")
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Hui Li <caelli@tencent.com>
Message-ID: <1680749090-14106-1-git-send-email-caelli@tencent.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
n_tty: Rename tail to old_tail in n_tty_read() 2023-08-03T08:22:29+00:00 Ilpo Järvinen ilpo.jarvinen@linux.intel.com 2022-11-11T14:25:02+00:00 d262770b95c717353d5beb3e6cc1abce98d59f30 [ Upstream commit 947d66b68f3c4e7cf8f3f3500807b9d2a0de28ce ] The local tail variable in n_tty_read() is used for one purpose, it keeps the old tail. Thus, rename it appropriately to improve code readability. Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Reviewed-by: Jiri Slaby <jirislaby@kernel.org> Link: https://lore.kernel.org/r/22b37499-ff9a-7fc1-f6e0-58411328d122@linux.intel.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Stable-dep-of: 4903fde8047a ("tty: fix hang on tty device with no_room set") Signed-off-by: Sasha Levin <sashal@kernel.org> 
[ Upstream commit 947d66b68f3c4e7cf8f3f3500807b9d2a0de28ce ]

The local tail variable in n_tty_read() is used for one purpose, it
keeps the old tail. Thus, rename it appropriately to improve code
readability.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Jiri Slaby <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/22b37499-ff9a-7fc1-f6e0-58411328d122@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Stable-dep-of: 4903fde8047a ("tty: fix hang on tty device with no_room set")
Signed-off-by: Sasha Levin <sashal@kernel.org>
tty: n_tty: Restore EOF push handling behavior 2022-06-14T16:36:03+00:00 Daniel Gibson daniel@gibson.sh 2022-03-29T23:58:10+00:00 c84fa729f8db9f86c0800515d6b72a56cceb15a7 [ Upstream commit 65a8b287023da68c4550deab5c764e6891cf1caf ] TTYs in ICANON mode have a special case that allows "pushing" a line without a regular EOL character (like newline), by using EOF (the EOT character - ASCII 0x4) as a pseudo-EOL. It is silently discarded, so the reader of the PTS will receive the line *without* EOF or any other terminating character. This special case has an edge case: What happens if the readers buffer is the same size as the line (without EOF)? Will they be able to tell if the whole line is received, i.e. if the next read() will return more of the same line or the next line? There are two possibilities, that both have (dis)advantages: 1. The next read() returns 0. FreeBSD (13.0) and OSX (10.11) do this. Advantage: The reader can interpret this as "the line is over". Disadvantage: read() returning 0 means EOF, the reader could also interpret it as "there's no more data" and stop reading or even close the PT. 2. The next read() returns the next line, the EOF is silently discarded. Solaris (or at least OpenIndiana 2021.10) does this, Linux has done do this since commit 40d5e0905a03 ("n_tty: Fix EOF push handling"); this behavior was recently broken by commit 359303076163 ("tty: n_tty: do not look ahead for EOL character past the end of the buffer"). Advantage: read() won't return 0 (EOF), reader less likely to be confused (and things like `while(read(..)>0)` don't break) Disadvantage: The reader can't really know if the read() continues the last line (that filled the whole read buffer) or starts a new line. As both options are defensible (and are used by other Unix-likes), it's best to stick to the "old" behavior since "n_tty: Fix EOF push handling" of 2013, i.e. silently discard that EOF. This patch - that I actually got from Linus for testing and only modified slightly - restores that behavior by skipping an EOF character if it's the next character after reading is done. Based on a patch from Linus Torvalds. Link: https://bugzilla.kernel.org/show_bug.cgi?id=215611 Fixes: 359303076163 ("tty: n_tty: do not look ahead for EOL character past the end of the buffer") Cc: Peter Hurley <peter@hurleysoftware.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Jiri Slaby <jirislaby@kernel.org> Reviewed-and-tested-by: Daniel Gibson <daniel@gibson.sh> Acked-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Daniel Gibson <daniel@gibson.sh> Link: https://lore.kernel.org/r/20220329235810.452513-2-daniel@gibson.sh Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Sasha Levin <sashal@kernel.org> 
[ Upstream commit 65a8b287023da68c4550deab5c764e6891cf1caf ]

TTYs in ICANON mode have a special case that allows "pushing" a line
without a regular EOL character (like newline), by using EOF (the EOT
character - ASCII 0x4) as a pseudo-EOL. It is silently discarded, so
the reader of the PTS will receive the line *without* EOF or any other
terminating character.

This special case has an edge case: What happens if the readers buffer
is the same size as the line (without EOF)? Will they be able to tell
if the whole line is received, i.e. if the next read() will return more
of the same line or the next line?

There are two possibilities,  that both have (dis)advantages:

1. The next read() returns 0. FreeBSD (13.0) and OSX (10.11) do this.
   Advantage: The reader can interpret this as "the line is over".
   Disadvantage: read() returning 0 means EOF, the reader could also
   interpret it as "there's no more data" and stop reading or even
   close the PT.

2. The next read() returns the next line, the EOF is silently discarded.
   Solaris (or at least OpenIndiana 2021.10) does this, Linux has done
   do this since commit 40d5e0905a03 ("n_tty: Fix EOF push handling");
   this behavior was recently broken by commit 359303076163 ("tty:
   n_tty: do not look ahead for EOL character past the end of the buffer").
   Advantage: read() won't return 0 (EOF), reader less likely to be
   confused (and things like `while(read(..)>0)` don't break)
   Disadvantage: The reader can't really know if the read() continues
   the last line (that filled the whole read buffer) or starts a
   new line.

As both options are defensible (and are used by other Unix-likes), it's
best to stick to the "old" behavior since "n_tty: Fix EOF push handling"
of 2013, i.e. silently discard that EOF.

This patch - that I actually got from Linus for testing and only
modified slightly - restores that behavior by skipping an EOF
character if it's the next character after reading is done.

Based on a patch from Linus Torvalds.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=215611
Fixes: 359303076163 ("tty: n_tty: do not look ahead for EOL character past the end of the buffer")
Cc: Peter Hurley <peter@hurleysoftware.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Reviewed-and-tested-by: Daniel Gibson <daniel@gibson.sh>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Daniel Gibson <daniel@gibson.sh>
Link: https://lore.kernel.org/r/20220329235810.452513-2-daniel@gibson.sh
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
tty: n_tty: do not look ahead for EOL character past the end of the buffer 2022-02-23T11:03:15+00:00 Linus Torvalds torvalds@linux-foundation.org 2022-02-15T23:28:00+00:00 e1dce8e127fdaa9039505a52b08b93863caac2a2 commit 3593030761630e09200072a4bd06468892c27be3 upstream. Daniel Gibson reports that the n_tty code gets line termination wrong in very specific cases: "If you feed a line with exactly 64 chars + terminating newline, and directly afterwards (without reading) another line into a pseudo terminal, the the first read() on the other side will return the 64 char line *without* terminating newline, and the next read() will return the missing terminating newline AND the complete next line (if it fits in the buffer)" and bisected the behavior to commit 3b830a9c34d5 ("tty: convert tty_ldisc_ops 'read()' function to take a kernel pointer"). Now, digging deeper, it turns out that the behavior isn't exactly new: what changed in commit 3b830a9c34d5 was that the tty line discipline .read() function is now passed an intermediate kernel buffer rather than the final user space buffer. And that intermediate kernel buffer is 64 bytes in size - thus that special case with exactly 64 bytes plus terminating newline. The same problem did exist before, but historically the boundary was not the 64-byte chunk, but the user-supplied buffer size, which is obviously generally bigger (and potentially bigger than N_TTY_BUF_SIZE, which would hide the issue entirely). The reason is that the n_tty canon_copy_from_read_buf() code would look ahead for the EOL character one byte further than it would actually copy. It would then decide that it had found the terminator, and unmark it as an EOL character - which in turn explains why the next read wouldn't then be terminated by it. Now, the reason it did all this in the first place is related to some historical and pretty obscure EOF behavior, see commit ac8f3bf8832a ("n_tty: Fix poll() after buffer-limited eof push read") and commit 40d5e0905a03 ("n_tty: Fix EOF push handling"). And the reason for the EOL confusion is that we treat EOF as a special EOL condition, with the EOL character being NUL (aka "__DISABLED_CHAR" in the kernel sources). So that EOF look-ahead also affects the normal EOL handling. This patch just removes the look-ahead that causes problems, because EOL is much more critical than the historical "EOF in the middle of a line that coincides with the end of the buffer" handling ever was. Now, it is possible that we should indeed re-introduce the "look at next character to see if it's a EOF" behavior, but if so, that should be done not at the kernel buffer chunk boundary in canon_copy_from_read_buf(), but at a higher level, when we run out of the user buffer. In particular, the place to do that would be at the top of 'n_tty_read()', where we check if it's a continuation of a previously started read, and there is no more buffer space left, we could decide to just eat the __DISABLED_CHAR at that point. But that would be a separate patch, because I suspect nobody actually cares, and I'd like to get a report about it before bothering. Fixes: 3b830a9c34d5 ("tty: convert tty_ldisc_ops 'read()' function to take a kernel pointer") Fixes: ac8f3bf8832a ("n_tty: Fix poll() after buffer-limited eof push read") Fixes: 40d5e0905a03 ("n_tty: Fix EOF push handling") Link: https://bugzilla.kernel.org/show_bug.cgi?id=215611 Reported-and-tested-by: Daniel Gibson <metalcaedes@gmail.com> Cc: Peter Hurley <peter@hurleysoftware.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Jiri Slaby <jirislaby@kernel.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
commit 3593030761630e09200072a4bd06468892c27be3 upstream.

Daniel Gibson reports that the n_tty code gets line termination wrong in
very specific cases:

 "If you feed a line with exactly 64 chars + terminating newline, and
  directly afterwards (without reading) another line into a pseudo
  terminal, the the first read() on the other side will return the 64
  char line *without* terminating newline, and the next read() will
  return the missing terminating newline AND the complete next line (if
  it fits in the buffer)"

and bisected the behavior to commit 3b830a9c34d5 ("tty: convert
tty_ldisc_ops 'read()' function to take a kernel pointer").

Now, digging deeper, it turns out that the behavior isn't exactly new:
what changed in commit 3b830a9c34d5 was that the tty line discipline
.read() function is now passed an intermediate kernel buffer rather than
the final user space buffer.

And that intermediate kernel buffer is 64 bytes in size - thus that
special case with exactly 64 bytes plus terminating newline.

The same problem did exist before, but historically the boundary was not
the 64-byte chunk, but the user-supplied buffer size, which is obviously
generally bigger (and potentially bigger than N_TTY_BUF_SIZE, which
would hide the issue entirely).

The reason is that the n_tty canon_copy_from_read_buf() code would look
ahead for the EOL character one byte further than it would actually
copy.  It would then decide that it had found the terminator, and unmark
it as an EOL character - which in turn explains why the next read
wouldn't then be terminated by it.

Now, the reason it did all this in the first place is related to some
historical and pretty obscure EOF behavior, see commit ac8f3bf8832a
("n_tty: Fix poll() after buffer-limited eof push read") and commit
40d5e0905a03 ("n_tty: Fix EOF push handling").

And the reason for the EOL confusion is that we treat EOF as a special
EOL condition, with the EOL character being NUL (aka "__DISABLED_CHAR"
in the kernel sources).

So that EOF look-ahead also affects the normal EOL handling.

This patch just removes the look-ahead that causes problems, because EOL
is much more critical than the historical "EOF in the middle of a line
that coincides with the end of the buffer" handling ever was.

Now, it is possible that we should indeed re-introduce the "look at next
character to see if it's a EOF" behavior, but if so, that should be done
not at the kernel buffer chunk boundary in canon_copy_from_read_buf(),
but at a higher level, when we run out of the user buffer.

In particular, the place to do that would be at the top of
'n_tty_read()', where we check if it's a continuation of a previously
started read, and there is no more buffer space left, we could decide to
just eat the __DISABLED_CHAR at that point.

But that would be a separate patch, because I suspect nobody actually
cares, and I'd like to get a report about it before bothering.

Fixes: 3b830a9c34d5 ("tty: convert tty_ldisc_ops 'read()' function to take a kernel pointer")
Fixes: ac8f3bf8832a ("n_tty: Fix  poll() after buffer-limited eof push read")
Fixes: 40d5e0905a03 ("n_tty: Fix EOF push handling")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=215611
Reported-and-tested-by: Daniel Gibson <metalcaedes@gmail.com>
Cc: Peter Hurley <peter@hurleysoftware.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
n_tty: wake up poll(POLLRDNORM) on receiving data 2022-02-16T11:56:33+00:00 TATSUKAWA KOSUKE (立川 江介) tatsu-ab1@nec.com 2022-01-26T23:35:02+00:00 decb36e9a9f02dc9e1e0ede0a93f968e12d86539 commit c816b2e65b0e86b95011418cad334f0524fc33b8 upstream. The poll man page says POLLRDNORM is equivalent to POLLIN when used as an event. $ man poll <snip> POLLRDNORM Equivalent to POLLIN. However, in n_tty driver, POLLRDNORM does not return until timeout even if there is terminal input, whereas POLLIN returns. The following test program works until kernel-3.17, but the test stops in poll() after commit 57087d515441 ("tty: Fix spurious poll() wakeups"). [Steps to run test program] $ cc -o test-pollrdnorm test-pollrdnorm.c $ ./test-pollrdnorm foo <-- Type in something from the terminal followed by [RET]. The string should be echoed back. ------------------------< test-pollrdnorm.c >------------------------ #include <stdio.h> #include <errno.h> #include <poll.h> #include <unistd.h> void main(void) { int n; unsigned char buf[8]; struct pollfd fds[1] = {{ 0, POLLRDNORM, 0 }}; n = poll(fds, 1, -1); if (n < 0) perror("poll"); n = read(0, buf, 8); if (n < 0) perror("read"); if (n > 0) write(1, buf, n); } ------------------------------------------------------------------------ The attached patch fixes this problem. Many calls to wake_up_interruptible_poll() in the kernel source code already specify "POLLIN | POLLRDNORM". Fixes: 57087d515441 ("tty: Fix spurious poll() wakeups") Cc: stable@vger.kernel.org Signed-off-by: Kosuke Tatsukawa <tatsu-ab1@nec.com> Link: https://lore.kernel.org/r/TYCPR01MB81901C0F932203D30E452B3EA5209@TYCPR01MB8190.jpnprd01.prod.outlook.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
commit c816b2e65b0e86b95011418cad334f0524fc33b8 upstream.

The poll man page says POLLRDNORM is equivalent to POLLIN when used as
an event.
$ man poll
<snip>
              POLLRDNORM
                     Equivalent to POLLIN.

However, in n_tty driver, POLLRDNORM does not return until timeout even
if there is terminal input, whereas POLLIN returns.

The following test program works until kernel-3.17, but the test stops
in poll() after commit 57087d515441 ("tty: Fix spurious poll() wakeups").

[Steps to run test program]
  $ cc -o test-pollrdnorm test-pollrdnorm.c
  $ ./test-pollrdnorm
  foo          <-- Type in something from the terminal followed by [RET].
                   The string should be echoed back.

  ------------------------< test-pollrdnorm.c >------------------------
  #include <stdio.h>
  #include <errno.h>
  #include <poll.h>
  #include <unistd.h>

  void main(void)
  {
	int		n;
	unsigned char	buf[8];
	struct pollfd	fds[1] = {{ 0, POLLRDNORM, 0 }};

	n = poll(fds, 1, -1);
	if (n < 0)
		perror("poll");
	n = read(0, buf, 8);
	if (n < 0)
		perror("read");
	if (n > 0)
		write(1, buf, n);
  }
  ------------------------------------------------------------------------

The attached patch fixes this problem.  Many calls to
wake_up_interruptible_poll() in the kernel source code already specify
"POLLIN | POLLRDNORM".

Fixes: 57087d515441 ("tty: Fix spurious poll() wakeups")
Cc: stable@vger.kernel.org
Signed-off-by: Kosuke Tatsukawa <tatsu-ab1@nec.com>
Link: https://lore.kernel.org/r/TYCPR01MB81901C0F932203D30E452B3EA5209@TYCPR01MB8190.jpnprd01.prod.outlook.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty: drop tty_ldisc_ops::refcount 2021-05-13T14:57:17+00:00 Jiri Slaby jslaby@suse.cz 2021-05-05T09:19:10+00:00 19475209331168cdb8070a011650535f1c54a730 The refcount is checked only in tty_unregister_ldisc and EBUSY returned if it is nonzero. But none of the tty_unregister_ldisc callers act anyhow if this (or any other) error is returned. So remove tty_ldisc_ops::refcount completely and make tty_unregister_ldisc return 'void' in the next patches. That means we assume tty_unregister_ldisc is not called while the ldisc might be in use. That relies on try_module_get in get_ldops and module_put in put_ldops. Signed-off-by: Jiri Slaby <jslaby@suse.cz> Link: https://lore.kernel.org/r/20210505091928.22010-18-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
The refcount is checked only in tty_unregister_ldisc and EBUSY returned
if it is nonzero. But none of the tty_unregister_ldisc callers act
anyhow if this (or any other) error is returned. So remove
tty_ldisc_ops::refcount completely and make tty_unregister_ldisc return
'void' in the next patches. That means we assume tty_unregister_ldisc is
not called while the ldisc might be in use. That relies on
try_module_get in get_ldops and module_put in put_ldops.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20210505091928.22010-18-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty: set tty_ldisc_ops::num statically 2021-05-13T14:57:16+00:00 Jiri Slaby jslaby@suse.cz 2021-05-05T09:19:07+00:00 fbadf70a8053b3dce78a45997ae55651693a2a81 There is no reason to pass the ldisc number to tty_register_ldisc separately. Just set it in the already defined tty_ldisc_ops in all the ldiscs. This simplifies tty_register_ldisc a bit too (no need to set the num member there). Signed-off-by: Jiri Slaby <jslaby@suse.cz> Cc: William Hubbs <w.d.hubbs@gmail.com> Cc: Chris Brannon <chris@the-brannons.com> Cc: Kirk Reiser <kirk@reisers.ca> Cc: Samuel Thibault <samuel.thibault@ens-lyon.org> Cc: Marcel Holtmann <marcel@holtmann.org> Cc: Johan Hedberg <johan.hedberg@gmail.com> Cc: Luiz Augusto von Dentz <luiz.dentz@gmail.com> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: "David S. Miller" <davem@davemloft.net> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Wolfgang Grandegger <wg@grandegger.com> Cc: Marc Kleine-Budde <mkl@pengutronix.de> Cc: Andreas Koensgen <ajk@comnets.uni-bremen.de> Cc: Paul Mackerras <paulus@samba.org> Cc: Rodolfo Giometti <giometti@enneenne.com> Cc: Peter Ujfalusi <peter.ujfalusi@gmail.com> Cc: Liam Girdwood <lgirdwood@gmail.com> Cc: Mark Brown <broonie@kernel.org> Cc: Jaroslav Kysela <perex@perex.cz> Cc: Takashi Iwai <tiwai@suse.com> Link: https://lore.kernel.org/r/20210505091928.22010-15-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
There is no reason to pass the ldisc number to tty_register_ldisc
separately. Just set it in the already defined tty_ldisc_ops in all the
ldiscs.

This simplifies tty_register_ldisc a bit too (no need to set the num
member there).

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: William Hubbs <w.d.hubbs@gmail.com>
Cc: Chris Brannon <chris@the-brannons.com>
Cc: Kirk Reiser <kirk@reisers.ca>
Cc: Samuel Thibault <samuel.thibault@ens-lyon.org>
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Wolfgang Grandegger <wg@grandegger.com>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: Andreas Koensgen <ajk@comnets.uni-bremen.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Rodolfo Giometti <giometti@enneenne.com>
Cc: Peter Ujfalusi <peter.ujfalusi@gmail.com>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Link: https://lore.kernel.org/r/20210505091928.22010-15-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty: cumulate and document tty_struct::ctrl* members 2021-05-13T14:57:16+00:00 Jiri Slaby jslaby@suse.cz 2021-05-05T09:19:06+00:00 64d608db38ffc0c7a25455387096e0aad9410397 Group the ctrl members under a single struct called ctrl. The new struct contains 'pgrp', 'session', 'pktstatus', and 'packet'. 'pktstatus' and 'packet' used to be bits in a bitfield. The struct also contains the lock protecting them to share the same cache line. Note that commit c545b66c6922b (tty: Serialize tcflow() with other tty flow control changes) added a padding to the original bitfield. It was for the bitfield to occupy a whole 64b word to avoid interferring stores on Alpha (cannot we evaporate this arch with weird implications to C code yet?). But it doesn't work as expected as the padding (tty_struct::ctrl_unused) is aligned to a 8B boundary too and occupies some bytes from the next word. So make it reliable by: 1) setting __aligned of the struct -- that aligns the start, and 2) making 'unsigned long unused[0]' as the last member of the struct -- pads the end. Add a kerneldoc comment for this grouped members. Signed-off-by: Jiri Slaby <jslaby@suse.cz> Cc: "David S. Miller" <davem@davemloft.net> Cc: Jakub Kicinski <kuba@kernel.org> Cc: netdev@vger.kernel.org Link: https://lore.kernel.org/r/20210505091928.22010-14-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
Group the ctrl members under a single struct called ctrl. The new struct
contains 'pgrp', 'session', 'pktstatus', and 'packet'. 'pktstatus' and
'packet' used to be bits in a bitfield. The struct also contains the
lock protecting them to share the same cache line.

Note that commit c545b66c6922b (tty: Serialize tcflow() with other tty
flow control changes) added a padding to the original bitfield. It was
for the bitfield to occupy a whole 64b word to avoid interferring stores
on Alpha (cannot we evaporate this arch with weird implications to C
code yet?). But it doesn't work as expected as the padding
(tty_struct::ctrl_unused) is aligned to a 8B boundary too and occupies
some bytes from the next word.

So make it reliable by:
1) setting __aligned of the struct -- that aligns the start, and
2) making 'unsigned long unused[0]' as the last member of the struct --
   pads the end.

Add a kerneldoc comment for this grouped members.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
Link: https://lore.kernel.org/r/20210505091928.22010-14-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty: cumulate and document tty_struct::flow* members 2021-05-13T14:57:16+00:00 Jiri Slaby jslaby@suse.cz 2021-05-05T09:19:05+00:00 6e94dbc7a4e49a028b81302d755bba1a518f973b Group the flow flags under a single struct called flow. The new struct contains 'stopped' and 'tco_stopped' bools which used to be bits in a bitfield. The struct also contains the lock protecting them to potentially share the same cache line. Note that commit c545b66c6922b (tty: Serialize tcflow() with other tty flow control changes) added a padding to the original bitfield. It was for the bitfield to occupy a whole 64b word to avoid interferring stores on Alpha (cannot we evaporate this arch with weird implications to C code yet?). But it doesn't work as expected as the padding (tty_struct::unused) is aligned to a 8B boundary too and occupies some bytes from the next word. So make it reliable by: 1) setting __aligned of the struct -- that aligns the start, and 2) making 'unsigned long unused[0]' as the last member of the struct -- pads the end. This is also the perfect time to start the documentation of tty_struct where all this lives. So we start by documenting what these bools actually serve for. And why we do all the alignment dances. Only the few up-to-date information from the Theodore's comment made it into this new Kerneldoc comment. Signed-off-by: Jiri Slaby <jslaby@suse.cz> Cc: "David S. Miller" <davem@davemloft.net> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Jonathan Corbet <corbet@lwn.net> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Ulf Hansson <ulf.hansson@linaro.org> Cc: Heiko Carstens <hca@linux.ibm.com> Cc: Vasily Gorbik <gor@linux.ibm.com> Cc: Christian Borntraeger <borntraeger@de.ibm.com> Cc: Shawn Guo <shawnguo@kernel.org> Cc: Sascha Hauer <s.hauer@pengutronix.de> Cc: Vineet Gupta <vgupta@synopsys.com> Cc: "Maciej W. Rozycki" <macro@orcam.me.uk> Link: https://lore.kernel.org/r/20210505091928.22010-13-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
Group the flow flags under a single struct called flow. The new struct
contains 'stopped' and 'tco_stopped' bools which used to be bits in a
bitfield. The struct also contains the lock protecting them to
potentially share the same cache line.

Note that commit c545b66c6922b (tty: Serialize tcflow() with other tty
flow control changes) added a padding to the original bitfield. It was
for the bitfield to occupy a whole 64b word to avoid interferring stores
on Alpha (cannot we evaporate this arch with weird implications to C
code yet?). But it doesn't work as expected as the padding
(tty_struct::unused) is aligned to a 8B boundary too and occupies some
bytes from the next word.

So make it reliable by:
1) setting __aligned of the struct -- that aligns the start, and
2) making 'unsigned long unused[0]' as the last member of the struct --
   pads the end.

This is also the perfect time to start the documentation of tty_struct
where all this lives. So we start by documenting what these bools
actually serve for. And why we do all the alignment dances. Only the few
up-to-date information from the Theodore's comment made it into this new
Kerneldoc comment.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: "Maciej W. Rozycki" <macro@orcam.me.uk>
Link: https://lore.kernel.org/r/20210505091928.22010-13-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty: make fp of tty_ldisc_ops::receive_buf{,2} const 2021-05-13T14:57:16+00:00 Jiri Slaby jslaby@suse.cz 2021-05-05T09:19:04+00:00 0f3dcf3b5d76669123bf99fec812b8b0acd60375 Char pointer (cp) passed to tty_ldisc_ops::receive_buf{,2} is const. There is no reason for flag pointer (fp) not to be too. So switch it in the definition and all uses. Signed-off-by: Jiri Slaby <jslaby@suse.cz> Cc: William Hubbs <w.d.hubbs@gmail.com> Cc: Chris Brannon <chris@the-brannons.com> Cc: Kirk Reiser <kirk@reisers.ca> Cc: Samuel Thibault <samuel.thibault@ens-lyon.org> Cc: Marcel Holtmann <marcel@holtmann.org> Cc: Johan Hedberg <johan.hedberg@gmail.com> Cc: Luiz Augusto von Dentz <luiz.dentz@gmail.com> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: "David S. Miller" <davem@davemloft.net> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Wolfgang Grandegger <wg@grandegger.com> Cc: Marc Kleine-Budde <mkl@pengutronix.de> Cc: Andreas Koensgen <ajk@comnets.uni-bremen.de> Cc: Paul Mackerras <paulus@samba.org> Cc: Liam Girdwood <lgirdwood@gmail.com> Cc: Mark Brown <broonie@kernel.org> Cc: Jaroslav Kysela <perex@perex.cz> Cc: Takashi Iwai <tiwai@suse.com> Cc: Peter Ujfalusi <peter.ujfalusi@gmail.com> Link: https://lore.kernel.org/r/20210505091928.22010-12-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
Char pointer (cp) passed to tty_ldisc_ops::receive_buf{,2} is const.
There is no reason for flag pointer (fp) not to be too. So switch it in
the definition and all uses.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: William Hubbs <w.d.hubbs@gmail.com>
Cc: Chris Brannon <chris@the-brannons.com>
Cc: Kirk Reiser <kirk@reisers.ca>
Cc: Samuel Thibault <samuel.thibault@ens-lyon.org>
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Wolfgang Grandegger <wg@grandegger.com>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: Andreas Koensgen <ajk@comnets.uni-bremen.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Peter Ujfalusi <peter.ujfalusi@gmail.com>
Link: https://lore.kernel.org/r/20210505091928.22010-12-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>