Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1690187
| From | Okash Khawaja <okash.khawaja@gmail.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export |
| Date | 2017-07-18 13:40 +0200 |
| Message-ID | <u4z19-1Qn-45@gated-at.bofh.it> (permalink) |
| References | (4 earlier) <u2uyB-3hF-7@gated-at.bofh.it> <u2KDo-51l-13@gated-at.bofh.it> <u4dtD-51t-3@gated-at.bofh.it> <u4eg2-5DQ-9@gated-at.bofh.it> <u4mng-2mh-29@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Mon, Jul 17, 2017 at 11:04:38PM +0100, Alan Cox wrote:
>
> > Sure. I can fix the tty->count mismatch based on Alan's suggestion. However I don't understand why the exclusivity flag should belong to tty_port and not tty_struct. It will be good to know why.
>
> We are trying to move all the flags that we can and structs into the
> tty_port, except any that are used internally within the struct tty
> level code. The main reason for this is to make the object lifetimes and
> locking simpler - because the tty_port lasts for the time the hardware is
> present.
I see. That does make sense. I have appended a version of the patch which
adds the flag to tty_port and uses that inside tty_kopen.
From readability point of view however, I think adding the flag to
tty_struct looks more intuitive. At least for now - until we move
other things from tty_struct to tty_port.
The patch is untested but I can work on this if that fits in with the
plans for tty.
Thanks,
Okash
---
drivers/tty/tty_io.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++----
include/linux/tty.h | 17 ++++++++++++
2 files changed, 79 insertions(+), 5 deletions(-)
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -280,7 +280,7 @@ static int check_tty_count(struct tty_st
{
#ifdef CHECK_TTY_COUNT
struct list_head *p;
- int count = 0;
+ int count = 0, kopen_count = 0;
spin_lock(&tty->files_lock);
list_for_each(p, &tty->tty_files) {
@@ -291,10 +291,12 @@ static int check_tty_count(struct tty_st
tty->driver->subtype == PTY_TYPE_SLAVE &&
tty->link && tty->link->count)
count++;
- if (tty->count != count) {
- tty_warn(tty, "%s: tty->count(%d) != #fd's(%d)\n",
- routine, tty->count, count);
- return count;
+ 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",
+ routine, tty->count, count, kopen_count);
+ return (count + kopen_count);
}
#endif
return 0;
@@ -1522,6 +1524,7 @@ static int tty_release_checks(struct tty
*/
void tty_release_struct(struct tty_struct *tty, int idx)
{
+ // TODO: reset TTY_PORT_KOPENED on tty->port
/*
* Ask the line discipline code to release its structures
*/
@@ -1786,6 +1789,54 @@ static struct tty_driver *tty_lookup_dri
}
/**
+ * tty_kopen - open a tty device for kernel
+ * @device: dev_t of device to open
+ *
+ * Opens tty exclusively for kernel. Performs the driver lookup,
+ * makes sure it's not already opened and performs the first-time
+ * tty initialization.
+ *
+ * Returns the locked initialized &tty_struct
+ *
+ * Claims the global tty_mutex to serialize:
+ * - concurrent first-time tty initialization
+ * - concurrent tty driver removal w/ lookup
+ * - concurrent tty removal from driver table
+ */
+struct tty_struct *tty_kopen(dev_t device)
+{
+ struct tty_struct *tty;
+ struct tty_driver *driver = NULL;
+ int index = -1;
+
+ mutex_lock(&tty_mutex);
+ driver = tty_lookup_driver(device, NULL, &index);
+ if (IS_ERR(driver)) {
+ mutex_unlock(&tty_mutex);
+ return ERR_CAST(driver);
+ }
+
+ /* check whether we're reopening an existing tty */
+ tty = tty_driver_lookup_tty(driver, NULL, index);
+ if (IS_ERR(tty))
+ goto out;
+
+ if (tty) {
+ /* drop kref from tty_driver_lookup_tty() */
+ tty_kref_put(tty);
+ tty = ERR_PTR(-EBUSY);
+ } else { /* tty_init_dev returns tty with the tty_lock held */
+ tty = tty_init_dev(driver, index);
+ tty_port_set_kopened(tty->port, 1);
+ }
+out:
+ mutex_unlock(&tty_mutex);
+ tty_driver_kref_put(driver);
+ return tty;
+}
+EXPORT_SYMBOL_GPL(tty_kopen);
+
+/**
* tty_open_by_driver - open a tty device
* @device: dev_t of device to open
* @inode: inode of device file
@@ -1824,6 +1875,12 @@ struct tty_struct *tty_open_by_driver(de
}
if (tty) {
+ if (tty_port_kopened(tty->port)) {
+ tty_kref_put(tty);
+ mutex_unlock(&tty_mutex);
+ tty = ERR_PTR(-EBUSY);
+ goto out;
+ }
mutex_unlock(&tty_mutex);
retval = tty_lock_interruptible(tty);
tty_kref_put(tty); /* drop kref from tty_driver_lookup_tty() */
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -261,6 +261,7 @@ struct tty_port {
*/
#define TTY_PORT_CTS_FLOW 3 /* h/w flow control enabled */
#define TTY_PORT_CHECK_CD 4 /* carrier detect enabled */
+#define TTY_PORT_KOPENED 5 /* device exclusively opened by kernel */
/*
* Where all of the state associated with a tty is kept while the tty
@@ -401,6 +402,7 @@ extern int __init tty_init(void);
extern const char *tty_name(const struct tty_struct *tty);
extern struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
struct file *filp);
+extern struct tty_struct *tty_kopen(dev_t device);
extern int tty_dev_name_to_number(const char *name, dev_t *number);
#else
static inline void tty_kref_put(struct tty_struct *tty)
@@ -425,6 +427,8 @@ static inline const char *tty_name(const
static inline struct tty_struct *tty_open_by_driver(dev_t device,
struct inode *inode, struct file *filp)
{ return NULL; }
+static inline struct tty_struct *tty_kopen(dev_t device)
+{ return NULL; }
static inline int tty_dev_name_to_number(const char *name, dev_t *number)
{ return -ENOTSUPP; }
#endif
@@ -652,6 +656,19 @@ static inline void tty_port_set_initiali
clear_bit(TTY_PORT_INITIALIZED, &port->iflags);
}
+static inline bool tty_port_kopened(struct tty_port *port)
+{
+ return test_bit(TTY_PORT_KOPENED, &port->iflags);
+}
+
+static inline void tty_port_set_kopened(struct tty_port *port, bool val)
+{
+ if (val)
+ set_bit(TTY_PORT_KOPENED, &port->iflags);
+ else
+ clear_bit(TTY_PORT_KOPENED, &port->iflags);
+}
+
extern struct tty_struct *tty_port_tty_get(struct tty_port *port);
extern void tty_port_tty_set(struct tty_port *port, struct tty_struct *tty);
extern int tty_port_carrier_raised(struct tty_port *port);
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
tty contention resulting from tty_open_by_device export Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-07 22:30 +0200
Re: tty contention resulting from tty_open_by_device export Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-07-08 10:40 +0200
Re: tty contention resulting from tty_open_by_device export Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-08 11:10 +0200
[patch 3/3] tty: undo export of tty_open_by_driver Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-09 13:50 +0200
[patch 0/3] Re: tty contention resulting from tty_open_by_device export Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-09 13:50 +0200
[patch 2/3] staging: speakup: use tty_kopen instead of tty_open_by_driver Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-09 13:50 +0200
Re: [patch 2/3] staging: speakup: use tty_kopen instead of tty_open_by_driver Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-07-09 14:00 +0200
Re: [patch 2/3] staging: speakup: use tty_kopen instead of tty_open_by_driver Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-09 14:30 +0200
[patch 1/3] tty: resolve tty contention between kernel and user space Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-09 13:50 +0200
Re: [patch 1/3] tty: resolve tty contention between kernel and user space Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-07-09 14:00 +0200
Re: [patch 1/3] tty: resolve tty contention between kernel and user space Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-07-09 17:10 +0200
Re: [patch 1/3] tty: resolve tty contention between kernel and user space Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-09 21:10 +0200
Re: [patch 1/3] tty: resolve tty contention between kernel and user space Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-10 10:40 +0200
Re: [patch 1/3] tty: resolve tty contention between kernel and user space Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-07-10 17:30 +0200
Re: [patch 1/3] tty: resolve tty contention between kernel and user space Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-10 18:20 +0200
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-07-09 14:00 +0200
[patch 4/3] tty: make tty_kopen return ENODEV in case of no TTY Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-09 14:40 +0200
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-07-10 14:00 +0200
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-10 14:40 +0200
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-10 18:30 +0200
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-07-12 20:30 +0200
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-13 13:40 +0200
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-07-17 14:40 +0200
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-17 15:30 +0200
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-07-18 00:10 +0200
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-18 13:40 +0200
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export Dan Carpenter <dan.carpenter@oracle.com> - 2017-07-18 14:30 +0200
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-18 21:30 +0200
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-07-18 16:00 +0200
[patch v3 3/3] tty: undo export of tty_open_by_driver Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-20 09:30 +0200
[patch v3 1/3] tty: resolve tty contention between kernel and user space Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-20 09:30 +0200
[patch v3 0/3] tty contention resulting from tty_open_by_driver export Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-20 09:30 +0200
[patch v3 2/3] staging: speakup: use tty_kopen and tty_kclose Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-20 09:40 +0200
[patch v2 1/3] tty: resolve tty contention between kernel and user space Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-17 23:10 +0200
[patch v2 3/3] tty: undo export of tty_open_by_driver Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-17 23:10 +0200
[patch v2 0/3] tty contention resulting from tty_open_by_driver export Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-17 23:10 +0200
[patch v2 2/3] staging: speakup: use tty_kopen instead of tty_open_by_driver Okash Khawaja <okash.khawaja@gmail.com> - 2017-07-17 23:10 +0200
csiph-web