On Sun, Jul 9, 2017 at 2:41 PM, Okash Khawaja <okash.khaw...@gmail.com> wrote:
> +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);
Hmm... perhaps
tty = ERR_CAST(driver);
goto out_unlock;
See below for further details.
> + }
> +
> + /* 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);
> + set_bit(TTY_KOPENED, &tty->flags);
> + }
> +out:
out_unlock: ?
> + mutex_unlock(&tty_mutex);
> + tty_driver_kref_put(driver);
I'm not sure I understand locking model here:
Above
1. take mutex
2. take reference
Here:
1. give mutex back
2. give reference back
I think we usually see symmetrical calls, i.e.
1. give reference back
2. give mutex back
So, what did I miss?
> + return tty;
> +}
--
With Best Regards,
Andy Shevchenko
