From: Peter Hurley <peter@hurleysoftware.com>
To: Gianluca Anzolin <gianluca@sottospazio.it>
Cc: gustavo@padovan.org, linux-bluetooth@vger.kernel.org,
marcel@holtmann.org
Subject: Re: [PATCH 4/8] Move tty initialization and cleanup out of open/close
Date: Tue, 16 Jul 2013 15:07:08 -0400 [thread overview]
Message-ID: <51E599DC.3050208@hurleysoftware.com> (raw)
In-Reply-To: <1373661649-1385-4-git-send-email-gianluca@sottospazio.it>
On 07/12/2013 04:40 PM, Gianluca Anzolin wrote:
> Move the tty initialization from rfcomm_tty_open to rfcomm_tty_install and do
> the same for the cleanup moving the code from rfcomm_tty_close to
> rfcomm_tty_cleanup.
>
> Signed-off-by: Gianluca Anzolin <gianluca@sottospazio.it>
This should be patch 3/8.
Thus changes to rfcomm_tty_open()/rfcomm_tty_close() can rely on these
operations occurring. (see my comments in current patch 3/8)
> ---
> net/bluetooth/rfcomm/tty.c | 82 +++++++++++++++++++++++++++-------------------
> 1 file changed, 49 insertions(+), 33 deletions(-)
>
> diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
> index 9b0b064..c1dd55d 100644
> --- a/net/bluetooth/rfcomm/tty.c
> +++ b/net/bluetooth/rfcomm/tty.c
> @@ -717,10 +717,55 @@ static void rfcomm_dev_modem_status(struct rfcomm_dlc *dlc, u8 v24_sig)
> }
>
> /* ---- TTY functions ---- */
> +/*
> + * here we acquire the tty_port reference since it's here the tty is first used
> + * by setting the termios. We also populate the driver_data field and install
> + * the tty_port.
> + */
> +static int rfcomm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
> +{
> + struct rfcomm_dev *dev;
> + int err;
> +
> + dev = rfcomm_dev_get(tty->index);
> + if (!dev)
> + return -ENODEV;
> +
> + /* Attach TTY */
> + rfcomm_dlc_lock(dev->dlc);
> + tty->driver_data = dev;
> + rfcomm_dlc_unlock(dev->dlc);
> + set_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
> +
> + err = tty_port_install(&dev->port, driver, tty);
> + if (err < 0) {
> + clear_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
> + tty_port_put(&dev->port);
> + }
> +
> + return err;
> +}
> +
> +/*
> + * here we do the reverse, by clearing the tty fields and releasing
> + * the reference to tty_port.
> + */
> +static void rfcomm_tty_cleanup(struct tty_struct *tty)
> +{
> + struct rfcomm_dev *dev = tty->driver_data;
> +
> + /* Remove driver data */
> + clear_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
> + rfcomm_dlc_lock(dev->dlc);
> + tty->driver_data = NULL;
> + rfcomm_dlc_unlock(dev->dlc);
> +
> + tty_port_put(&dev->port);
> +}
> +
> static int rfcomm_tty_open(struct tty_struct *tty, struct file *filp)
> {
> - struct rfcomm_dev *dev;
> - struct rfcomm_dlc *dlc;
> + struct rfcomm_dev *dev = tty->driver_data;
> unsigned long flags;
> int id;
>
> @@ -728,14 +773,6 @@ static int rfcomm_tty_open(struct tty_struct *tty, struct file *filp)
>
> BT_DBG("tty %p id %d", tty, id);
>
> - /* We don't leak this refcount. For reasons which are not entirely
> - clear, the TTY layer will call our ->close() method even if the
> - open fails. We decrease the refcount there, and decreasing it
> - here too would cause breakage. */
> - dev = rfcomm_dev_get(id);
> - if (!dev)
> - return -ENODEV;
> -
> BT_DBG("dev %p dst %pMR channel %d opened %d", dev, &dev->dst,
> dev->channel, dev->port.count);
>
> @@ -746,16 +783,6 @@ static int rfcomm_tty_open(struct tty_struct *tty, struct file *filp)
> }
> spin_unlock_irqrestore(&dev->port.lock, flags);
>
> - dlc = dev->dlc;
> -
> - /* Attach TTY and open DLC */
> -
> - rfcomm_dlc_lock(dlc);
> - tty->driver_data = dev;
> - tty_port_tty_set(&dev->port, tty);
> - rfcomm_dlc_unlock(dlc);
> - set_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
> -
> return 0;
> }
>
> @@ -764,9 +791,6 @@ static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
> struct rfcomm_dev *dev = (struct rfcomm_dev *) tty->driver_data;
> unsigned long flags;
>
> - if (!dev)
> - return;
> -
> BT_DBG("tty %p dev %p dlc %p opened %d", tty, dev, dev->dlc,
> dev->port.count);
>
> @@ -774,14 +798,6 @@ static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
> if (!--dev->port.count) {
> spin_unlock_irqrestore(&dev->port.lock, flags);
>
> - /* detach the TTY */
> - clear_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
> -
> - rfcomm_dlc_lock(dev->dlc);
> - tty->driver_data = NULL;
> - tty_port_tty_set(&dev->port, NULL);
> - rfcomm_dlc_unlock(dev->dlc);
> -
> if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags)) {
> spin_lock(&rfcomm_dev_lock);
> list_del_init(&dev->list);
> @@ -791,8 +807,6 @@ static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
> }
> } else
> spin_unlock_irqrestore(&dev->port.lock, flags);
> -
> - tty_port_put(&dev->port);
> }
>
> static int rfcomm_tty_write(struct tty_struct *tty, const unsigned char *buf, int count)
> @@ -1165,6 +1179,8 @@ static const struct tty_operations rfcomm_ops = {
> .wait_until_sent = rfcomm_tty_wait_until_sent,
> .tiocmget = rfcomm_tty_tiocmget,
> .tiocmset = rfcomm_tty_tiocmset,
> + .install = rfcomm_tty_install,
> + .cleanup = rfcomm_tty_cleanup,
> };
>
> int __init rfcomm_init_ttys(void)
>
next prev parent reply other threads:[~2013-07-16 19:07 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-12 20:40 [PATCH 1/8] Take proper tty references in net/bluetooth/rfcomm/tty.c Gianluca Anzolin
2013-07-12 20:40 ` [PATCH 2/8] Move functions before the definition of rfcomm_port_ops Gianluca Anzolin
2013-07-16 15:14 ` Peter Hurley
2013-07-12 20:40 ` [PATCH 3/8] Move device initialization and shutdown to tty_port_operations Gianluca Anzolin
2013-07-16 20:48 ` Peter Hurley
2013-07-20 7:10 ` Gianluca Anzolin
2013-07-20 14:11 ` Peter Hurley
2013-07-21 8:08 ` Gianluca Anzolin
2013-07-21 17:04 ` Peter Hurley
2013-07-21 17:31 ` Gianluca Anzolin
2013-07-12 20:40 ` [PATCH 4/8] Move tty initialization and cleanup out of open/close Gianluca Anzolin
2013-07-16 19:07 ` Peter Hurley [this message]
2013-07-12 20:40 ` [PATCH 5/8] Use the tty_port_* functions in tty_open/tty_close/tty_hangup Gianluca Anzolin
2013-07-16 20:51 ` Peter Hurley
2013-07-17 8:03 ` Gianluca Anzolin
2013-07-12 20:40 ` [PATCH 6/8] Fix the reference counting of tty_port Gianluca Anzolin
2013-07-17 14:02 ` Peter Hurley
2013-07-17 17:05 ` Gianluca Anzolin
2013-07-17 18:10 ` Peter Hurley
2013-07-18 12:45 ` Peter Hurley
2013-07-18 14:13 ` Gianluca Anzolin
2013-07-18 15:19 ` Peter Hurley
2013-07-12 20:40 ` [PATCH 7/8] Avoid a circular dependency between dev and dev->dlc Gianluca Anzolin
2013-07-12 20:40 ` [PATCH 8/8] Add module_put in rfcomm_dev_add error path Gianluca Anzolin
2013-07-17 15:20 ` Peter Hurley
2013-07-16 14:53 ` [PATCH 1/8] Take proper tty references in net/bluetooth/rfcomm/tty.c Peter Hurley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51E599DC.3050208@hurleysoftware.com \
--to=peter@hurleysoftware.com \
--cc=gianluca@sottospazio.it \
--cc=gustavo@padovan.org \
--cc=linux-bluetooth@vger.kernel.org \
--cc=marcel@holtmann.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).