public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Alan Cox <alan@linux.intel.com>
To: Havard Skinnemoen <hskinnemoen@google.com>
Cc: Jiri Slaby <jslaby@suse.cz>, Dave Jones <davej@redhat.com>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@suse.de>,
	Jiri Slaby <jirislaby@gmail.com>
Subject: Re: [RFC] cdc-acm: Fix potential deadlock (lockdep warning)
Date: Wed, 23 Nov 2011 19:44:23 +0000	[thread overview]
Message-ID: <20111123194423.158002cf@bob.linux.org.uk> (raw)
In-Reply-To: <CAFQmdRYh3vSz4s-ks5dbz7bXJx-EZNHEGzsr5bcpEYquXmB6Fg@mail.gmail.com>

> Ok, by USB side kref do you mean the tty_port_get/put calls I
> introduced in this patch, or a kref associated with the USB device
> itself?

The USB serial driver krefs its USB related objects too

> Ok, so the basic idea is:
>   * USB side: tty_port_get() in probe, tty_port_put() in disconnect
>   * TTY side: tty_port_get() in install, tty_port_put() in remove
> 
> And I need to check if the device is properly marked as disconnected,
> switch to vhangup, and possibly reorder things a bit as well. Right?

You shouldn't need to do the port_get in the probe or the put in the
disconnect path. The install/remove one is sufficient for all the
tty_port data because the only way a new open can occur is if the
install succeeds. So providing install and remove are suitably locked
(see the usb_serial.c code) the other cases shouldn't matter.

Any data owned by the USB part needs to be handled by the USB driver.
In the case of usb-serial it keeps its own krefs for that because the
lifetime of the port/USB serial data is not the same as the lifetime of
the tty itself.

The lifetime model is intended to be

object discovered
	Port created

	one or more iterations of
	{
		tty_install (refcounts the port and USB data)
		tty opened
		tty created

		[activity]

		tty closed final time
		tty destroyed (drops a reference on the port and USB
		data)
	}

	Port destroyed

and if the object is destroyed while the tty is opened tty_vhangup
ensures that all the linkages are broken and the port itself can be
destroyed after the last user.

That is tty_struct has a lifetime of open - close (plus a bit for any
active receive etc) while the tty_port is usually embedded in the USB
related material and has a life time of object creation to the point
that the object has gone away AND there are no ttys in existence using
it, hung up or otherwise. The tty "cleanup" hook may be useful for
that. It is called asynchronously as part of the the destructor for the
tty object.

In some ways its easier to read usb-serial.c than describe it




  reply	other threads:[~2011-11-23 19:33 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-23  3:38 tty related lockdep trace during bootup on 3.2-rc2 Dave Jones
2011-11-23  7:28 ` Havard Skinnemoen
2011-11-23  7:39   ` Cong Wang
2011-11-23 10:12   ` Jiri Slaby
2011-11-23 10:14     ` Jiri Slaby
2011-11-23 17:58     ` Havard Skinnemoen
2011-11-23 18:53       ` [RFC] cdc-acm: Fix potential deadlock (lockdep warning) Havard Skinnemoen
2011-11-23 19:22         ` Alan Cox
2011-11-23 19:22           ` Havard Skinnemoen
2011-11-23 19:44             ` Alan Cox [this message]
2011-11-23 21:03               ` Havard Skinnemoen
2011-11-23 21:59                 ` Alan Cox
2011-11-23 19:34           ` Oliver Neukum
2011-11-23 22:00             ` Alan Cox
2011-11-23 19:55         ` Jiri Slaby
2011-11-23 21:08           ` Havard Skinnemoen
2011-11-23 21:11             ` Jiri Slaby
2011-11-23 21:19               ` Jiri Slaby
2011-11-27 21:37         ` [RFC v2] " Havard Skinnemoen
2011-11-28 18:15           ` Havard Skinnemoen
2011-11-23  7:29 ` tty related lockdep trace during bootup on 3.2-rc2 Cong Wang
2011-11-23 17:29   ` Havard Skinnemoen

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=20111123194423.158002cf@bob.linux.org.uk \
    --to=alan@linux.intel.com \
    --cc=davej@redhat.com \
    --cc=gregkh@suse.de \
    --cc=hskinnemoen@google.com \
    --cc=jirislaby@gmail.com \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@vger.kernel.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