Hi Greg,
As you requested [1], I re-worked the tty_debug() macro to format printk
output like dev_dbg(), ie., <driver> <tty name>: <fmt>. At the same
time, I rolled in Joe's original suggestions [2].
This series also removes the __func__ from tty_debug() since it's
redundant when dynamic_debug is enabled (I edited the messages to
provide enough context if dynamic_debug is not enabled).
The remainder of this series adds tty_*() printk macros (like netdev_*())
and cleans up printk() usage in the tty core.
[1] http://www.gossamer-threads.com/lists/linux/kernel/2232363#2232363
[2] http://www.gossamer-threads.com/lists/linux/kernel/2213266#2213266
Regards,
Peter Hurley (14):
tty: Improve tty_debug() macro
tty: Make tty_paranoia_check() file scope
tty: synclink_gt: Rename tty_driver_name
tty: core: Remove redundant oom message
tty: core: Add helper fn to deref tty driver name
tty: Define tty_*() printk macros
tty: Convert SAK messages to tty_notice()
tty: core: Add driver name to invalid device registration message
tty: core: Refactor parameters for unset_locked_termios() helper
tty: Remove unset_locked_termios() error message
tty: core: Prefer pr_* to printk(*)
tty: Remove __func__ from tty_debug() macro
tty: Merge conditional + error message + WARN_ON()
tty: core: Prefer dev_dbg() over pr_debug()
drivers/tty/n_tty.c | 7 ++--
drivers/tty/pty.c | 2 +-
drivers/tty/synclink_gt.c | 4 +--
drivers/tty/tty_io.c | 86 +++++++++++++++++++++--------------------------
drivers/tty/tty_ioctl.c | 21 ++++--------
drivers/tty/tty_mutex.c | 10 ++----
drivers/tty/tty_port.c | 9 +++--
include/linux/tty.h | 19 +++++++----
8 files changed, 69 insertions(+), 89 deletions(-)
--
2.6.3
Incorporate suggestions for tty core debug macro improvements
- printk(KERN_DEBUG) => pr_debug()
- ##args => ##__VA_ARGS__
- remove do {} while()
- output tty_name() first
cc: Joe Perches <[email protected]>
Signed-off-by: Peter Hurley <[email protected]>
---
include/linux/tty.h | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 5e31f1b..3695c88 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -667,10 +667,7 @@ static inline void proc_tty_register_driver(struct tty_driver *d) {}
static inline void proc_tty_unregister_driver(struct tty_driver *d) {}
#endif
-#define tty_debug(tty, f, args...) \
- do { \
- printk(KERN_DEBUG "%s: %s: " f, __func__, \
- tty_name(tty), ##args); \
- } while (0)
+#define tty_debug(tty, f, ...) \
+ pr_debug("%s: %s: " f, tty_name(tty), __func__, ##__VA_ARGS__)
#endif
--
2.6.3
tty_paranoia_check() is only used within drivers/tty/tty_io.c;
remove extern declaration in header and limit symbol to file scope.
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/tty_io.c | 2 +-
include/linux/tty.h | 2 --
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 0c41dbc..aef9bc1 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -256,7 +256,7 @@ const char *tty_name(const struct tty_struct *tty)
EXPORT_SYMBOL(tty_name);
-int tty_paranoia_check(struct tty_struct *tty, struct inode *inode,
+static int tty_paranoia_check(struct tty_struct *tty, struct inode *inode,
const char *routine)
{
#ifdef TTY_PARANOIA_CHECK
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 3695c88..0532465 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -419,8 +419,6 @@ static inline struct tty_struct *tty_kref_get(struct tty_struct *tty)
return tty;
}
-extern int tty_paranoia_check(struct tty_struct *tty, struct inode *inode,
- const char *routine);
extern const char *tty_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);
--
2.6.3
Eliminate symbol name collision with new tty core function,
tty_driver_name().
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/synclink_gt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/synclink_gt.c b/drivers/tty/synclink_gt.c
index 6fc39fb..5505ea8 100644
--- a/drivers/tty/synclink_gt.c
+++ b/drivers/tty/synclink_gt.c
@@ -89,7 +89,7 @@
* module identification
*/
static char *driver_name = "SyncLink GT";
-static char *tty_driver_name = "synclink_gt";
+static char *slgt_driver_name = "synclink_gt";
static char *tty_dev_prefix = "ttySLG";
MODULE_LICENSE("GPL");
#define MGSL_MAGIC 0x5401
@@ -3799,7 +3799,7 @@ static int __init slgt_init(void)
/* Initialize the tty_driver structure */
- serial_driver->driver_name = tty_driver_name;
+ serial_driver->driver_name = slgt_driver_name;
serial_driver->name = tty_dev_prefix;
serial_driver->major = ttymajor;
serial_driver->minor_start = 64;
--
2.6.3
kmalloc() already emits a diagnostic for failed allocations; remove
tty-specific message.
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/tty_io.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index aef9bc1..f463894 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1576,10 +1576,8 @@ void tty_free_termios(struct tty_struct *tty)
tp = tty->driver->termios[idx];
if (tp == NULL) {
tp = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
- if (tp == NULL) {
- pr_warn("tty: no memory to save termios state.\n");
+ if (tp == NULL)
return;
- }
tty->driver->termios[idx] = tp;
}
*tp = tty->termios;
--
2.6.3
Similar to tty_name(), add tty_driver_name() helper to safely
dereference tty->driver->name (otherwise return empty string).
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/tty_io.c | 7 +++++++
include/linux/tty.h | 1 +
2 files changed, 8 insertions(+)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index f463894..7a7b77d 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -256,6 +256,13 @@ 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 0532465..a9c1af9 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -420,6 +420,7 @@ static inline struct tty_struct *tty_kref_get(struct tty_struct *tty)
}
extern const char *tty_name(const struct tty_struct *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.6.3
Since not all ttys are devices (eg., SysV ptys), dev_*() printk macros
cannot be used. Define tty_*() printk macros that output in similar
format to dev_*() macros (ie., <driver> <tty>: .....).
Transform the most-trivial printk( LEVEL ...) usage to tty_*() usage.
NB: The function name has been eliminated from messages with unique
context, or prefixed to the format when given.
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/n_tty.c | 7 ++-----
drivers/tty/tty_io.c | 27 ++++++++++-----------------
drivers/tty/tty_port.c | 9 ++++-----
include/linux/tty.h | 12 +++++++++++-
4 files changed, 27 insertions(+), 28 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index ed77614..c37c15d 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1201,9 +1201,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)) {
- printk(KERN_WARNING "%s: %d input overrun(s)\n",
- tty_name(tty),
- ldata->num_overrun);
+ tty_warn(tty, "%d input overrun(s)\n", ldata->num_overrun);
ldata->overrun_time = jiffies;
ldata->num_overrun = 0;
}
@@ -1486,8 +1484,7 @@ n_tty_receive_char_flagged(struct tty_struct *tty, unsigned char c, char flag)
n_tty_receive_overrun(tty);
break;
default:
- printk(KERN_ERR "%s: unknown flag %d\n",
- tty_name(tty), flag);
+ tty_err(tty, "unknown flag %d\n", flag);
break;
}
}
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 7a7b77d..121dbeb 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -300,9 +300,8 @@ static int check_tty_count(struct tty_struct *tty, const char *routine)
tty->link && tty->link->count)
count++;
if (tty->count != count) {
- printk(KERN_WARNING "Warning: dev (%s) tty->count(%d) "
- "!= #fd's(%d) in %s\n",
- tty->name, tty->count, count, routine);
+ tty_warn(tty, "%s: tty->count(%d) != #fd's(%d)\n",
+ routine, tty->count, count);
return count;
}
#endif
@@ -427,10 +426,8 @@ int __tty_check_change(struct tty_struct *tty, int sig)
}
rcu_read_unlock();
- if (!tty_pgrp) {
- pr_warn("%s: tty_check_change: sig=%d, tty->pgrp == NULL!\n",
- tty_name(tty), sig);
- }
+ if (!tty_pgrp)
+ tty_warn(tty, "sig=%d, tty->pgrp == NULL!\n", sig);
return ret;
}
@@ -1246,8 +1243,7 @@ static ssize_t tty_write(struct file *file, const char __user *buf,
return -EIO;
/* Short term debug to catch buggy drivers */
if (tty->ops->write_room == NULL)
- printk(KERN_ERR "tty driver %s lacks a write_room method.\n",
- tty->driver->name);
+ tty_err(tty, "missing write_room method\n");
ld = tty_ldisc_ref_wait(tty);
if (!ld->ops->write)
ret = -EIO;
@@ -1564,8 +1560,8 @@ err_module_put:
/* call the tty release_tty routine to clean out this slot */
err_release_tty:
tty_unlock(tty);
- printk_ratelimited(KERN_INFO "tty_init_dev: ldisc open failed, "
- "clearing slot %d\n", idx);
+ tty_info_ratelimited(tty, "ldisc open failed (%d), clearing slot %d\n",
+ retval, idx);
release_tty(tty, idx);
return ERR_PTR(retval);
}
@@ -1838,8 +1834,7 @@ int tty_release(struct inode *inode, struct file *filp)
if (once) {
once = 0;
- printk(KERN_WARNING "%s: %s: read/write wait queue active!\n",
- __func__, tty_name(tty));
+ tty_warn(tty, "read/write wait queue active!\n");
}
schedule_timeout_killable(timeout);
if (timeout < 120 * HZ)
@@ -1850,14 +1845,12 @@ int tty_release(struct inode *inode, struct file *filp)
if (o_tty) {
if (--o_tty->count < 0) {
- printk(KERN_WARNING "%s: bad pty slave count (%d) for %s\n",
- __func__, o_tty->count, tty_name(o_tty));
+ tty_warn(tty, "bad slave count (%d)\n", o_tty->count);
o_tty->count = 0;
}
}
if (--tty->count < 0) {
- printk(KERN_WARNING "%s: bad tty->count (%d) for %s\n",
- __func__, tty->count, tty_name(tty));
+ tty_warn(tty, "bad tty->count (%d)\n", tty->count);
tty->count = 0;
}
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index 482f33f..846ed48 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -462,14 +462,13 @@ int tty_port_close_start(struct tty_port *port,
spin_lock_irqsave(&port->lock, flags);
if (tty->count == 1 && port->count != 1) {
- printk(KERN_WARNING
- "tty_port_close_start: tty->count = 1 port count = %d.\n",
- port->count);
+ tty_warn(tty, "%s: tty->count = 1 port count = %d\n", __func__,
+ port->count);
port->count = 1;
}
if (--port->count < 0) {
- printk(KERN_WARNING "tty_port_close_start: count = %d\n",
- port->count);
+ tty_warn(tty, "%s: bad port count (%d)\n", __func__,
+ port->count);
port->count = 0;
}
diff --git a/include/linux/tty.h b/include/linux/tty.h
index a9c1af9..f578e84 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -666,7 +666,17 @@ static inline void proc_tty_register_driver(struct tty_driver *d) {}
static inline void proc_tty_unregister_driver(struct tty_driver *d) {}
#endif
+#define tty_msg(fn, tty, f, ...) \
+ fn("%s %s: " f, tty_driver_name(tty), tty_name(tty), ##__VA_ARGS__)
+
#define tty_debug(tty, f, ...) \
- pr_debug("%s: %s: " f, tty_name(tty), __func__, ##__VA_ARGS__)
+ tty_msg(pr_debug, tty, "%s:" f, __func__, ##__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__)
+
+#define tty_info_ratelimited(tty, f, ...) \
+ tty_msg(pr_info_ratelimited, tty, f, ##__VA_ARGS__)
#endif
--
2.6.3
Use tty_notice() for unified message format from the tty core.
Fix each message to accurately reflect the cause of each termination.
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/tty_io.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 121dbeb..a1a87d4 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3022,28 +3022,24 @@ void __do_SAK(struct tty_struct *tty)
read_lock(&tasklist_lock);
/* Kill the entire session */
do_each_pid_task(session, PIDTYPE_SID, p) {
- printk(KERN_NOTICE "SAK: killed process %d"
- " (%s): task_session(p)==tty->session\n",
- task_pid_nr(p), p->comm);
+ tty_notice(tty, "SAK: killed process %d (%s): by session\n",
+ task_pid_nr(p), p->comm);
send_sig(SIGKILL, p, 1);
} while_each_pid_task(session, PIDTYPE_SID, p);
- /* Now kill any processes that happen to have the
- * tty open.
- */
+
+ /* Now kill any processes that happen to have the tty open */
do_each_thread(g, p) {
if (p->signal->tty == tty) {
- printk(KERN_NOTICE "SAK: killed process %d"
- " (%s): task_session(p)==tty->session\n",
- task_pid_nr(p), p->comm);
+ tty_notice(tty, "SAK: killed process %d (%s): by controlling tty\n",
+ task_pid_nr(p), p->comm);
send_sig(SIGKILL, p, 1);
continue;
}
task_lock(p);
i = iterate_fd(p->files, 0, this_tty, tty);
if (i != 0) {
- printk(KERN_NOTICE "SAK: killed process %d"
- " (%s): fd#%d opened to the tty\n",
- task_pid_nr(p), p->comm, i - 1);
+ tty_notice(tty, "SAK: killed process %d (%s): by fd#%d\n",
+ task_pid_nr(p), p->comm, i - 1);
force_sig(SIGKILL, p);
}
task_unlock(p);
--
2.6.3
Include the driver name in the tty_register_device_attr() error
message for invalid index.
Note that tty_err() cannot be used here because there is no tty;
use pr_err().
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/tty_io.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index a1a87d4..b26b83b 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3245,8 +3245,8 @@ struct device *tty_register_device_attr(struct tty_driver *driver,
bool cdev = false;
if (index >= driver->num) {
- printk(KERN_ERR "Attempt to register invalid tty line number "
- " (%d).\n", index);
+ pr_err("%s: Attempt to register invalid tty line number (%d)\n",
+ driver->name, index);
return ERR_PTR(-EINVAL);
}
--
2.6.3
Add tty as parameter to unset_locked_termios() and extract former
parameters, termios and locked, as locals.
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/tty_ioctl.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 9c5aebf..40d91a3 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -239,10 +239,10 @@ EXPORT_SYMBOL(tty_wait_until_sent);
* Termios Helper Methods
*/
-static void unset_locked_termios(struct ktermios *termios,
- struct ktermios *old,
- struct ktermios *locked)
+static void unset_locked_termios(struct tty_struct *tty, struct ktermios *old)
{
+ struct ktermios *termios = &tty->termios;
+ struct ktermios *locked = &tty->termios_locked;
int i;
#define NOSET_MASK(x, y, z) (x = ((x) & ~(z)) | ((y) & (z)))
@@ -556,7 +556,7 @@ int tty_set_termios(struct tty_struct *tty, struct ktermios *new_termios)
down_write(&tty->termios_rwsem);
old_termios = tty->termios;
tty->termios = *new_termios;
- unset_locked_termios(&tty->termios, &old_termios, &tty->termios_locked);
+ unset_locked_termios(tty, &old_termios);
if (tty->ops->set_termios)
tty->ops->set_termios(tty, &old_termios);
--
2.6.3
With the refactor of 'locked' from parameter to local,
it's now obvious locked cannot be NULL. Remove entire conditional.
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/tty_ioctl.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 40d91a3..4957e4c 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -247,11 +247,6 @@ static void unset_locked_termios(struct tty_struct *tty, struct ktermios *old)
#define NOSET_MASK(x, y, z) (x = ((x) & ~(z)) | ((y) & (z)))
- if (!locked) {
- printk(KERN_WARNING "Warning?!? termios_locked is NULL.\n");
- return;
- }
-
NOSET_MASK(termios->c_iflag, old->c_iflag, locked->c_iflag);
NOSET_MASK(termios->c_oflag, old->c_oflag, locked->c_oflag);
NOSET_MASK(termios->c_cflag, old->c_cflag, locked->c_cflag);
--
2.6.3
Convert remaining printk() use to pr_*() when tty is unknown or
unsafe to use.
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/tty_io.c | 6 ++----
drivers/tty/tty_ioctl.c | 6 ++----
2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index b26b83b..04e8b56 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -268,14 +268,12 @@ static int tty_paranoia_check(struct tty_struct *tty, struct inode *inode,
{
#ifdef TTY_PARANOIA_CHECK
if (!tty) {
- printk(KERN_WARNING
- "null TTY for (%d:%d) in %s\n",
+ pr_warn("(%d:%d): %s: NULL tty\n",
imajor(inode), iminor(inode), routine);
return 1;
}
if (tty->magic != TTY_MAGIC) {
- printk(KERN_WARNING
- "bad magic number for tty struct (%d:%d) in %s\n",
+ pr_warn("(%d:%d): %s: bad magic number\n",
imajor(inode), iminor(inode), routine);
return 1;
}
diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 4957e4c..6063912 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -458,10 +458,8 @@ void tty_termios_encode_baud_rate(struct ktermios *termios,
if (ifound == -1 && (ibaud != obaud || ibinput))
termios->c_cflag |= (BOTHER << IBSHIFT);
#else
- if (ifound == -1 || ofound == -1) {
- printk_once(KERN_WARNING "tty: Unable to return correct "
- "speed data as your architecture needs updating.\n");
- }
+ if (ifound == -1 || ofound == -1)
+ pr_warn_once("tty: Unable to return correct speed data as your architecture needs updating.\n");
#endif
}
EXPORT_SYMBOL_GPL(tty_termios_encode_baud_rate);
--
2.6.3
Now that tty_debug() macro uses pr_debug(), the function name can
be printed when using dynamic debug; printing the function name within
the format string is redundant.
Remove the __func__ parameter and print specifier from the format string.
Add context to messages for when the function name is not printed by
dynamic debug, or when dynamic debug is not enabled.
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/pty.c | 2 +-
drivers/tty/tty_io.c | 14 +++++++-------
drivers/tty/tty_ioctl.c | 2 +-
include/linux/tty.h | 3 +--
4 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index a45660f..b311004 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -788,7 +788,7 @@ static int ptmx_open(struct inode *inode, struct file *filp)
if (retval)
goto err_release;
- tty_debug_hangup(tty, "(tty count=%d)\n", tty->count);
+ tty_debug_hangup(tty, "opening (count=%d)\n", tty->count);
tty_unlock(tty);
return 0;
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 04e8b56..479e9c2 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -783,7 +783,7 @@ static void do_tty_hangup(struct work_struct *work)
void tty_hangup(struct tty_struct *tty)
{
- tty_debug_hangup(tty, "\n");
+ tty_debug_hangup(tty, "hangup\n");
schedule_work(&tty->hangup_work);
}
@@ -800,7 +800,7 @@ EXPORT_SYMBOL(tty_hangup);
void tty_vhangup(struct tty_struct *tty)
{
- tty_debug_hangup(tty, "\n");
+ tty_debug_hangup(tty, "vhangup\n");
__tty_hangup(tty, 0);
}
@@ -837,7 +837,7 @@ void tty_vhangup_self(void)
static void tty_vhangup_session(struct tty_struct *tty)
{
- tty_debug_hangup(tty, "\n");
+ tty_debug_hangup(tty, "session hangup\n");
__tty_hangup(tty, 1);
}
@@ -1783,7 +1783,7 @@ int tty_release(struct inode *inode, struct file *filp)
return 0;
}
- tty_debug_hangup(tty, "(tty count=%d)...\n", tty->count);
+ tty_debug_hangup(tty, "releasing (count=%d)\n", tty->count);
if (tty->ops->close)
tty->ops->close(tty, filp);
@@ -1899,7 +1899,7 @@ int tty_release(struct inode *inode, struct file *filp)
/* Wait for pending work before tty destruction commmences */
tty_flush_works(tty);
- tty_debug_hangup(tty, "freeing structure...\n");
+ tty_debug_hangup(tty, "freeing structure\n");
/*
* The release_tty function takes care of the details of clearing
* the slots and preserving the termios structure. The tty_unlock_pair
@@ -2089,7 +2089,7 @@ retry_open:
tty->driver->subtype == PTY_TYPE_MASTER)
noctty = 1;
- tty_debug_hangup(tty, "(tty count=%d)\n", tty->count);
+ tty_debug_hangup(tty, "opening (count=%d)\n", tty->count);
if (tty->ops->open)
retval = tty->ops->open(tty, filp);
@@ -2098,7 +2098,7 @@ retry_open:
filp->f_flags = saved_flags;
if (retval) {
- tty_debug_hangup(tty, "error %d, releasing...\n", retval);
+ tty_debug_hangup(tty, "open error %d, releasing\n", retval);
tty_unlock(tty); /* need to call tty_release without BTM */
tty_release(inode, filp);
diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 6063912..b8c5c12 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -216,7 +216,7 @@ int tty_unthrottle_safe(struct tty_struct *tty)
void tty_wait_until_sent(struct tty_struct *tty, long timeout)
{
- tty_debug_wait_until_sent(tty, "\n");
+ tty_debug_wait_until_sent(tty, "wait until sent, timeout=%ld\n", timeout);
if (!timeout)
timeout = MAX_SCHEDULE_TIMEOUT;
diff --git a/include/linux/tty.h b/include/linux/tty.h
index f578e84..f06dd7a 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -669,8 +669,7 @@ static inline void proc_tty_unregister_driver(struct tty_driver *d) {}
#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, "%s:" f, __func__, ##__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__)
--
2.6.3
WARN() does all of these things in one statement.
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/tty_mutex.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/tty_mutex.c b/drivers/tty/tty_mutex.c
index 0efcf71..77703a3 100644
--- a/drivers/tty/tty_mutex.c
+++ b/drivers/tty/tty_mutex.c
@@ -12,11 +12,8 @@
void __lockfunc tty_lock(struct tty_struct *tty)
{
- if (tty->magic != TTY_MAGIC) {
- pr_err("L Bad %p\n", tty);
- WARN_ON(1);
+ if (WARN(tty->magic != TTY_MAGIC, "L Bad %p\n", tty))
return;
- }
tty_kref_get(tty);
mutex_lock(&tty->legacy_mutex);
}
@@ -24,11 +21,8 @@ EXPORT_SYMBOL(tty_lock);
void __lockfunc tty_unlock(struct tty_struct *tty)
{
- if (tty->magic != TTY_MAGIC) {
- pr_err("U Bad %p\n", tty);
- WARN_ON(1);
+ if (WARN(tty->magic != TTY_MAGIC, "U Bad %p\n", tty))
return;
- }
mutex_unlock(&tty->legacy_mutex);
tty_kref_put(tty);
}
--
2.6.3
Where possible, use dev_dbg() instead of pr_debug()
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/tty_io.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 479e9c2..9fa5a4f 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3207,7 +3207,7 @@ EXPORT_SYMBOL(tty_register_device);
static void tty_device_create_release(struct device *dev)
{
- pr_debug("device: '%s': %s\n", dev_name(dev), __func__);
+ dev_dbg(dev, "releasing...\n");
kfree(dev);
}
--
2.6.3
On Sun, 2015-11-08 at 13:01 -0500, Peter Hurley wrote:
> Incorporate suggestions for tty core debug macro improvements
> - printk(KERN_DEBUG) => pr_debug()
> - ##args => ##__VA_ARGS__
> - remove do {} while()
> - output tty_name() first
[]
> diff --git a/include/linux/tty.h b/include/linux/tty.h
[]
> @@ -667,10 +667,7 @@ static inline void proc_tty_register_driver(struct tty_driver *d) {}
> static inline void proc_tty_unregister_driver(struct tty_driver *d) {}
> #endif
>
> -#define tty_debug(tty, f, args...) \
> - do { \
> - printk(KERN_DEBUG "%s: %s: " f, __func__, \
> - tty_name(tty), ##args); \
> - } while (0)
> +#define tty_debug(tty, f, ...) \
> + pr_debug("%s: %s: " f, tty_name(tty), __func__, ##__VA_ARGS__)
>
> #endif
Unless there's a #define DEBUG in the path, this now
basically requires dynamic debugging to output anything.
Given that, I suggest removing __func__ from the output
because dynamic debugging can add it.
And f is more commonly fmt, so:
#define tty_debug(tty, fmt, ...) \
pr_debug("%s: " fmt, tty_name(tty), ##__VA_ARGS__)
Hi Joe,
On 11/08/2015 01:51 PM, Joe Perches wrote:
> On Sun, 2015-11-08 at 13:01 -0500, Peter Hurley wrote:
>> Incorporate suggestions for tty core debug macro improvements
>> - printk(KERN_DEBUG) => pr_debug()
>> - ##args => ##__VA_ARGS__
>> - remove do {} while()
>> - output tty_name() first
> []
>> diff --git a/include/linux/tty.h b/include/linux/tty.h
> []
>> @@ -667,10 +667,7 @@ static inline void proc_tty_register_driver(struct tty_driver *d) {}
>> static inline void proc_tty_unregister_driver(struct tty_driver *d) {}
>> #endif
>>
>> -#define tty_debug(tty, f, args...) \
>> - do { \
>> - printk(KERN_DEBUG "%s: %s: " f, __func__, \
>> - tty_name(tty), ##args); \
>> - } while (0)
>> +#define tty_debug(tty, f, ...) \
>> + pr_debug("%s: %s: " f, tty_name(tty), __func__, ##__VA_ARGS__)
>>
>> #endif
>
> Unless there's a #define DEBUG in the path, this now
> basically requires dynamic debugging to output anything.
It's what Greg wanted.
> Given that, I suggest removing __func__ from the output
> because dynamic debugging can add it.
Done in a follow-on patch in the series because the messages needed
some (minor) context if the function name is not output.
> And f is more commonly fmt, so:
I have a terse style :)
Regards,
Peter Hurley
On Sun, 2015-11-08 at 14:30 -0500, Peter Hurley wrote:
> Hi Joe,
Hello Peter.
> > I suggest removing __func__ from the output
> > because dynamic debugging can add it.
>
> Done in a follow-on patch in the series because the messages needed
> some (minor) context if the function name is not output.
No worries, I wasn't cc'd.
For logging debug messages that need context, most likely
the function tracer could be used instead.