Turns out there is a lot of tty-internal stuff in include/linux/tty.h
that do not belong there. Create a internal-to-the-tty-layer .h file
for these types of things and move function prototypes to it instead of
being in the system-wide header file.
Along the way clean up the use of some old tty-only debugging macros and
use the in-kernel dev_*() calls instead.
Greg Kroah-Hartman (13):
tty: create internal tty.h file
tty: tty.h: remove tty_info()
tty: remove tty_err()
tty: remove tty_notice()
tty: remove tty_warn()
tty: remove tty_info_ratelimited()
tty: remove tty_debug()
tty: audit: move some local functions out of tty.h
tty: move some internal tty lock enums and functions out of tty.h
tty: make tty_release_redirect() static
tty: move some tty-only functions to drivers/tty/tty.h
tty: remove tty_driver_name()
tty: clean include/linux/tty.h up
drivers/tty/n_gsm.c | 1 +
drivers/tty/n_hdlc.c | 1 +
drivers/tty/n_tty.c | 5 +-
drivers/tty/pty.c | 3 +-
drivers/tty/tty.h | 103 ++++++++++++++++++++++++++++++++++++++
drivers/tty/tty_audit.c | 1 +
drivers/tty/tty_buffer.c | 2 +-
drivers/tty/tty_io.c | 43 +++++++---------
drivers/tty/tty_ioctl.c | 3 +-
drivers/tty/tty_jobctrl.c | 7 +--
drivers/tty/tty_ldisc.c | 3 +-
drivers/tty/tty_mutex.c | 1 +
drivers/tty/tty_port.c | 5 +-
include/linux/tty.h | 100 ------------------------------------
14 files changed, 142 insertions(+), 136 deletions(-)
create mode 100644 drivers/tty/tty.h
--
2.31.1
The flow change and restricted_tty_write() logic is internal to the tty
core only, so move it out of the include/linux/tty.h file.
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/tty.h | 17 +++++++++++++++++
drivers/tty/tty_ioctl.c | 1 +
include/linux/tty.h | 16 ----------------
3 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/tty/tty.h b/drivers/tty/tty.h
index ff904e947483..b0d78bfdbd8c 100644
--- a/drivers/tty/tty.h
+++ b/drivers/tty/tty.h
@@ -29,6 +29,21 @@ enum {
TTY_LOCK_SLAVE,
};
+/* Values for tty->flow_change */
+#define TTY_THROTTLE_SAFE 1
+#define TTY_UNTHROTTLE_SAFE 2
+
+static inline void __tty_set_flow_change(struct tty_struct *tty, int val)
+{
+ tty->flow_change = val;
+}
+
+static inline void tty_set_flow_change(struct tty_struct *tty, int val)
+{
+ tty->flow_change = val;
+ smp_mb();
+}
+
int tty_ldisc_lock(struct tty_struct *tty, unsigned long timeout);
void tty_ldisc_unlock(struct tty_struct *tty);
@@ -46,4 +61,6 @@ static inline void tty_audit_tiocsti(struct tty_struct *tty, char ch)
}
#endif
+ssize_t redirected_tty_write(struct kiocb *, struct iov_iter *);
+
#endif
diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 053290ab5cb8..70972344946e 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -21,6 +21,7 @@
#include <linux/bitops.h>
#include <linux/mutex.h>
#include <linux/compat.h>
+#include "tty.h"
#include <asm/io.h>
#include <linux/uaccess.h>
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 1b6f5dc3dcb2..777887d8bd6d 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -349,21 +349,6 @@ struct tty_file_private {
#define TTY_LDISC_CHANGING 20 /* Change pending - non-block IO */
#define TTY_LDISC_HALTED 22 /* Line discipline is halted */
-/* Values for tty->flow_change */
-#define TTY_THROTTLE_SAFE 1
-#define TTY_UNTHROTTLE_SAFE 2
-
-static inline void __tty_set_flow_change(struct tty_struct *tty, int val)
-{
- tty->flow_change = val;
-}
-
-static inline void tty_set_flow_change(struct tty_struct *tty, int val)
-{
- tty->flow_change = val;
- smp_mb();
-}
-
static inline bool tty_io_nonblock(struct tty_struct *tty, struct file *file)
{
return file->f_flags & O_NONBLOCK ||
@@ -395,7 +380,6 @@ extern struct tty_struct *tty_kopen_exclusive(dev_t device);
extern struct tty_struct *tty_kopen_shared(dev_t device);
extern void tty_kclose(struct tty_struct *tty);
extern int tty_dev_name_to_number(const char *name, dev_t *number);
-extern ssize_t redirected_tty_write(struct kiocb *, struct iov_iter *);
#else
static inline void tty_kref_put(struct tty_struct *tty)
{ }
--
2.31.1
No one uses it, so remove it, it is dead code.
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/tty_io.c | 7 -------
include/linux/tty.h | 1 -
2 files changed, 8 deletions(-)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index aa959f3371b1..5089104cafda 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -251,13 +251,6 @@ const char *tty_name(const struct tty_struct *tty)
EXPORT_SYMBOL(tty_name);
-const char *tty_driver_name(const struct tty_struct *tty)
-{
- if (!tty || !tty->driver)
- return "";
- return tty->driver->name;
-}
-
static int tty_paranoia_check(struct tty_struct *tty, struct inode *inode,
const char *routine)
{
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 777887d8bd6d..143f393dca3b 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -430,7 +430,6 @@ static inline struct tty_struct *tty_kref_get(struct tty_struct *tty)
return tty;
}
-extern const char *tty_driver_name(const struct tty_struct *tty);
extern void tty_wait_until_sent(struct tty_struct *tty, long timeout);
extern int __tty_check_change(struct tty_struct *tty, int sig);
extern int tty_check_change(struct tty_struct *tty);
--
2.31.1
There are a lot of tty-core-only functions that are listed in
include/linux/tty.h. Move them to drivers/tty/tty.h so that no one else
can accidentally call them or think that they are public functions.
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/n_gsm.c | 1 +
drivers/tty/n_hdlc.c | 1 +
drivers/tty/tty.h | 37 +++++++++++++++++++++++++++++++++++++
include/linux/tty.h | 34 ----------------------------------
4 files changed, 39 insertions(+), 34 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 51dafc06f541..6114980c832d 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -50,6 +50,7 @@
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
#include <linux/gsmmux.h>
+#include "tty.h"
static int debug;
module_param(debug, int, 0600);
diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 1363e659dc1d..e64ab74c9a2c 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -100,6 +100,7 @@
#include <asm/termios.h>
#include <linux/uaccess.h>
+#include "tty.h"
/*
* Buffers for individual HDLC frames
diff --git a/drivers/tty/tty.h b/drivers/tty/tty.h
index b0d78bfdbd8c..caaf97ba5267 100644
--- a/drivers/tty/tty.h
+++ b/drivers/tty/tty.h
@@ -47,6 +47,43 @@ static inline void tty_set_flow_change(struct tty_struct *tty, int val)
int tty_ldisc_lock(struct tty_struct *tty, unsigned long timeout);
void tty_ldisc_unlock(struct tty_struct *tty);
+int __tty_check_change(struct tty_struct *tty, int sig);
+int tty_check_change(struct tty_struct *tty);
+void __stop_tty(struct tty_struct *tty);
+void __start_tty(struct tty_struct *tty);
+void tty_vhangup_session(struct tty_struct *tty);
+void tty_open_proc_set_tty(struct file *filp, struct tty_struct *tty);
+int tty_signal_session_leader(struct tty_struct *tty, int exit_session);
+void session_clear_tty(struct pid *session);
+void tty_buffer_free_all(struct tty_port *port);
+void tty_buffer_flush(struct tty_struct *tty, struct tty_ldisc *ld);
+void tty_buffer_init(struct tty_port *port);
+void tty_buffer_set_lock_subclass(struct tty_port *port);
+bool tty_buffer_restart_work(struct tty_port *port);
+bool tty_buffer_cancel_work(struct tty_port *port);
+void tty_buffer_flush_work(struct tty_port *port);
+speed_t tty_termios_input_baud_rate(struct ktermios *termios);
+void tty_ldisc_hangup(struct tty_struct *tty, bool reset);
+int tty_ldisc_reinit(struct tty_struct *tty, int disc);
+long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
+long tty_jobctrl_ioctl(struct tty_struct *tty, struct tty_struct *real_tty,
+ struct file *file, unsigned int cmd, unsigned long arg);
+void tty_default_fops(struct file_operations *fops);
+struct tty_struct *alloc_tty_struct(struct tty_driver *driver, int idx);
+int tty_alloc_file(struct file *file);
+void tty_add_file(struct tty_struct *tty, struct file *file);
+void tty_free_file(struct file *file);
+int tty_release(struct inode *inode, struct file *filp);
+
+#define tty_is_writelocked(tty) (mutex_is_locked(&tty->atomic_write_lock))
+
+int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty);
+void tty_ldisc_release(struct tty_struct *tty);
+int __must_check tty_ldisc_init(struct tty_struct *tty);
+void tty_ldisc_deinit(struct tty_struct *tty);
+
+void tty_sysctl_init(void);
+
/* tty_audit.c */
#ifdef CONFIG_AUDIT
void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 143f393dca3b..1611214c8457 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -431,11 +431,7 @@ static inline struct tty_struct *tty_kref_get(struct tty_struct *tty)
}
extern void tty_wait_until_sent(struct tty_struct *tty, long timeout);
-extern int __tty_check_change(struct tty_struct *tty, int sig);
-extern int tty_check_change(struct tty_struct *tty);
-extern void __stop_tty(struct tty_struct *tty);
extern void stop_tty(struct tty_struct *tty);
-extern void __start_tty(struct tty_struct *tty);
extern void start_tty(struct tty_struct *tty);
extern int tty_register_driver(struct tty_driver *driver);
extern int tty_unregister_driver(struct tty_driver *driver);
@@ -462,23 +458,11 @@ extern int tty_get_icount(struct tty_struct *tty,
extern int is_current_pgrp_orphaned(void);
extern void tty_hangup(struct tty_struct *tty);
extern void tty_vhangup(struct tty_struct *tty);
-extern void tty_vhangup_session(struct tty_struct *tty);
extern int tty_hung_up_p(struct file *filp);
extern void do_SAK(struct tty_struct *tty);
extern void __do_SAK(struct tty_struct *tty);
-extern void tty_open_proc_set_tty(struct file *filp, struct tty_struct *tty);
-extern int tty_signal_session_leader(struct tty_struct *tty, int exit_session);
-extern void session_clear_tty(struct pid *session);
extern void no_tty(void);
-extern void tty_buffer_free_all(struct tty_port *port);
-extern void tty_buffer_flush(struct tty_struct *tty, struct tty_ldisc *ld);
-extern void tty_buffer_init(struct tty_port *port);
-extern void tty_buffer_set_lock_subclass(struct tty_port *port);
-extern bool tty_buffer_restart_work(struct tty_port *port);
-extern bool tty_buffer_cancel_work(struct tty_port *port);
-extern void tty_buffer_flush_work(struct tty_port *port);
extern speed_t tty_termios_baud_rate(struct ktermios *termios);
-extern speed_t tty_termios_input_baud_rate(struct ktermios *termios);
extern void tty_termios_encode_baud_rate(struct ktermios *termios,
speed_t ibaud, speed_t obaud);
extern void tty_encode_baud_rate(struct tty_struct *tty,
@@ -506,27 +490,16 @@ extern int tty_set_termios(struct tty_struct *tty, struct ktermios *kt);
extern struct tty_ldisc *tty_ldisc_ref(struct tty_struct *);
extern void tty_ldisc_deref(struct tty_ldisc *);
extern struct tty_ldisc *tty_ldisc_ref_wait(struct tty_struct *);
-extern void tty_ldisc_hangup(struct tty_struct *tty, bool reset);
-extern int tty_ldisc_reinit(struct tty_struct *tty, int disc);
extern const struct seq_operations tty_ldiscs_seq_ops;
extern void tty_wakeup(struct tty_struct *tty);
extern void tty_ldisc_flush(struct tty_struct *tty);
-extern long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
extern int tty_mode_ioctl(struct tty_struct *tty, struct file *file,
unsigned int cmd, unsigned long arg);
-extern long tty_jobctrl_ioctl(struct tty_struct *tty, struct tty_struct *real_tty,
- struct file *file, unsigned int cmd, unsigned long arg);
extern int tty_perform_flush(struct tty_struct *tty, unsigned long arg);
-extern void tty_default_fops(struct file_operations *fops);
-extern struct tty_struct *alloc_tty_struct(struct tty_driver *driver, int idx);
-extern int tty_alloc_file(struct file *file);
-extern void tty_add_file(struct tty_struct *tty, struct file *file);
-extern void tty_free_file(struct file *file);
extern struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx);
extern void tty_release_struct(struct tty_struct *tty, int idx);
-extern int tty_release(struct inode *inode, struct file *filp);
extern void tty_init_termios(struct tty_struct *tty);
extern void tty_save_termios(struct tty_struct *tty);
extern int tty_standard_install(struct tty_driver *driver,
@@ -534,8 +507,6 @@ extern int tty_standard_install(struct tty_driver *driver,
extern struct mutex tty_mutex;
-#define tty_is_writelocked(tty) (mutex_is_locked(&tty->atomic_write_lock))
-
extern void tty_port_init(struct tty_port *port);
extern void tty_port_link_device(struct tty_port *port,
struct tty_driver *driver, unsigned index);
@@ -655,13 +626,8 @@ static inline int tty_port_users(struct tty_port *port)
extern int tty_register_ldisc(int disc, struct tty_ldisc_ops *new_ldisc);
extern int tty_unregister_ldisc(int disc);
extern int tty_set_ldisc(struct tty_struct *tty, int disc);
-extern int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty);
-extern void tty_ldisc_release(struct tty_struct *tty);
-extern int __must_check tty_ldisc_init(struct tty_struct *tty);
-extern void tty_ldisc_deinit(struct tty_struct *tty);
extern int tty_ldisc_receive_buf(struct tty_ldisc *ld, const unsigned char *p,
char *f, int count);
-extern void tty_sysctl_init(void);
/* n_tty.c */
extern void n_tty_inherit_ops(struct tty_ldisc_ops *ops);
--
2.31.1
No one is calling this macro, and no one should, so remove it from the
.h file.
Cc: Tetsuo Handa <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/tty.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/tty/tty.h b/drivers/tty/tty.h
index f4cd20261e91..75624d7d84ae 100644
--- a/drivers/tty/tty.h
+++ b/drivers/tty/tty.h
@@ -10,7 +10,6 @@
fn("%s %s: " f, tty_driver_name(tty), tty_name(tty), ##__VA_ARGS__)
#define tty_debug(tty, f, ...) tty_msg(pr_debug, tty, f, ##__VA_ARGS__)
-#define tty_info(tty, f, ...) tty_msg(pr_info, tty, f, ##__VA_ARGS__)
#define tty_notice(tty, f, ...) tty_msg(pr_notice, tty, f, ##__VA_ARGS__)
#define tty_warn(tty, f, ...) tty_msg(pr_warn, tty, f, ##__VA_ARGS__)
#define tty_err(tty, f, ...) tty_msg(pr_err, tty, f, ##__VA_ARGS__)
--
2.31.1
Remove the 2 users of tty_err() and replace it with calls to dev_err()
which provides more information about the tty that has the error and
uses the standard formatting logic.
Cc: Tetsuo Handa <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/n_tty.c | 2 +-
drivers/tty/tty.h | 1 -
drivers/tty/tty_io.c | 2 +-
3 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index ff1b3154ba0c..dbe208342258 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1483,7 +1483,7 @@ n_tty_receive_char_flagged(struct tty_struct *tty, unsigned char c, char flag)
n_tty_receive_overrun(tty);
break;
default:
- tty_err(tty, "unknown flag %d\n", flag);
+ dev_err(tty->dev, "unknown flag %d\n", flag);
break;
}
}
diff --git a/drivers/tty/tty.h b/drivers/tty/tty.h
index 75624d7d84ae..eda037c48317 100644
--- a/drivers/tty/tty.h
+++ b/drivers/tty/tty.h
@@ -12,7 +12,6 @@
#define tty_debug(tty, f, ...) tty_msg(pr_debug, tty, f, ##__VA_ARGS__)
#define tty_notice(tty, f, ...) tty_msg(pr_notice, tty, f, ##__VA_ARGS__)
#define tty_warn(tty, f, ...) tty_msg(pr_warn, tty, f, ##__VA_ARGS__)
-#define tty_err(tty, f, ...) tty_msg(pr_err, tty, f, ##__VA_ARGS__)
#define tty_info_ratelimited(tty, f, ...) \
tty_msg(pr_info_ratelimited, tty, f, ##__VA_ARGS__)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index c95f72085cdb..f8b96f3674af 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1124,7 +1124,7 @@ static ssize_t file_tty_write(struct file *file, struct kiocb *iocb, struct iov_
return -EIO;
/* Short term debug to catch buggy drivers */
if (tty->ops->write_room == NULL)
- tty_err(tty, "missing write_room method\n");
+ dev_err(tty->dev, "missing write_room method\n");
ld = tty_ldisc_ref_wait(tty);
if (!ld)
return hung_up_tty_write(iocb, from);
--
2.31.1
Remove the 3 users of tty_notice() and replace them with calls to
dev_notice() which provides more information about the tty that has the
error and uses the standard formatting logic.
Cc: Tetsuo Handa <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/tty.h | 1 -
drivers/tty/tty_io.c | 6 +++---
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/tty.h b/drivers/tty/tty.h
index eda037c48317..0323bc2cd6ba 100644
--- a/drivers/tty/tty.h
+++ b/drivers/tty/tty.h
@@ -10,7 +10,6 @@
fn("%s %s: " f, tty_driver_name(tty), tty_name(tty), ##__VA_ARGS__)
#define tty_debug(tty, f, ...) tty_msg(pr_debug, tty, f, ##__VA_ARGS__)
-#define tty_notice(tty, f, ...) tty_msg(pr_notice, tty, f, ##__VA_ARGS__)
#define tty_warn(tty, f, ...) tty_msg(pr_warn, tty, f, ##__VA_ARGS__)
#define tty_info_ratelimited(tty, f, ...) \
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index f8b96f3674af..0079ffd0cb9c 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3059,7 +3059,7 @@ void __do_SAK(struct tty_struct *tty)
read_lock(&tasklist_lock);
/* Kill the entire session */
do_each_pid_task(session, PIDTYPE_SID, p) {
- tty_notice(tty, "SAK: killed process %d (%s): by session\n",
+ dev_notice(tty->dev, "SAK: killed process %d (%s): by session\n",
task_pid_nr(p), p->comm);
group_send_sig_info(SIGKILL, SEND_SIG_PRIV, p, PIDTYPE_SID);
} while_each_pid_task(session, PIDTYPE_SID, p);
@@ -3067,7 +3067,7 @@ void __do_SAK(struct tty_struct *tty)
/* Now kill any processes that happen to have the tty open */
do_each_thread(g, p) {
if (p->signal->tty == tty) {
- tty_notice(tty, "SAK: killed process %d (%s): by controlling tty\n",
+ dev_notice(tty->dev, "SAK: killed process %d (%s): by controlling tty\n",
task_pid_nr(p), p->comm);
group_send_sig_info(SIGKILL, SEND_SIG_PRIV, p, PIDTYPE_SID);
continue;
@@ -3075,7 +3075,7 @@ void __do_SAK(struct tty_struct *tty)
task_lock(p);
i = iterate_fd(p->files, 0, this_tty, tty);
if (i != 0) {
- tty_notice(tty, "SAK: killed process %d (%s): by fd#%d\n",
+ dev_notice(tty->dev, "SAK: killed process %d (%s): by fd#%d\n",
task_pid_nr(p), p->comm, i - 1);
group_send_sig_info(SIGKILL, SEND_SIG_PRIV, p, PIDTYPE_SID);
}
--
2.31.1
Remove users of tty_warn() and replace them with calls to dev_warn()
which provides more information about the tty that has the error and
uses the standard formatting logic.
Cc: Tetsuo Handa <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/n_tty.c | 2 +-
drivers/tty/tty.h | 1 -
drivers/tty/tty_io.c | 8 ++++----
drivers/tty/tty_jobctrl.c | 2 +-
drivers/tty/tty_port.c | 4 ++--
5 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index dbe208342258..39a448ef0aed 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1200,7 +1200,7 @@ static void n_tty_receive_overrun(struct tty_struct *tty)
ldata->num_overrun++;
if (time_after(jiffies, ldata->overrun_time + HZ) ||
time_after(ldata->overrun_time, jiffies)) {
- tty_warn(tty, "%d input overrun(s)\n", ldata->num_overrun);
+ dev_warn(tty->dev, "%d input overrun(s)\n", ldata->num_overrun);
ldata->overrun_time = jiffies;
ldata->num_overrun = 0;
}
diff --git a/drivers/tty/tty.h b/drivers/tty/tty.h
index 0323bc2cd6ba..45b15cc250e8 100644
--- a/drivers/tty/tty.h
+++ b/drivers/tty/tty.h
@@ -10,7 +10,6 @@
fn("%s %s: " f, tty_driver_name(tty), tty_name(tty), ##__VA_ARGS__)
#define tty_debug(tty, f, ...) tty_msg(pr_debug, tty, f, ##__VA_ARGS__)
-#define tty_warn(tty, f, ...) tty_msg(pr_warn, tty, f, ##__VA_ARGS__)
#define tty_info_ratelimited(tty, f, ...) \
tty_msg(pr_info_ratelimited, tty, f, ##__VA_ARGS__)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 0079ffd0cb9c..2c3efa854ba5 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -295,7 +295,7 @@ static int check_tty_count(struct tty_struct *tty, const char *routine)
if (tty_port_kopened(tty->port))
kopen_count++;
if (tty->count != (count + kopen_count)) {
- tty_warn(tty, "%s: tty->count(%d) != (#fd's(%d) + #kopen's(%d))\n",
+ dev_warn(tty->dev, "%s: tty->count(%d) != (#fd's(%d) + #kopen's(%d))\n",
routine, tty->count, count, kopen_count);
return (count + kopen_count);
}
@@ -1823,7 +1823,7 @@ int tty_release(struct inode *inode, struct file *filp)
if (once) {
once = 0;
- tty_warn(tty, "read/write wait queue active!\n");
+ dev_warn(tty->dev, "read/write wait queue active!\n");
}
schedule_timeout_killable(timeout);
if (timeout < 120 * HZ)
@@ -1834,12 +1834,12 @@ int tty_release(struct inode *inode, struct file *filp)
if (o_tty) {
if (--o_tty->count < 0) {
- tty_warn(tty, "bad slave count (%d)\n", o_tty->count);
+ dev_warn(tty->dev, "bad slave count (%d)\n", o_tty->count);
o_tty->count = 0;
}
}
if (--tty->count < 0) {
- tty_warn(tty, "bad tty->count (%d)\n", tty->count);
+ dev_warn(tty->dev, "bad tty->count (%d)\n", tty->count);
tty->count = 0;
}
diff --git a/drivers/tty/tty_jobctrl.c b/drivers/tty/tty_jobctrl.c
index 0728730d38d1..19ec43a6ef76 100644
--- a/drivers/tty/tty_jobctrl.c
+++ b/drivers/tty/tty_jobctrl.c
@@ -61,7 +61,7 @@ int __tty_check_change(struct tty_struct *tty, int sig)
rcu_read_unlock();
if (!tty_pgrp)
- tty_warn(tty, "sig=%d, tty->pgrp == NULL!\n", sig);
+ dev_warn(tty->dev, "sig=%d, tty->pgrp == NULL!\n", sig);
return ret;
}
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index 303c198fbf5c..575fe3933ff9 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -567,12 +567,12 @@ int tty_port_close_start(struct tty_port *port,
spin_lock_irqsave(&port->lock, flags);
if (tty->count == 1 && port->count != 1) {
- tty_warn(tty, "%s: tty->count = 1 port count = %d\n", __func__,
+ dev_warn(tty->dev, "%s: tty->count = 1 port count = %d\n", __func__,
port->count);
port->count = 1;
}
if (--port->count < 0) {
- tty_warn(tty, "%s: bad port count (%d)\n", __func__,
+ dev_warn(tty->dev, "%s: bad port count (%d)\n", __func__,
port->count);
port->count = 0;
}
--
2.31.1
Remove the one user of tty_info_ratelimited() and replace it with a
calls to dev_info_ratelimited() which provides more information about
the tty that has the error and uses the standard formatting logic.
Cc: Tetsuo Handa <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/tty.h | 3 ---
drivers/tty/tty_io.c | 2 +-
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/tty/tty.h b/drivers/tty/tty.h
index 45b15cc250e8..a2084b58d4f3 100644
--- a/drivers/tty/tty.h
+++ b/drivers/tty/tty.h
@@ -11,7 +11,4 @@
#define tty_debug(tty, f, ...) tty_msg(pr_debug, tty, f, ##__VA_ARGS__)
-#define tty_info_ratelimited(tty, f, ...) \
- tty_msg(pr_info_ratelimited, tty, f, ##__VA_ARGS__)
-
#endif
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 2c3efa854ba5..91062fcc6667 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1477,7 +1477,7 @@ struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx)
/* call the tty release_tty routine to clean out this slot */
err_release_tty:
tty_ldisc_unlock(tty);
- tty_info_ratelimited(tty, "ldisc open failed (%d), clearing slot %d\n",
+ dev_info_ratelimited(tty->dev, "ldisc open failed (%d), clearing slot %d\n",
retval, idx);
err_release_lock:
tty_unlock(tty);
--
2.31.1
The functions tty_audit_add_data() and tty_audit_tiocsti() are local to
the tty core code, and do not need to be in a "kernel-wide" header file
so move them to drivers/tty/tty.h
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/tty.h | 14 ++++++++++++++
drivers/tty/tty_audit.c | 1 +
include/linux/tty.h | 10 ----------
3 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/tty/tty.h b/drivers/tty/tty.h
index e9cb918348cf..a8a7abe5d635 100644
--- a/drivers/tty/tty.h
+++ b/drivers/tty/tty.h
@@ -6,4 +6,18 @@
#ifndef _TTY_INTERNAL_H
#define _TTY_INTERNAL_H
+/* tty_audit.c */
+#ifdef CONFIG_AUDIT
+void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size);
+void tty_audit_tiocsti(struct tty_struct *tty, char ch);
+#else
+static inline void tty_audit_add_data(struct tty_struct *tty, const void *data,
+ size_t size)
+{
+}
+static inline void tty_audit_tiocsti(struct tty_struct *tty, char ch)
+{
+}
+#endif
+
#endif
diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 32898aabcd06..ca7afd7b2716 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -10,6 +10,7 @@
#include <linux/audit.h>
#include <linux/slab.h>
#include <linux/tty.h>
+#include "tty.h"
struct tty_audit_buf {
struct mutex mutex; /* Protects all data below */
diff --git a/include/linux/tty.h b/include/linux/tty.h
index fd8308a1f37e..981ee31c58e1 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -717,20 +717,10 @@ static inline void n_tty_init(void) { }
/* tty_audit.c */
#ifdef CONFIG_AUDIT
-extern void tty_audit_add_data(struct tty_struct *tty, const void *data,
- size_t size);
extern void tty_audit_exit(void);
extern void tty_audit_fork(struct signal_struct *sig);
-extern void tty_audit_tiocsti(struct tty_struct *tty, char ch);
extern int tty_audit_push(void);
#else
-static inline void tty_audit_add_data(struct tty_struct *tty, const void *data,
- size_t size)
-{
-}
-static inline void tty_audit_tiocsti(struct tty_struct *tty, char ch)
-{
-}
static inline void tty_audit_exit(void)
{
}
--
2.31.1
No one calls this outside of the tty_io.c file, so mark this static and
do not export the symbol anymore.
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/tty_io.c | 3 +--
include/linux/tty.h | 1 -
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 1c480c04374c..aa959f3371b1 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -547,7 +547,7 @@ EXPORT_SYMBOL_GPL(tty_wakeup);
* This is available to the pty code so if the master closes, if the
* slave is a redirect it can release the redirect.
*/
-struct file *tty_release_redirect(struct tty_struct *tty)
+static struct file *tty_release_redirect(struct tty_struct *tty)
{
struct file *f = NULL;
@@ -560,7 +560,6 @@ struct file *tty_release_redirect(struct tty_struct *tty)
return f;
}
-EXPORT_SYMBOL_GPL(tty_release_redirect);
/**
* __tty_hangup - actual handler for hangup events
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 4b21d47bc098..1b6f5dc3dcb2 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -396,7 +396,6 @@ extern struct tty_struct *tty_kopen_shared(dev_t device);
extern void tty_kclose(struct tty_struct *tty);
extern int tty_dev_name_to_number(const char *name, dev_t *number);
extern ssize_t redirected_tty_write(struct kiocb *, struct iov_iter *);
-extern struct file *tty_release_redirect(struct tty_struct *tty);
#else
static inline void tty_kref_put(struct tty_struct *tty)
{ }
--
2.31.1
Remove users of tty_debug() and replace them with calls to dev_dbg()
which provides more information about the tty that has the error and
uses the standard formatting logic.
Cc: Tetsuo Handa <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/pty.c | 2 +-
drivers/tty/tty.h | 5 -----
drivers/tty/tty_io.c | 14 +++++++-------
drivers/tty/tty_ioctl.c | 2 +-
drivers/tty/tty_jobctrl.c | 4 ++--
drivers/tty/tty_ldisc.c | 2 +-
6 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 6c90d3fd2d51..5695c78fbe55 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -33,7 +33,7 @@
#undef TTY_DEBUG_HANGUP
#ifdef TTY_DEBUG_HANGUP
-# define tty_debug_hangup(tty, f, args...) tty_debug(tty, f, ##args)
+# define tty_debug_hangup(tty, f, args...) dev_dbg(tty->dev, f, ##args)
#else
# define tty_debug_hangup(tty, f, args...) do {} while (0)
#endif
diff --git a/drivers/tty/tty.h b/drivers/tty/tty.h
index a2084b58d4f3..e9cb918348cf 100644
--- a/drivers/tty/tty.h
+++ b/drivers/tty/tty.h
@@ -6,9 +6,4 @@
#ifndef _TTY_INTERNAL_H
#define _TTY_INTERNAL_H
-#define tty_msg(fn, tty, f, ...) \
- fn("%s %s: " f, tty_driver_name(tty), tty_name(tty), ##__VA_ARGS__)
-
-#define tty_debug(tty, f, ...) tty_msg(pr_debug, tty, f, ##__VA_ARGS__)
-
#endif
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 91062fcc6667..1c480c04374c 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -112,7 +112,7 @@
#undef TTY_DEBUG_HANGUP
#ifdef TTY_DEBUG_HANGUP
-# define tty_debug_hangup(tty, f, args...) tty_debug(tty, f, ##args)
+# define tty_debug_hangup(tty, f, args...) dev_dbg(tty->dev, f, ##args)
#else
# define tty_debug_hangup(tty, f, args...) do { } while (0)
#endif
@@ -1639,7 +1639,7 @@ static int tty_release_checks(struct tty_struct *tty, int idx)
{
#ifdef TTY_PARANOIA_CHECK
if (idx < 0 || idx >= tty->driver->num) {
- tty_debug(tty, "bad idx %d\n", idx);
+ dev_dbg(tty->dev, "bad idx %d\n", idx);
return -1;
}
@@ -1648,20 +1648,20 @@ static int tty_release_checks(struct tty_struct *tty, int idx)
return 0;
if (tty != tty->driver->ttys[idx]) {
- tty_debug(tty, "bad driver table[%d] = %p\n",
- idx, tty->driver->ttys[idx]);
+ dev_dbg(tty->dev, "bad driver table[%d] = %p\n",
+ idx, tty->driver->ttys[idx]);
return -1;
}
if (tty->driver->other) {
struct tty_struct *o_tty = tty->link;
if (o_tty != tty->driver->other->ttys[idx]) {
- tty_debug(tty, "bad other table[%d] = %p\n",
- idx, tty->driver->other->ttys[idx]);
+ dev_dbg(tty->dev, "bad other table[%d] = %p\n",
+ idx, tty->driver->other->ttys[idx]);
return -1;
}
if (o_tty->link != tty) {
- tty_debug(tty, "bad link = %p\n", o_tty->link);
+ dev_dbg(tty->dev, "bad link = %p\n", o_tty->link);
return -1;
}
}
diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 4de1c6ddb8ff..053290ab5cb8 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -28,7 +28,7 @@
#undef TTY_DEBUG_WAIT_UNTIL_SENT
#ifdef TTY_DEBUG_WAIT_UNTIL_SENT
-# define tty_debug_wait_until_sent(tty, f, args...) tty_debug(tty, f, ##args)
+# define tty_debug_wait_until_sent(tty, f, args...) dev_dbg(tty->dev, f, ##args)
#else
# define tty_debug_wait_until_sent(tty, f, args...) do {} while (0)
#endif
diff --git a/drivers/tty/tty_jobctrl.c b/drivers/tty/tty_jobctrl.c
index 19ec43a6ef76..bbc404255291 100644
--- a/drivers/tty/tty_jobctrl.c
+++ b/drivers/tty/tty_jobctrl.c
@@ -109,8 +109,8 @@ static void __proc_set_tty(struct tty_struct *tty)
tty->session = get_pid(task_session(current));
spin_unlock_irqrestore(&tty->ctrl_lock, flags);
if (current->signal->tty) {
- tty_debug(tty, "current tty %s not NULL!!\n",
- current->signal->tty->name);
+ dev_dbg(tty->dev, "current tty %s not NULL!!\n",
+ current->signal->tty->name);
tty_kref_put(current->signal->tty);
}
put_pid(current->signal->tty_old_pgrp);
diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 2e8da820c303..b2e821e14a13 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -24,7 +24,7 @@
#undef LDISC_DEBUG_HANGUP
#ifdef LDISC_DEBUG_HANGUP
-#define tty_ldisc_debug(tty, f, args...) tty_debug(tty, f, ##args)
+#define tty_ldisc_debug(tty, f, args...) dev_dbg(tty->dev, f, ##args)
#else
#define tty_ldisc_debug(tty, f, args...)
#endif
--
2.31.1
Move the TTY_LOCK_* enums and tty_ldisc lock functions out of the global
tty.h into the local header file to clean things up.
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/tty.h | 26 ++++++++++++++++++++++++++
drivers/tty/tty_buffer.c | 2 +-
drivers/tty/tty_mutex.c | 1 +
include/linux/tty.h | 26 --------------------------
4 files changed, 28 insertions(+), 27 deletions(-)
diff --git a/drivers/tty/tty.h b/drivers/tty/tty.h
index a8a7abe5d635..ff904e947483 100644
--- a/drivers/tty/tty.h
+++ b/drivers/tty/tty.h
@@ -6,6 +6,32 @@
#ifndef _TTY_INTERNAL_H
#define _TTY_INTERNAL_H
+/*
+ * Lock subclasses for tty locks
+ *
+ * TTY_LOCK_NORMAL is for normal ttys and master ptys.
+ * TTY_LOCK_SLAVE is for slave ptys only.
+ *
+ * Lock subclasses are necessary for handling nested locking with pty pairs.
+ * tty locks which use nested locking:
+ *
+ * legacy_mutex - Nested tty locks are necessary for releasing pty pairs.
+ * The stable lock order is master pty first, then slave pty.
+ * termios_rwsem - The stable lock order is tty_buffer lock->termios_rwsem.
+ * Subclassing this lock enables the slave pty to hold its
+ * termios_rwsem when claiming the master tty_buffer lock.
+ * tty_buffer lock - slave ptys can claim nested buffer lock when handling
+ * signal chars. The stable lock order is slave pty, then
+ * master.
+ */
+enum {
+ TTY_LOCK_NORMAL = 0,
+ TTY_LOCK_SLAVE,
+};
+
+int tty_ldisc_lock(struct tty_struct *tty, unsigned long timeout);
+void tty_ldisc_unlock(struct tty_struct *tty);
+
/* tty_audit.c */
#ifdef CONFIG_AUDIT
void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size);
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 6d4995a5f318..9733469a14b2 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -17,7 +17,7 @@
#include <linux/delay.h>
#include <linux/module.h>
#include <linux/ratelimit.h>
-
+#include "tty.h"
#define MIN_TTYB_SIZE 256
#define TTYB_ALIGN_MASK 255
diff --git a/drivers/tty/tty_mutex.c b/drivers/tty/tty_mutex.c
index 2640635ee177..393518a24cfe 100644
--- a/drivers/tty/tty_mutex.c
+++ b/drivers/tty/tty_mutex.c
@@ -4,6 +4,7 @@
#include <linux/kallsyms.h>
#include <linux/semaphore.h>
#include <linux/sched.h>
+#include "tty.h"
/* Legacy tty mutex glue */
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 981ee31c58e1..4b21d47bc098 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -16,30 +16,6 @@
#include <linux/llist.h>
-/*
- * Lock subclasses for tty locks
- *
- * TTY_LOCK_NORMAL is for normal ttys and master ptys.
- * TTY_LOCK_SLAVE is for slave ptys only.
- *
- * Lock subclasses are necessary for handling nested locking with pty pairs.
- * tty locks which use nested locking:
- *
- * legacy_mutex - Nested tty locks are necessary for releasing pty pairs.
- * The stable lock order is master pty first, then slave pty.
- * termios_rwsem - The stable lock order is tty_buffer lock->termios_rwsem.
- * Subclassing this lock enables the slave pty to hold its
- * termios_rwsem when claiming the master tty_buffer lock.
- * tty_buffer lock - slave ptys can claim nested buffer lock when handling
- * signal chars. The stable lock order is slave pty, then
- * master.
- */
-
-enum {
- TTY_LOCK_NORMAL = 0,
- TTY_LOCK_SLAVE,
-};
-
/*
* (Note: the *_driver.minor_start values 1, 64, 128, 192 are
* hardcoded at present.)
@@ -419,8 +395,6 @@ extern struct tty_struct *tty_kopen_exclusive(dev_t device);
extern struct tty_struct *tty_kopen_shared(dev_t device);
extern void tty_kclose(struct tty_struct *tty);
extern int tty_dev_name_to_number(const char *name, dev_t *number);
-extern int tty_ldisc_lock(struct tty_struct *tty, unsigned long timeout);
-extern void tty_ldisc_unlock(struct tty_struct *tty);
extern ssize_t redirected_tty_write(struct kiocb *, struct iov_iter *);
extern struct file *tty_release_redirect(struct tty_struct *tty);
#else
--
2.31.1
On 2021/04/08 21:51, Greg Kroah-Hartman wrote:
> Remove users of tty_warn() and replace them with calls to dev_warn()
> which provides more information about the tty that has the error and
> uses the standard formatting logic.
Ouch. This series would be good for clean up, but this series might be
bad for handling lockdep warning syzbot is reporting.
Since tty_warn() is using plain printk(), we can avoid lockdep warning by
using printk_deferred(). If we use dev_warn() instead, we need to modify
__dev_printk() to use printk_deferred(), which means that all dev_*() users
are affected by this change.
Also, we need to modify dev_printk_emit()/dev_vprintk_emit() callers to embed
loglevel into the format string so that we pass LOGLEVEL_SCHED to vprintk_emit() ...
maybe just change from "if (!in_sched)" to "if (!in_sched && !dev_info)" instead ?
Also, dev_vprintk_emit() need to start calling defer_console_output()
after returning from vprintk_emit() in order to behave like printk_deferred().
I'm not sure whether this change is safe.
On Thu, Apr 08, 2021 at 02:51:21PM +0200, Greg Kroah-Hartman wrote:
> Turns out there is a lot of tty-internal stuff in include/linux/tty.h
> that do not belong there. Create a internal-to-the-tty-layer .h file
> for these types of things and move function prototypes to it instead of
> being in the system-wide header file.
>
> Along the way clean up the use of some old tty-only debugging macros and
> use the in-kernel dev_*() calls instead.
I'm afraid that's not a good idea since not all ttys have a
corresponding class device. Notable exception include pseudo terminals
and serdev.
While dev_printk() can handle a NULL device argument without crashing,
we'll actually lose log information by removing the tty printk helpers.
Johan
On Thu, Apr 08, 2021 at 02:51:34PM +0200, Greg Kroah-Hartman wrote:
> There are a lot of tty-core-only functions that are listed in
> include/linux/tty.h. Move them to drivers/tty/tty.h so that no one else
> can accidentally call them or think that they are public functions.
>
> Cc: Jiri Slaby <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> ---
> drivers/tty/n_gsm.c | 1 +
> drivers/tty/n_hdlc.c | 1 +
> drivers/tty/tty.h | 37 +++++++++++++++++++++++++++++++++++++
> include/linux/tty.h | 34 ----------------------------------
> 4 files changed, 39 insertions(+), 34 deletions(-)
This needs a "tty.h" inclusion into drivers/tty/tty_baudrate.c,
otherwise it's a build warning, I missed that, sorry. Will add that to
the next revision if it's needed, or just fix it up when committing it.
thanks,
greg k-h
On Thu, Apr 08, 2021 at 04:25:22PM +0200, Johan Hovold wrote:
> On Thu, Apr 08, 2021 at 02:51:21PM +0200, Greg Kroah-Hartman wrote:
> > Turns out there is a lot of tty-internal stuff in include/linux/tty.h
> > that do not belong there. Create a internal-to-the-tty-layer .h file
> > for these types of things and move function prototypes to it instead of
> > being in the system-wide header file.
> >
> > Along the way clean up the use of some old tty-only debugging macros and
> > use the in-kernel dev_*() calls instead.
>
> I'm afraid that's not a good idea since not all ttys have a
> corresponding class device. Notable exception include pseudo terminals
> and serdev.
>
> While dev_printk() can handle a NULL device argument without crashing,
> we'll actually lose log information by removing the tty printk helpers.
I think the same info will be printed here as before, just some NULL
information at the beginning, right? And the benifits overall (for real
tty devices), should outweigh the few devices that do not have this
information.
But let me run some tests, on those devices to see just how this
looks...
thanks,
greg k-h
On Thu, Apr 08, 2021 at 10:47:21PM +0900, Tetsuo Handa wrote:
> On 2021/04/08 21:51, Greg Kroah-Hartman wrote:
> > Remove users of tty_warn() and replace them with calls to dev_warn()
> > which provides more information about the tty that has the error and
> > uses the standard formatting logic.
>
> Ouch. This series would be good for clean up, but this series might be
> bad for handling lockdep warning syzbot is reporting.
Again, we can worry about lockdep stuff for the real places where it
matters, which should not have been the same place as all of these were
used (they were used very infrequently.)
> Since tty_warn() is using plain printk(), we can avoid lockdep warning by
> using printk_deferred(). If we use dev_warn() instead, we need to modify
> __dev_printk() to use printk_deferred(), which means that all dev_*() users
> are affected by this change.
I don't want to use printk_deffered() if at all possible, let's let the
printk developers fix up their implementation which should make that
change not needed.
And worst case, take the few places that really really really need it,
and call printk_deferred() so it's obvious what we are doing.
> Also, we need to modify dev_printk_emit()/dev_vprintk_emit() callers to embed
> loglevel into the format string so that we pass LOGLEVEL_SCHED to vprintk_emit() ...
> maybe just change from "if (!in_sched)" to "if (!in_sched && !dev_info)" instead ?
Huh? No.
> Also, dev_vprintk_emit() need to start calling defer_console_output()
> after returning from vprintk_emit() in order to behave like printk_deferred().
Again, no. If we really need to deferr a printk, let's call that, but
that should not be the case for all of the places these macros were
used.
thanks,
greg k-h
On Thu, Apr 08, 2021 at 08:01:08PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Apr 08, 2021 at 04:25:22PM +0200, Johan Hovold wrote:
> > On Thu, Apr 08, 2021 at 02:51:21PM +0200, Greg Kroah-Hartman wrote:
> > > Turns out there is a lot of tty-internal stuff in include/linux/tty.h
> > > that do not belong there. Create a internal-to-the-tty-layer .h file
> > > for these types of things and move function prototypes to it instead of
> > > being in the system-wide header file.
> > >
> > > Along the way clean up the use of some old tty-only debugging macros and
> > > use the in-kernel dev_*() calls instead.
> >
> > I'm afraid that's not a good idea since not all ttys have a
> > corresponding class device. Notable exception include pseudo terminals
> > and serdev.
> >
> > While dev_printk() can handle a NULL device argument without crashing,
> > we'll actually lose log information by removing the tty printk helpers.
>
> I think the same info will be printed here as before, just some NULL
> information at the beginning, right? And the benifits overall (for real
> tty devices), should outweigh the few devices that do not have this
> information.
No, you'll only be losing information (tty driver and tty name). Here's
a pty example, where the first line in each pair use dev_info() and the
second tty_info():
[ 10.235331] (NULL device *): tty_get_device
[ 10.235441] ptm ptm0: tty_get_device
[ 10.235586] (NULL device *): tty_get_device
[ 10.235674] pts pts0: tty_get_device
and similar for serdev, which is becoming more and more common.
Johan
On Fri, Apr 09, 2021 at 09:32:45AM +0200, Johan Hovold wrote:
> On Thu, Apr 08, 2021 at 08:01:08PM +0200, Greg Kroah-Hartman wrote:
> > On Thu, Apr 08, 2021 at 04:25:22PM +0200, Johan Hovold wrote:
> > > On Thu, Apr 08, 2021 at 02:51:21PM +0200, Greg Kroah-Hartman wrote:
> > > > Turns out there is a lot of tty-internal stuff in include/linux/tty.h
> > > > that do not belong there. Create a internal-to-the-tty-layer .h file
> > > > for these types of things and move function prototypes to it instead of
> > > > being in the system-wide header file.
> > > >
> > > > Along the way clean up the use of some old tty-only debugging macros and
> > > > use the in-kernel dev_*() calls instead.
> > >
> > > I'm afraid that's not a good idea since not all ttys have a
> > > corresponding class device. Notable exception include pseudo terminals
> > > and serdev.
> > >
> > > While dev_printk() can handle a NULL device argument without crashing,
> > > we'll actually lose log information by removing the tty printk helpers.
> >
> > I think the same info will be printed here as before, just some NULL
> > information at the beginning, right? And the benifits overall (for real
> > tty devices), should outweigh the few devices that do not have this
> > information.
>
> No, you'll only be losing information (tty driver and tty name). Here's
> a pty example, where the first line in each pair use dev_info() and the
> second tty_info():
>
> [ 10.235331] (NULL device *): tty_get_device
> [ 10.235441] ptm ptm0: tty_get_device
>
> [ 10.235586] (NULL device *): tty_get_device
> [ 10.235674] pts pts0: tty_get_device
>
> and similar for serdev, which is becoming more and more common.
Ok, good point, I'll go apply only the first 2 patches in this series
(moving the macros out of tty.h and removing the unused one) and then
will redo this set of patches again. I think a better tty_msg() macro
is warrented so that we can provide dev_*() output if we have a device,
otherwise fall back to the old style to preserve functionality.
thanks,
greg k-h
On Thu, Apr 15, 2021 at 10:21:54AM +0200, Greg Kroah-Hartman wrote:
> On Fri, Apr 09, 2021 at 09:32:45AM +0200, Johan Hovold wrote:
> > On Thu, Apr 08, 2021 at 08:01:08PM +0200, Greg Kroah-Hartman wrote:
> > > On Thu, Apr 08, 2021 at 04:25:22PM +0200, Johan Hovold wrote:
> > > > On Thu, Apr 08, 2021 at 02:51:21PM +0200, Greg Kroah-Hartman wrote:
> > > > > Turns out there is a lot of tty-internal stuff in include/linux/tty.h
> > > > > that do not belong there. Create a internal-to-the-tty-layer .h file
> > > > > for these types of things and move function prototypes to it instead of
> > > > > being in the system-wide header file.
> > > > >
> > > > > Along the way clean up the use of some old tty-only debugging macros and
> > > > > use the in-kernel dev_*() calls instead.
> > > >
> > > > I'm afraid that's not a good idea since not all ttys have a
> > > > corresponding class device. Notable exception include pseudo terminals
> > > > and serdev.
> > > >
> > > > While dev_printk() can handle a NULL device argument without crashing,
> > > > we'll actually lose log information by removing the tty printk helpers.
> > >
> > > I think the same info will be printed here as before, just some NULL
> > > information at the beginning, right? And the benifits overall (for real
> > > tty devices), should outweigh the few devices that do not have this
> > > information.
> >
> > No, you'll only be losing information (tty driver and tty name). Here's
> > a pty example, where the first line in each pair use dev_info() and the
> > second tty_info():
> >
> > [ 10.235331] (NULL device *): tty_get_device
> > [ 10.235441] ptm ptm0: tty_get_device
> >
> > [ 10.235586] (NULL device *): tty_get_device
> > [ 10.235674] pts pts0: tty_get_device
> >
> > and similar for serdev, which is becoming more and more common.
>
> Ok, good point, I'll go apply only the first 2 patches in this series
> (moving the macros out of tty.h and removing the unused one) and then
> will redo this set of patches again.
Perhaps no harm in leaving the tty_info() on in there for consistency.
We have users of the _ratelimited() flavour of it (even if there's no
dependency).
> I think a better tty_msg() macro is warrented so that we can provide
> dev_*() output if we have a device, otherwise fall back to the old
> style to preserve functionality.
Possibly, but the dev_printk() for the tty class devices wouldn't
provide any more info than what's already there (i.e. driver name + tty
name).
(And associating ttys with other devices and drivers (e.g. a serdev
client and its driver) might not be what we want since you lose the
connection to the underlying tty driver.)
Johan
On Thu, Apr 15, 2021 at 04:14:33PM +0200, Johan Hovold wrote:
> On Thu, Apr 15, 2021 at 10:21:54AM +0200, Greg Kroah-Hartman wrote:
> > On Fri, Apr 09, 2021 at 09:32:45AM +0200, Johan Hovold wrote:
> > > On Thu, Apr 08, 2021 at 08:01:08PM +0200, Greg Kroah-Hartman wrote:
> > > > On Thu, Apr 08, 2021 at 04:25:22PM +0200, Johan Hovold wrote:
> > > > > On Thu, Apr 08, 2021 at 02:51:21PM +0200, Greg Kroah-Hartman wrote:
> > > > > > Turns out there is a lot of tty-internal stuff in include/linux/tty.h
> > > > > > that do not belong there. Create a internal-to-the-tty-layer .h file
> > > > > > for these types of things and move function prototypes to it instead of
> > > > > > being in the system-wide header file.
> > > > > >
> > > > > > Along the way clean up the use of some old tty-only debugging macros and
> > > > > > use the in-kernel dev_*() calls instead.
> > > > >
> > > > > I'm afraid that's not a good idea since not all ttys have a
> > > > > corresponding class device. Notable exception include pseudo terminals
> > > > > and serdev.
> > > > >
> > > > > While dev_printk() can handle a NULL device argument without crashing,
> > > > > we'll actually lose log information by removing the tty printk helpers.
> > > >
> > > > I think the same info will be printed here as before, just some NULL
> > > > information at the beginning, right? And the benifits overall (for real
> > > > tty devices), should outweigh the few devices that do not have this
> > > > information.
> > >
> > > No, you'll only be losing information (tty driver and tty name). Here's
> > > a pty example, where the first line in each pair use dev_info() and the
> > > second tty_info():
> > >
> > > [ 10.235331] (NULL device *): tty_get_device
> > > [ 10.235441] ptm ptm0: tty_get_device
> > >
> > > [ 10.235586] (NULL device *): tty_get_device
> > > [ 10.235674] pts pts0: tty_get_device
> > >
> > > and similar for serdev, which is becoming more and more common.
> >
> > Ok, good point, I'll go apply only the first 2 patches in this series
> > (moving the macros out of tty.h and removing the unused one) and then
> > will redo this set of patches again.
>
> Perhaps no harm in leaving the tty_info() on in there for consistency.
> We have users of the _ratelimited() flavour of it (even if there's no
> dependency).
I dropped it, no need to keep around unused macros :)
> > I think a better tty_msg() macro is warrented so that we can provide
> > dev_*() output if we have a device, otherwise fall back to the old
> > style to preserve functionality.
>
> Possibly, but the dev_printk() for the tty class devices wouldn't
> provide any more info than what's already there (i.e. driver name + tty
> name).
>
> (And associating ttys with other devices and drivers (e.g. a serdev
> client and its driver) might not be what we want since you lose the
> connection to the underlying tty driver.)
Yeah, I messed with this for a bit today and I think I'm just going to
give up and leave it as-is for now...
thanks,
greg k-h