[PATCH 00/24] rfcomm fixes

[PATCH 00/24] rfcomm fixes

[Peter Hurley]

Marcel,

This patch series addresses a number of previously unknown issues
with the RFCOMM tty device implementation, in addition to
addressing the locking regression recently reported [1].

As Gianluca suggested and I agree, this series first reverts
3 of the 4 patches of 3.14-rc1 for bluetooth/rfcomm/tty.c.

The reasoning is detailed in the changelog for
  Revert "Bluetooth: Always wait for a connection on RFCOMM open()"
but the short answer is that it re-implements a long-standing
bug by blocking on a non-blocking open.

This patch series corrects the reported regressions from 3.13
(to the extent that correction is required). Specifically,
the ModemManager regression reported by Gianluca Anzolin [2]
and the rfcomm bind with wvdial reported by Andrey Vihrov [3].

tty: Fix ref counting for port krefs
Bluetooth: Fix racy acquire of rfcomm_dev reference
Bluetooth: Exclude released devices from RFCOMMGETDEVLIST ioctl
Bluetooth: Release rfcomm_dev only once
Bluetooth: Fix unreleased rfcomm_dev reference
   These first 5 patches after the reverts
   fix 4 different rfcomm_dev ref count mishandling bugs.

Bluetooth: Fix RFCOMM tty teardown race and
Bluetooth: Serialize RFCOMMCREATEDEV and RFCOMMRELEASEDEV ioctls
   Fix races which occur due to the design of the rfcomm ioctls
   (note that buses don't have these kinds of races).

Bluetooth: Verify dlci not in use before rfcomm_dev create
Bluetooth: Simplify RFCOMM session state eval
Bluetooth: Refactor deferred setup test in rfcomm_dlc_close()
Bluetooth: Refactor dlc disconnect logic in rfcomm_dlc_close()
Bluetooth: Directly close dlc for not yet started RFCOMM session
   These 5 patches fix issues with reusing the dlci after
   closing the tty (found by unit test).

Bluetooth: Fix unsafe RFCOMM device parenting
Bluetooth: Fix RFCOMM parent device for reused dlc
   These 2 patches fix the ModemManager regression.

Bluetooth: Refactor rfcomm_dev_add()
Bluetooth: Cleanup RFCOMM device registration error handling
   These 2 patches fix an unreleased module reference while
   error handling.

Bluetooth: Rename __rfcomm_dev_get() to __rfcomm_dev_lookup()
   This is a trivial naming patch with no functional impact.

Bluetooth: Force -EIO from tty read/write if .activate() fails
   The tty core provides an existing mechanism for failing
   reads/writes if device activation fails (like an error
   allocating the dlc).

Bluetooth: Don't fail RFCOMM tty writes
   This patch implements buffered writes even if the device
   is not connected.

While unit testing this, I discovered a serious defect in
the way available space is computed that under-utilizes
rfcomm i/o and may even halt further tx on that link, which
is fixed by:
  Bluetooth: Refactor write_room() calculation
  Bluetooth: Fix write_room() calculation


Note that this series does not fix the naively inefficient
method of packetizing tty output; packetizing should be
done on the krfcommd thread to take advantage of aggregating
multiple tty writes into 1 or more packets. Look at any
line-by-line console output to realize how under-utilized
the rfcomm tty packeting is.

[1] http://www.spinics.net/lists/linux-wireless/msg117818.html
[2] http://www.spinics.net/lists/linux-bluetooth/msg42075.html
[3] http://www.spinics.net/lists/linux-bluetooth/msg42057.html


Regards,


Peter Hurley (24):
  Revert "Bluetooth: Remove rfcomm_carrier_raised()"
  Revert "Bluetooth: Always wait for a connection on RFCOMM open()"
  Revert "Bluetooth: Move rfcomm_get_device() before
    rfcomm_dev_activate()"
  tty: Fix ref counting for port krefs
  Bluetooth: Fix racy acquire of rfcomm_dev reference
  Bluetooth: Exclude released devices from RFCOMMGETDEVLIST ioctl
  Bluetooth: Release rfcomm_dev only once
  Bluetooth: Fix unreleased rfcomm_dev reference
  Bluetooth: Fix RFCOMM tty teardown race
  Bluetooth: Verify dlci not in use before rfcomm_dev create
  Bluetooth: Simplify RFCOMM session state eval
  Bluetooth: Refactor deferred setup test in rfcomm_dlc_close()
  Bluetooth: Refactor dlc disconnect logic in rfcomm_dlc_close()
  Bluetooth: Directly close dlc for not yet started RFCOMM session
  Bluetooth: Fix unsafe RFCOMM device parenting
  Bluetooth: Fix RFCOMM parent device for reused dlc
  Bluetooth: Rename __rfcomm_dev_get() to __rfcomm_dev_lookup()
  Bluetooth: Serialize RFCOMMCREATEDEV and RFCOMMRELEASEDEV ioctls
  Bluetooth: Refactor rfcomm_dev_add()
  Bluetooth: Cleanup RFCOMM device registration error handling
  Bluetooth: Force -EIO from tty read/write if .activate() fails
  Bluetooth: Don't fail RFCOMM tty writes
  Bluetooth: Refactor write_room() calculation
  Bluetooth: Fix write_room() calculation

 include/linux/tty.h            |   6 +-
 include/net/bluetooth/rfcomm.h |   9 +-
 net/bluetooth/rfcomm/core.c    |  88 ++++++++++----
 net/bluetooth/rfcomm/tty.c     | 262 ++++++++++++++++++++++-------------------
 4 files changed, 223 insertions(+), 142 deletions(-)

-- 
1.8.1.2

[PATCH 01/24] Revert "Bluetooth: Remove rfcomm_carrier_raised()"

[Peter Hurley]

This reverts commit f86772af6a0f643d3e13eb3f4f9213ae0c333ee4.

This is the first of a 3-patch revert, together with
Revert "Bluetooth: Always wait for a connection on RFCOMM open()" and
Revert "Bluetooth: Move rfcomm_get_device() before rfcomm_dev_activate()".

Commit 4a2fb3ecc7467c775b154813861f25a0ddc11aa0,
"Bluetooth: Always wait for a connection on RFCOMM open()" open-codes
blocking on tty open(), rather than using the default behavior
implemented by the tty port.

The reasons for reverting that patch are detailed in that changelog;
this patch restores required functionality for that revert.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index f9c0980a..aeabade 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -160,6 +160,14 @@ static int rfcomm_dev_activate(struct tty_port *port, struct tty_struct *tty)
 	return err;
 }
 
+/* we block the open until the dlc->state becomes BT_CONNECTED */
+static int rfcomm_dev_carrier_raised(struct tty_port *port)
+{
+	struct rfcomm_dev *dev = container_of(port, struct rfcomm_dev, port);
+
+	return (dev->dlc->state == BT_CONNECTED);
+}
+
 /* device-specific cleanup: close the dlc */
 static void rfcomm_dev_shutdown(struct tty_port *port)
 {
-- 
1.8.1.2

[PATCH 02/24] Revert "Bluetooth: Always wait for a connection on RFCOMM open()"

[Peter Hurley]

This reverts commit 4a2fb3ecc7467c775b154813861f25a0ddc11aa0.

This is the second of a 3-patch revert, together with
Revert "Bluetooth: Remove rfcomm_carrier_raised()" and
Revert "Bluetooth: Move rfcomm_get_device() before rfcomm_dev_activate()".

Before commit cad348a17e170451ea8688b532a6ca3e98c63b60,
  Bluetooth: Implement .activate, .shutdown and .carrier_raised methods,
tty_port_block_til_ready() was open-coded in rfcomm_tty_install() as
part of the RFCOMM tty open().

Unfortunately, it did not implement non-blocking open nor CLOCAL open,
but rather always blocked for carrier. This is not the expected or
typical behavior for ttys, and prevents several common terminal
programming idioms from working (eg., opening in non-blocking
mode to initialize desired termios settings then re-opening for
connection).

Commit cad348a17e170451ea8688b532a6ca3e98c63b60,
  Bluetooth: Implement .activate, .shutdown and .carrier_raised methods,
added the necessary tty_port methods to use the default tty_port_open().
However, this triggered two important user-space regressions.

The first regression involves the complicated mechanism for reparenting
the rfcomm tty device to the ACL link device which represents an
open link to a specific bluetooth host. This regression causes ModemManager
to conclude the rfcomm tty device does not front a modem so it makes
no attempt to initialize an attached modem. This regression is
caused by the lack of a device_move() if the dlc is already open (and
not specifically related to the open-coded block_til_ready()).

A more appropriate solution is submitted in
"Bluetooth: Fix unsafe RFCOMM device parenting" and
"Bluetooth: Fix RFCOMM parent device for reused dlc"

The second regression involves "rfcomm bind" and wvdial (a ppp dialer).
rfcomm bind creates a device node for a /dev/rfcomm<n>. wvdial opens
that device in non-blocking mode (because it expects the connection
to have already been established). In addition, subsequent writes
to the rfcomm tty device fail (because the link is not yet connected;
rfcomm connection begins with the actual tty open()).

However, restoring the original behavior (in the patch which
this reverts) was undesirable.

Firstly, the original reporter notes that a trivial userspace
"workaround" already exists: rfcomm connect, which creates the
device node and establishes the expected connection.

Secondly, the failed writes occur because the rfcomm tty driver
does not buffer writes to an unconnected device; this contrasts with
the dozen of other tty drivers (in fact, all of them) that do just
that. The submitted patch "Bluetooth: Don't fail RFCOMM tty writes"
corrects this.

Thirdly, it was a long-standing bug to block on non-blocking open,
which is re-fixed by revert.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 46 ++++++++--------------------------------------
 1 file changed, 8 insertions(+), 38 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index aeabade..32ef9f9 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -58,7 +58,6 @@ struct rfcomm_dev {
 	uint			modem_status;
 
 	struct rfcomm_dlc	*dlc;
-	wait_queue_head_t       conn_wait;
 
 	struct device		*tty_dev;
 
@@ -124,40 +123,8 @@ static struct device *rfcomm_get_device(struct rfcomm_dev *dev)
 static int rfcomm_dev_activate(struct tty_port *port, struct tty_struct *tty)
 {
 	struct rfcomm_dev *dev = container_of(port, struct rfcomm_dev, port);
-	DEFINE_WAIT(wait);
-	int err;
-
-	err = rfcomm_dlc_open(dev->dlc, &dev->src, &dev->dst, dev->channel);
-	if (err)
-		return err;
-
-	while (1) {
-		prepare_to_wait(&dev->conn_wait, &wait, TASK_INTERRUPTIBLE);
 
-		if (dev->dlc->state == BT_CLOSED) {
-			err = -dev->err;
-			break;
-		}
-
-		if (dev->dlc->state == BT_CONNECTED)
-			break;
-
-		if (signal_pending(current)) {
-			err = -ERESTARTSYS;
-			break;
-		}
-
-		tty_unlock(tty);
-		schedule();
-		tty_lock(tty);
-	}
-	finish_wait(&dev->conn_wait, &wait);
-
-	if (!err)
-		device_move(dev->tty_dev, rfcomm_get_device(dev),
-			    DPM_ORDER_DEV_AFTER_PARENT);
-
-	return err;
+	return rfcomm_dlc_open(dev->dlc, &dev->src, &dev->dst, dev->channel);
 }
 
 /* we block the open until the dlc->state becomes BT_CONNECTED */
@@ -184,6 +151,7 @@ static const struct tty_port_operations rfcomm_port_ops = {
 	.destruct = rfcomm_dev_destruct,
 	.activate = rfcomm_dev_activate,
 	.shutdown = rfcomm_dev_shutdown,
+	.carrier_raised = rfcomm_dev_carrier_raised,
 };
 
 static struct rfcomm_dev *__rfcomm_dev_get(int id)
@@ -290,7 +258,6 @@ static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
 
 	tty_port_init(&dev->port);
 	dev->port.ops = &rfcomm_port_ops;
-	init_waitqueue_head(&dev->conn_wait);
 
 	skb_queue_head_init(&dev->pending);
 
@@ -609,9 +576,12 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err)
 	BT_DBG("dlc %p dev %p err %d", dlc, dev, err);
 
 	dev->err = err;
-	wake_up_interruptible(&dev->conn_wait);
+	if (dlc->state == BT_CONNECTED) {
+		device_move(dev->tty_dev, rfcomm_get_device(dev),
+			    DPM_ORDER_DEV_AFTER_PARENT);
 
-	if (dlc->state == BT_CLOSED)
+		wake_up_interruptible(&dev->port.open_wait);
+	} else if (dlc->state == BT_CLOSED)
 		tty_port_tty_hangup(&dev->port, false);
 }
 
@@ -1133,7 +1103,7 @@ int __init rfcomm_init_ttys(void)
 	rfcomm_tty_driver->subtype	= SERIAL_TYPE_NORMAL;
 	rfcomm_tty_driver->flags	= TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;
 	rfcomm_tty_driver->init_termios	= tty_std_termios;
-	rfcomm_tty_driver->init_termios.c_cflag	= B9600 | CS8 | CREAD | HUPCL | CLOCAL;
+	rfcomm_tty_driver->init_termios.c_cflag	= B9600 | CS8 | CREAD | HUPCL;
 	rfcomm_tty_driver->init_termios.c_lflag &= ~ICANON;
 	tty_set_operations(rfcomm_tty_driver, &rfcomm_ops);
 
-- 
1.8.1.2

[PATCH 03/24] Revert "Bluetooth: Move rfcomm_get_device() before rfcomm_dev_activate()"

[Peter Hurley]

This reverts commit e228b63390536f5b737056059a9a04ea016b1abf.

This is the third of a 3-patch revert, together with
Revert "Bluetooth: Remove rfcomm_carrier_raised()" and
Revert "Bluetooth: Always wait for a connection on RFCOMM open()".

Commit 4a2fb3ecc7467c775b154813861f25a0ddc11aa0,
"Bluetooth: Always wait for a connection on RFCOMM open()" open-codes
blocking on tty open(), rather than using the default behavior
implemented by the tty port.

The reasons for reverting that patch are detailed in that changelog;
this patch restores required functionality for that revert.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 32ef9f9..a535ef1 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -103,22 +103,6 @@ static void rfcomm_dev_destruct(struct tty_port *port)
 	module_put(THIS_MODULE);
 }
 
-static struct device *rfcomm_get_device(struct rfcomm_dev *dev)
-{
-	struct hci_dev *hdev;
-	struct hci_conn *conn;
-
-	hdev = hci_get_route(&dev->dst, &dev->src);
-	if (!hdev)
-		return NULL;
-
-	conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &dev->dst);
-
-	hci_dev_put(hdev);
-
-	return conn ? &conn->dev : NULL;
-}
-
 /* device-specific initialization: open the dlc */
 static int rfcomm_dev_activate(struct tty_port *port, struct tty_struct *tty)
 {
@@ -185,6 +169,22 @@ static struct rfcomm_dev *rfcomm_dev_get(int id)
 	return dev;
 }
 
+static struct device *rfcomm_get_device(struct rfcomm_dev *dev)
+{
+	struct hci_dev *hdev;
+	struct hci_conn *conn;
+
+	hdev = hci_get_route(&dev->dst, &dev->src);
+	if (!hdev)
+		return NULL;
+
+	conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &dev->dst);
+
+	hci_dev_put(hdev);
+
+	return conn ? &conn->dev : NULL;
+}
+
 static ssize_t show_address(struct device *tty_dev, struct device_attribute *attr, char *buf)
 {
 	struct rfcomm_dev *dev = dev_get_drvdata(tty_dev);
-- 
1.8.1.2

[PATCH 04/24] tty: Fix ref counting for port krefs

[Peter Hurley]

The tty core supports two models for handling tty_port lifetimes;
the tty_port can use the kref supplied by tty_port (which will
automatically destruct the tty_port when the ref count drops to
zero) or it can destruct the tty_port manually.

For tty drivers that choose to use the port kref to manage the
tty_port lifetime, it is not possible to safely acquire a port
reference conditionally. If the last reference is released after
evaluating the condition but before acquiring the reference, a
bogus reference will be held while the tty_port destruction
commences.

Rather, only acquire a port reference if the ref count is non-zero
and allow the caller to distinguish if a reference has successfully
been acquired.

Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 include/linux/tty.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/tty.h b/include/linux/tty.h
index 90b4fdc..4781d7b 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -518,9 +518,9 @@ extern void tty_port_put(struct tty_port *port);
 
 static inline struct tty_port *tty_port_get(struct tty_port *port)
 {
-	if (port)
-		kref_get(&port->kref);
-	return port;
+	if (port && kref_get_unless_zero(&port->kref))
+		return port;
+	return NULL;
 }
 
 /* If the cts flow control is enabled, return true. */
-- 
1.8.1.2

[PATCH 05/24] Bluetooth: Fix racy acquire of rfcomm_dev reference

[Peter Hurley]

rfcomm_dev_get() can return a rfcomm_dev reference for a
device for which destruction may be commencing. This can happen
on tty destruction, which calls rfcomm_tty_cleanup(), the last
port reference may have been released but RFCOMM_TTY_RELEASED
was not set. The following race is also possible:

CPU 0                            | CPU 1
                                 | rfcomm_release_dev
rfcomm_dev_get                   |   .
  spin_lock                      |   .
    dev  = __rfcomm_dev_get      |   .
    if dev                       |   .
      if test_bit(TTY_RELEASED)  |   .
                                 |   !test_and_set_bit(TTY_RELEASED)
                                 |     tty_port_put   <<<< last reference
      else                       |
        tty_port_get             |

The reference acquire is bogus because destruction will commence
with the release of the last reference.

Ignore the external state change of TTY_RELEASED and instead rely
on the reference acquire itself to determine if the reference is
valid.

Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index a535ef1..7cf193f 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -157,12 +157,8 @@ static struct rfcomm_dev *rfcomm_dev_get(int id)
 
 	dev = __rfcomm_dev_get(id);
 
-	if (dev) {
-		if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
-			dev = NULL;
-		else
-			tty_port_get(&dev->port);
-	}
+	if (dev && !tty_port_get(&dev->port))
+		dev = NULL;
 
 	spin_unlock(&rfcomm_dev_lock);
 
-- 
1.8.1.2

[PATCH 06/24] Bluetooth: Exclude released devices from RFCOMMGETDEVLIST ioctl

[Peter Hurley]

When enumerating RFCOMM devices in the rfcomm_dev_list, holding
the rfcomm_dev_lock only guarantees the existence of the enumerated
rfcomm_dev in memory, and not safe access to its state. Testing
the device state (such as RFCOMM_TTY_RELEASED) does not guarantee
the device will remain in that state for the subsequent access
to the rfcomm_dev's fields, nor guarantee that teardown has not
commenced.

Obtain an rfcomm_dev reference for the duration of rfcomm_dev
access.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 7cf193f..b385d99 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -468,7 +468,7 @@ static int rfcomm_get_dev_list(void __user *arg)
 	spin_lock(&rfcomm_dev_lock);
 
 	list_for_each_entry(dev, &rfcomm_dev_list, list) {
-		if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
+		if (!tty_port_get(&dev->port))
 			continue;
 		(di + n)->id      = dev->id;
 		(di + n)->flags   = dev->flags;
@@ -476,6 +476,7 @@ static int rfcomm_get_dev_list(void __user *arg)
 		(di + n)->channel = dev->channel;
 		bacpy(&(di + n)->src, &dev->src);
 		bacpy(&(di + n)->dst, &dev->dst);
+		tty_port_put(&dev->port);
 		if (++n >= dev_num)
 			break;
 	}
-- 
1.8.1.2

[PATCH 07/24] Bluetooth: Release rfcomm_dev only once

[Peter Hurley]

No logic prevents an rfcomm_dev from being released multiple
times. For example, if the rfcomm_dev ref count is large due
to pending tx, then multiple RFCOMMRELEASEDEV ioctls may
mistakenly release the rfcomm_dev too many times. Note that
concurrent ioctls are not required to create this condition.

Introduce RFCOMM_DEV_RELEASED status bit which guarantees the
rfcomm_dev can only be released once.

NB: Since the flags are exported to userspace, introduce the status
field to track state for which userspace should not be aware.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 include/net/bluetooth/rfcomm.h |  6 +++++-
 net/bluetooth/rfcomm/tty.c     | 11 +++++++++--
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index 486213a..29d9727 100644
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -323,11 +323,15 @@ int  rfcomm_connect_ind(struct rfcomm_session *s, u8 channel,
 #define RFCOMMGETDEVINFO	_IOR('R', 211, int)
 #define RFCOMMSTEALDLC		_IOW('R', 220, int)
 
+/* rfcomm_dev.flags bit definitions */
 #define RFCOMM_REUSE_DLC      0
 #define RFCOMM_RELEASE_ONHUP  1
 #define RFCOMM_HANGUP_NOW     2
 #define RFCOMM_TTY_ATTACHED   3
-#define RFCOMM_TTY_RELEASED   4
+#define RFCOMM_DEFUNCT_BIT4   4	  /* don't reuse this bit - userspace visible */
+
+/* rfcomm_dev.status bit definitions */
+#define RFCOMM_DEV_RELEASED   0
 
 struct rfcomm_dev_req {
 	s16      dev_id;
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index b385d99..d9d4bc8 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -51,6 +51,8 @@ struct rfcomm_dev {
 	unsigned long		flags;
 	int			err;
 
+	unsigned long		status;		/* don't export to userspace */
+
 	bdaddr_t		src;
 	bdaddr_t		dst;
 	u8			channel;
@@ -423,6 +425,12 @@ static int rfcomm_release_dev(void __user *arg)
 		return -EPERM;
 	}
 
+	/* only release once */
+	if (test_and_set_bit(RFCOMM_DEV_RELEASED, &dev->status)) {
+		tty_port_put(&dev->port);
+		return -EALREADY;
+	}
+
 	if (req.flags & (1 << RFCOMM_HANGUP_NOW))
 		rfcomm_dlc_close(dev->dlc, 0);
 
@@ -433,8 +441,7 @@ static int rfcomm_release_dev(void __user *arg)
 		tty_kref_put(tty);
 	}
 
-	if (!test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags) &&
-	    !test_and_set_bit(RFCOMM_TTY_RELEASED, &dev->flags))
+	if (!test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags))
 		tty_port_put(&dev->port);
 
 	tty_port_put(&dev->port);
-- 
1.8.1.2

[PATCH 08/24] Bluetooth: Fix unreleased rfcomm_dev reference

[Peter Hurley]

When RFCOMM_RELEASE_ONHUP is set, the rfcomm tty driver 'takes over'
the initial rfcomm_dev reference created by the RFCOMMCREATEDEV ioctl.
The assumption is that the rfcomm tty driver will release the
rfcomm_dev reference when the tty is freed (in rfcomm_tty_cleanup()).
However, if the tty is never opened, the 'take over' never occurs,
so when RFCOMMRELEASEDEV ioctl is called, the reference is not
released.

Track the state of the reference 'take over' so that the release
is guaranteed by either the RFCOMMRELEASEDEV ioctl or the rfcomm tty
driver.

Note that the synchronous hangup in rfcomm_release_dev() ensures
that rfcomm_tty_install() cannot race with the RFCOMMRELEASEDEV ioctl.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 include/net/bluetooth/rfcomm.h | 1 +
 net/bluetooth/rfcomm/tty.c     | 6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index 29d9727..6f3fbc5 100644
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -332,6 +332,7 @@ int  rfcomm_connect_ind(struct rfcomm_session *s, u8 channel,
 
 /* rfcomm_dev.status bit definitions */
 #define RFCOMM_DEV_RELEASED   0
+#define RFCOMM_TTY_OWNED      1
 
 struct rfcomm_dev_req {
 	s16      dev_id;
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index d9d4bc8..bb570d9 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -441,7 +441,7 @@ static int rfcomm_release_dev(void __user *arg)
 		tty_kref_put(tty);
 	}
 
-	if (!test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags))
+	if (!test_bit(RFCOMM_TTY_OWNED, &dev->status))
 		tty_port_put(&dev->port);
 
 	tty_port_put(&dev->port);
@@ -685,8 +685,10 @@ static int rfcomm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
 	 * when the last process closes the tty. The behaviour is expected by
 	 * userspace.
 	 */
-	if (test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags))
+	if (test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags)) {
+		set_bit(RFCOMM_TTY_OWNED, &dev->status);
 		tty_port_put(&dev->port);
+	}
 
 	return 0;
 }
-- 
1.8.1.2

[PATCH 09/24] Bluetooth: Fix RFCOMM tty teardown race

[Peter Hurley]

RFCOMM tty device teardown can race with new tty device registration
for the same device id:

CPU 0                           | CPU 1
rfcomm_dev_add                  | rfcomm_dev_destruct
                                |   spin_lock
                                |   list_del   <== dev_id no longer used
                                |   spin_unlock
  spin_lock                     |     .
  [search rfcomm_dev_list]      |     .
  [dev_id not in use]           |     .
  [initialize new rfcomm_dev]   |     .
  spin_unlock                   |     .
                                |     .
  tty_port_register_device      |   tty_unregister_device

Don't remove rfcomm_dev from the device list until after tty device
unregistration has completed.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index bb570d9..6ea08b0 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -84,10 +84,6 @@ static void rfcomm_dev_destruct(struct tty_port *port)
 
 	BT_DBG("dev %p dlc %p", dev, dlc);
 
-	spin_lock(&rfcomm_dev_lock);
-	list_del(&dev->list);
-	spin_unlock(&rfcomm_dev_lock);
-
 	rfcomm_dlc_lock(dlc);
 	/* Detach DLC if it's owned by this dev */
 	if (dlc->owner == dev)
@@ -98,6 +94,10 @@ static void rfcomm_dev_destruct(struct tty_port *port)
 
 	tty_unregister_device(rfcomm_tty_driver, dev->id);
 
+	spin_lock(&rfcomm_dev_lock);
+	list_del(&dev->list);
+	spin_unlock(&rfcomm_dev_lock);
+
 	kfree(dev);
 
 	/* It's safe to call module_put() here because socket still
-- 
1.8.1.2

[PATCH 10/24] Bluetooth: Verify dlci not in use before rfcomm_dev create

[Peter Hurley]

Only one session/channel combination may be in use at any one
time. However, the failure does not occur until the tty is
opened (in rfcomm_dlc_open()).

Because these settings are actually bound at rfcomm device
creation (via RFCOMMCREATEDEV ioctl), validate and fail before
creating the rfcomm tty device.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 include/net/bluetooth/rfcomm.h |  1 +
 net/bluetooth/rfcomm/core.c    | 26 +++++++++++++++++++++++++-
 net/bluetooth/rfcomm/tty.c     |  8 ++++++++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index 6f3fbc5..79bb913 100644
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -241,6 +241,7 @@ int  rfcomm_dlc_send(struct rfcomm_dlc *d, struct sk_buff *skb);
 int  rfcomm_dlc_set_modem_status(struct rfcomm_dlc *d, u8 v24_sig);
 int  rfcomm_dlc_get_modem_status(struct rfcomm_dlc *d, u8 *v24_sig);
 void rfcomm_dlc_accept(struct rfcomm_dlc *d);
+struct rfcomm_dlc *rfcomm_dlc_exists(bdaddr_t *src, bdaddr_t *dst, u8 channel);
 
 #define rfcomm_dlc_lock(d)     spin_lock(&d->lock)
 #define rfcomm_dlc_unlock(d)   spin_unlock(&d->lock)
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index facd8a7..646b6ff 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -359,6 +359,11 @@ static struct rfcomm_dlc *rfcomm_dlc_get(struct rfcomm_session *s, u8 dlci)
 	return NULL;
 }
 
+static int rfcomm_check_channel(u8 channel)
+{
+	return channel < 1 || channel > 30;
+}
+
 static int __rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst, u8 channel)
 {
 	struct rfcomm_session *s;
@@ -368,7 +373,7 @@ static int __rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst,
 	BT_DBG("dlc %p state %ld %pMR -> %pMR channel %d",
 	       d, d->state, src, dst, channel);
 
-	if (channel < 1 || channel > 30)
+	if (rfcomm_check_channel(channel))
 		return -EINVAL;
 
 	if (d->state != BT_OPEN && d->state != BT_CLOSED)
@@ -513,6 +518,25 @@ no_session:
 	return r;
 }
 
+struct rfcomm_dlc *rfcomm_dlc_exists(bdaddr_t *src, bdaddr_t *dst, u8 channel)
+{
+	struct rfcomm_session *s;
+	struct rfcomm_dlc *dlc = NULL;
+	u8 dlci;
+
+	if (rfcomm_check_channel(channel))
+		return ERR_PTR(-EINVAL);
+
+	rfcomm_lock();
+	s = rfcomm_session_get(src, dst);
+	if (s) {
+		dlci = __dlci(!s->initiator, channel);
+		dlc = rfcomm_dlc_get(s, dlci);
+	}
+	rfcomm_unlock();
+	return dlc;
+}
+
 int rfcomm_dlc_send(struct rfcomm_dlc *d, struct sk_buff *skb)
 {
 	int len = skb->len;
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 6ea08b0..a58d693 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -385,6 +385,14 @@ static int rfcomm_create_dev(struct sock *sk, void __user *arg)
 		dlc = rfcomm_pi(sk)->dlc;
 		rfcomm_dlc_hold(dlc);
 	} else {
+		/* Validate the channel is unused */
+		dlc = rfcomm_dlc_exists(&req.src, &req.dst, req.channel);
+		if (IS_ERR(dlc))
+			return PTR_ERR(dlc);
+		else if (dlc) {
+			rfcomm_dlc_put(dlc);
+			return -EBUSY;
+		}
 		dlc = rfcomm_dlc_alloc(GFP_KERNEL);
 		if (!dlc)
 			return -ENOMEM;
-- 
1.8.1.2

[PATCH 11/24] Bluetooth: Simplify RFCOMM session state eval

[Peter Hurley]

Merge conditional test for BT_LISTEN session state into following
switch statement (which is functionally equivalent).

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 646b6ff..b5a3785 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1967,12 +1967,11 @@ static void rfcomm_process_sessions(void)
 			continue;
 		}
 
-		if (s->state == BT_LISTEN) {
+		switch (s->state) {
+		case BT_LISTEN:
 			rfcomm_accept_connection(s);
 			continue;
-		}
 
-		switch (s->state) {
 		case BT_BOUND:
 			s = rfcomm_check_connection(s);
 			break;
-- 
1.8.1.2

[PATCH 12/24] Bluetooth: Refactor deferred setup test in rfcomm_dlc_close()

[Peter Hurley]

Prepare for directly closing dlc if the RFCOMM session has not
yet been started; refactor the deferred setup test for only those
dlc states to which the test applies. Retains functional
equivalence.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/core.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index b5a3785..797b7e1 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -442,11 +442,18 @@ static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
 	switch (d->state) {
 	case BT_CONNECT:
 	case BT_CONFIG:
+	case BT_OPEN:
+	case BT_CONNECT2:
 		if (test_and_clear_bit(RFCOMM_DEFER_SETUP, &d->flags)) {
 			set_bit(RFCOMM_AUTH_REJECT, &d->flags);
 			rfcomm_schedule();
-			break;
+			return 0;
 		}
+	}
+
+	switch (d->state) {
+	case BT_CONNECT:
+	case BT_CONFIG:
 		/* Fall through */
 
 	case BT_CONNECTED:
@@ -460,15 +467,6 @@ static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
 		}
 		break;
 
-	case BT_OPEN:
-	case BT_CONNECT2:
-		if (test_and_clear_bit(RFCOMM_DEFER_SETUP, &d->flags)) {
-			set_bit(RFCOMM_AUTH_REJECT, &d->flags);
-			rfcomm_schedule();
-			break;
-		}
-		/* Fall through */
-
 	default:
 		rfcomm_dlc_clear_timer(d);
 
-- 
1.8.1.2

[PATCH 13/24] Bluetooth: Refactor dlc disconnect logic in rfcomm_dlc_close()

[Peter Hurley]

Prepare for directly closing dlc if the RFCOMM session has not
yet been started; refactor the dlc disconnect logic into a separate
local function, __rfcomm_dlc_disconn(). Retains functional
equivalence.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/core.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 797b7e1..88f0edc 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -430,6 +430,20 @@ int rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst, u8 chann
 	return r;
 }
 
+static void __rfcomm_dlc_disconn(struct rfcomm_dlc *d)
+{
+	struct rfcomm_session *s = d->session;
+
+	d->state = BT_DISCONN;
+	if (skb_queue_empty(&d->tx_queue)) {
+		rfcomm_send_disc(s, d->dlci);
+		rfcomm_dlc_set_timer(d, RFCOMM_DISC_TIMEOUT);
+	} else {
+		rfcomm_queue_disc(d);
+		rfcomm_dlc_set_timer(d, RFCOMM_DISC_TIMEOUT * 2);
+	}
+}
+
 static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
 {
 	struct rfcomm_session *s = d->session;
@@ -457,14 +471,7 @@ static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
 		/* Fall through */
 
 	case BT_CONNECTED:
-		d->state = BT_DISCONN;
-		if (skb_queue_empty(&d->tx_queue)) {
-			rfcomm_send_disc(s, d->dlci);
-			rfcomm_dlc_set_timer(d, RFCOMM_DISC_TIMEOUT);
-		} else {
-			rfcomm_queue_disc(d);
-			rfcomm_dlc_set_timer(d, RFCOMM_DISC_TIMEOUT * 2);
-		}
+		__rfcomm_dlc_disconn(d);
 		break;
 
 	default:
-- 
1.8.1.2

[PATCH 14/24] Bluetooth: Directly close dlc for not yet started RFCOMM session

[Peter Hurley]

If the RFCOMM session has not yet been started (ie., session is
still in BT_BOUND state) when a dlc is closed, directly close and
unlink the dlc rather than sending a DISC frame that is never
sent.

This allows the dlci to be immediately reused rather than waiting
for a 20 second timeout.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/core.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 88f0edc..8e14df6 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -467,13 +467,19 @@ static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
 
 	switch (d->state) {
 	case BT_CONNECT:
-	case BT_CONFIG:
-		/* Fall through */
-
 	case BT_CONNECTED:
 		__rfcomm_dlc_disconn(d);
 		break;
 
+	case BT_CONFIG:
+		if (s->state != BT_BOUND) {
+			__rfcomm_dlc_disconn(d);
+			break;
+		}
+		/* if closing a dlc in a session that hasn't been started,
+		 * just close and unlink the dlc
+		 */
+
 	default:
 		rfcomm_dlc_clear_timer(d);
 
-- 
1.8.1.2

[PATCH 15/24] Bluetooth: Fix unsafe RFCOMM device parenting

[Peter Hurley]

Accessing the results of hci_conn_hash_lookup_ba() is unsafe without
holding the hci_dev_lock() during the lookup. For example:

CPU 0                             | CPU 1
hci_conn_hash_lookup_ba           | hci_conn_del
  rcu_read_lock                   |   hci_conn_hash_del
  list_for_each_entry_rcu         |     list_del_rcu
    if (.....)                    |       synchronize_rcu
      rcu_read_unlock             |
                                  |   hci_conn_del_sysfs
                                  |   hci_dev_put
                                  |   hci_conn_put
                                  |     put_device (last reference)
                                  |       bt_link_release
                                  |         kfree(conn)
      return p  << just freed     |

Even if a hci_conn reference were taken (via hci_conn_get), would
not guarantee the lifetime of the sysfs device, but only safe
access to the in-memory structure.

Ensure the hci_conn device stays valid while the rfcomm device
is reparented; rename rfcomm_get_device() to rfcomm_reparent_device()
and perform the reparenting within the function while holding the
hci_dev_lock.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index a58d693..2975bc4 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -167,20 +167,29 @@ static struct rfcomm_dev *rfcomm_dev_get(int id)
 	return dev;
 }
 
-static struct device *rfcomm_get_device(struct rfcomm_dev *dev)
+static void rfcomm_reparent_device(struct rfcomm_dev *dev)
 {
 	struct hci_dev *hdev;
 	struct hci_conn *conn;
 
 	hdev = hci_get_route(&dev->dst, &dev->src);
 	if (!hdev)
-		return NULL;
+		return;
 
+	/* The lookup results are unsafe to access without the
+	 * hci device lock (FIXME: why is this not documented?)
+	 */
+	hci_dev_lock(hdev);
 	conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &dev->dst);
 
-	hci_dev_put(hdev);
+	/* Just because the acl link is in the hash table is no
+	 * guarantee the sysfs device has been added ...
+	 */
+	if (conn && device_is_registered(&conn->dev))
+		device_move(dev->tty_dev, &conn->dev, DPM_ORDER_DEV_AFTER_PARENT);
 
-	return conn ? &conn->dev : NULL;
+	hci_dev_unlock(hdev);
+	hci_dev_put(hdev);
 }
 
 static ssize_t show_address(struct device *tty_dev, struct device_attribute *attr, char *buf)
@@ -589,8 +598,7 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err)
 
 	dev->err = err;
 	if (dlc->state == BT_CONNECTED) {
-		device_move(dev->tty_dev, rfcomm_get_device(dev),
-			    DPM_ORDER_DEV_AFTER_PARENT);
+		rfcomm_reparent_device(dev);
 
 		wake_up_interruptible(&dev->port.open_wait);
 	} else if (dlc->state == BT_CLOSED)
-- 
1.8.1.2

[PATCH 16/24] Bluetooth: Fix RFCOMM parent device for reused dlc

[Peter Hurley]

The RFCOMM tty device is parented to the acl link device when
the dlc state_change(BT_CONNECTED) notification is received.
However, if the dlc from the RFCOMM socket is being reused
(RFCOMM_REUSE_DLC is set), then the dlc may already be connected,
and no notification will occur.

Instead, always parent the RFCOMM tty device to the acl link
device at registration time. If the acl link device is not available
(eg, because the dlc is not connected) then the tty will remain
unparented until the BT_CONNECTED notification is received.

Fixes regression with ModemManager when the rfcomm device is
created with the flag RFCOMM_REUSE_DLC.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 2975bc4..fa1226f 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -316,6 +316,7 @@ out:
 		goto free;
 	}
 
+	rfcomm_reparent_device(dev);
 	dev_set_drvdata(dev->tty_dev, dev);
 
 	if (device_create_file(dev->tty_dev, &dev_attr_address) < 0)
-- 
1.8.1.2

[PATCH 17/24] Bluetooth: Rename __rfcomm_dev_get() to __rfcomm_dev_lookup()

[Peter Hurley]

Functions which search lists for matching id's are more
commonly named *_lookup, which is the convention in the
bluetooth core as well.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index fa1226f..4a38b54 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -140,7 +140,7 @@ static const struct tty_port_operations rfcomm_port_ops = {
 	.carrier_raised = rfcomm_dev_carrier_raised,
 };
 
-static struct rfcomm_dev *__rfcomm_dev_get(int id)
+static struct rfcomm_dev *__rfcomm_dev_lookup(int id)
 {
 	struct rfcomm_dev *dev;
 
@@ -157,7 +157,7 @@ static struct rfcomm_dev *rfcomm_dev_get(int id)
 
 	spin_lock(&rfcomm_dev_lock);
 
-	dev = __rfcomm_dev_get(id);
+	dev = __rfcomm_dev_lookup(id);
 
 	if (dev && !tty_port_get(&dev->port))
 		dev = NULL;
-- 
1.8.1.2

[PATCH 18/24] Bluetooth: Serialize RFCOMMCREATEDEV and RFCOMMRELEASEDEV ioctls

[Peter Hurley]

At least two different race conditions exist with multiple concurrent
RFCOMMCREATEDEV and RFCOMMRELEASEDEV ioctls:
* Multiple concurrent RFCOMMCREATEDEVs with RFCOMM_REUSE_DLC can
  mistakenly share the same DLC.
* RFCOMMRELEASEDEV can destruct the rfcomm_dev still being
  constructed by RFCOMMCREATEDEV.

Introduce rfcomm_ioctl_mutex to serialize these add/remove operations.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 4a38b54..ef27695 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -40,6 +40,7 @@
 #define RFCOMM_TTY_MAJOR 216		/* device node major id of the usb/bluetooth.c driver */
 #define RFCOMM_TTY_MINOR 0
 
+static DEFINE_MUTEX(rfcomm_ioctl_mutex);
 static struct tty_driver *rfcomm_tty_driver;
 
 struct rfcomm_dev {
@@ -373,7 +374,7 @@ static struct sk_buff *rfcomm_wmalloc(struct rfcomm_dev *dev, unsigned long size
 
 #define NOCAP_FLAGS ((1 << RFCOMM_REUSE_DLC) | (1 << RFCOMM_RELEASE_ONHUP))
 
-static int rfcomm_create_dev(struct sock *sk, void __user *arg)
+static int __rfcomm_create_dev(struct sock *sk, void __user *arg)
 {
 	struct rfcomm_dev_req req;
 	struct rfcomm_dlc *dlc;
@@ -423,7 +424,7 @@ static int rfcomm_create_dev(struct sock *sk, void __user *arg)
 	return id;
 }
 
-static int rfcomm_release_dev(void __user *arg)
+static int __rfcomm_release_dev(void __user *arg)
 {
 	struct rfcomm_dev_req req;
 	struct rfcomm_dev *dev;
@@ -466,6 +467,28 @@ static int rfcomm_release_dev(void __user *arg)
 	return 0;
 }
 
+static int rfcomm_create_dev(struct sock *sk, void __user *arg)
+{
+	int ret;
+
+	mutex_lock(&rfcomm_ioctl_mutex);
+	ret = __rfcomm_create_dev(sk, arg);
+	mutex_unlock(&rfcomm_ioctl_mutex);
+
+	return ret;
+}
+
+static int rfcomm_release_dev(void __user *arg)
+{
+	int ret;
+
+	mutex_lock(&rfcomm_ioctl_mutex);
+	ret = __rfcomm_release_dev(arg);
+	mutex_unlock(&rfcomm_ioctl_mutex);
+
+	return ret;
+}
+
 static int rfcomm_get_dev_list(void __user *arg)
 {
 	struct rfcomm_dev *dev;
-- 
1.8.1.2

[PATCH 19/24] Bluetooth: Refactor rfcomm_dev_add()

[Peter Hurley]

Move rfcomm_dev allocation and initialization into new function,
__rfcomm_dev_add(), to simplify resource release in error handling.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index ef27695..0537a05 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -208,17 +208,16 @@ static ssize_t show_channel(struct device *tty_dev, struct device_attribute *att
 static DEVICE_ATTR(address, S_IRUGO, show_address, NULL);
 static DEVICE_ATTR(channel, S_IRUGO, show_channel, NULL);
 
-static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
+static struct rfcomm_dev *__rfcomm_dev_add(struct rfcomm_dev_req *req,
+					   struct rfcomm_dlc *dlc)
 {
 	struct rfcomm_dev *dev, *entry;
 	struct list_head *head = &rfcomm_dev_list;
 	int err = 0;
 
-	BT_DBG("id %d channel %d", req->dev_id, req->channel);
-
 	dev = kzalloc(sizeof(struct rfcomm_dev), GFP_KERNEL);
 	if (!dev)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	spin_lock(&rfcomm_dev_lock);
 
@@ -301,22 +300,37 @@ static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
 	   holds reference to this module. */
 	__module_get(THIS_MODULE);
 
+	spin_unlock(&rfcomm_dev_lock);
+	return dev;
+
 out:
 	spin_unlock(&rfcomm_dev_lock);
+	kfree(dev);
+	return ERR_PTR(err);
+}
 
-	if (err < 0)
-		goto free;
+static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
+{
+	struct rfcomm_dev *dev;
+	struct device *tty;
+
+	BT_DBG("id %d channel %d", req->dev_id, req->channel);
 
-	dev->tty_dev = tty_port_register_device(&dev->port, rfcomm_tty_driver,
+	dev = __rfcomm_dev_add(req, dlc);
+	if (IS_ERR(dev))
+		return PTR_ERR(dev);
+
+	tty = tty_port_register_device(&dev->port, rfcomm_tty_driver,
 			dev->id, NULL);
-	if (IS_ERR(dev->tty_dev)) {
-		err = PTR_ERR(dev->tty_dev);
+	if (IS_ERR(tty)) {
 		spin_lock(&rfcomm_dev_lock);
 		list_del(&dev->list);
 		spin_unlock(&rfcomm_dev_lock);
-		goto free;
+		kfree(dev);
+		return PTR_ERR(tty);
 	}
 
+	dev->tty_dev = tty;
 	rfcomm_reparent_device(dev);
 	dev_set_drvdata(dev->tty_dev, dev);
 
@@ -327,10 +341,6 @@ out:
 		BT_ERR("Failed to create channel attribute");
 
 	return dev->id;
-
-free:
-	kfree(dev);
-	return err;
 }
 
 /* ---- Send buffer ---- */
-- 
1.8.1.2

[PATCH 20/24] Bluetooth: Cleanup RFCOMM device registration error handling

[Peter Hurley]

If RFCOMM tty device registration fails, cleanup by releasing
the tty_port reference to trigger rfcomm_dev destruction
(rather than open-coding it).

The dlc reference release is moved into rfcomm_dev_add(),
which ensures cleanup in both error paths -- ie., if
__rfcomm_dev_add() fails or if tty_port_register_device() fails.

Fixes releasing the module reference if device registration fails.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 0537a05..375b60d 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -93,7 +93,8 @@ static void rfcomm_dev_destruct(struct tty_port *port)
 
 	rfcomm_dlc_put(dlc);
 
-	tty_unregister_device(rfcomm_tty_driver, dev->id);
+	if (dev->tty_dev)
+		tty_unregister_device(rfcomm_tty_driver, dev->id);
 
 	spin_lock(&rfcomm_dev_lock);
 	list_del(&dev->list);
@@ -317,16 +318,15 @@ static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
 	BT_DBG("id %d channel %d", req->dev_id, req->channel);
 
 	dev = __rfcomm_dev_add(req, dlc);
-	if (IS_ERR(dev))
+	if (IS_ERR(dev)) {
+		rfcomm_dlc_put(dlc);
 		return PTR_ERR(dev);
+	}
 
 	tty = tty_port_register_device(&dev->port, rfcomm_tty_driver,
 			dev->id, NULL);
 	if (IS_ERR(tty)) {
-		spin_lock(&rfcomm_dev_lock);
-		list_del(&dev->list);
-		spin_unlock(&rfcomm_dev_lock);
-		kfree(dev);
+		tty_port_put(&dev->port);
 		return PTR_ERR(tty);
 	}
 
@@ -420,10 +420,8 @@ static int __rfcomm_create_dev(struct sock *sk, void __user *arg)
 	}
 
 	id = rfcomm_dev_add(&req, dlc);
-	if (id < 0) {
-		rfcomm_dlc_put(dlc);
+	if (id < 0)
 		return id;
-	}
 
 	if (req.flags & (1 << RFCOMM_REUSE_DLC)) {
 		/* DLC is now used by device.
-- 
1.8.1.2

[PATCH 21/24] Bluetooth: Force -EIO from tty read/write if .activate() fails

[Peter Hurley]

If rfcomm_dlc_open() fails, set tty into error state which returns
-EIO from reads and writes.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 375b60d..f6b9f0c 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -111,8 +111,12 @@ static void rfcomm_dev_destruct(struct tty_port *port)
 static int rfcomm_dev_activate(struct tty_port *port, struct tty_struct *tty)
 {
 	struct rfcomm_dev *dev = container_of(port, struct rfcomm_dev, port);
+	int err;
 
-	return rfcomm_dlc_open(dev->dlc, &dev->src, &dev->dst, dev->channel);
+	err = rfcomm_dlc_open(dev->dlc, &dev->src, &dev->dst, dev->channel);
+	if (err)
+		set_bit(TTY_IO_ERROR, &tty->flags);
+	return err;
 }
 
 /* we block the open until the dlc->state becomes BT_CONNECTED */
-- 
1.8.1.2

[PATCH 22/24] Bluetooth: Don't fail RFCOMM tty writes

[Peter Hurley]

The tty driver api design prefers no-fail writes if the driver
write_room() method has previously indicated space is available
to accept writes. Since this is trivially possible for the
RFCOMM tty driver, do so.

Introduce rfcomm_dlc_send_noerror(), which queues but does not
schedule the krfcomm thread if the dlc is not yet connected
(and thus does not error based on the connection state).
The mtu size test is also unnecessary since the caller already
chunks the written data into mtu size.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 include/net/bluetooth/rfcomm.h |  1 +
 net/bluetooth/rfcomm/core.c    | 14 ++++++++++++++
 net/bluetooth/rfcomm/tty.c     | 23 +++++++----------------
 3 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index 79bb913..11aca1e 100644
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -238,6 +238,7 @@ int  rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst,
 								u8 channel);
 int  rfcomm_dlc_close(struct rfcomm_dlc *d, int reason);
 int  rfcomm_dlc_send(struct rfcomm_dlc *d, struct sk_buff *skb);
+void rfcomm_dlc_send_noerror(struct rfcomm_dlc *d, struct sk_buff *skb);
 int  rfcomm_dlc_set_modem_status(struct rfcomm_dlc *d, u8 v24_sig);
 int  rfcomm_dlc_get_modem_status(struct rfcomm_dlc *d, u8 *v24_sig);
 void rfcomm_dlc_accept(struct rfcomm_dlc *d);
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 8e14df6..112749c 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -568,6 +568,20 @@ int rfcomm_dlc_send(struct rfcomm_dlc *d, struct sk_buff *skb)
 	return len;
 }
 
+void rfcomm_dlc_send_noerror(struct rfcomm_dlc *d, struct sk_buff *skb)
+{
+	int len = skb->len;
+
+	BT_DBG("dlc %p mtu %d len %d", d, d->mtu, len);
+
+	rfcomm_make_uih(skb, d->addr);
+	skb_queue_tail(&d->tx_queue, skb);
+
+	if (d->state == BT_CONNECTED &&
+	    !test_bit(RFCOMM_TX_THROTTLED, &d->flags))
+		rfcomm_schedule();
+}
+
 void __rfcomm_dlc_throttle(struct rfcomm_dlc *d)
 {
 	BT_DBG("dlc %p state %ld", d, d->state);
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index f6b9f0c..af775f3 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -374,14 +374,10 @@ static void rfcomm_set_owner_w(struct sk_buff *skb, struct rfcomm_dev *dev)
 
 static struct sk_buff *rfcomm_wmalloc(struct rfcomm_dev *dev, unsigned long size, gfp_t priority)
 {
-	if (atomic_read(&dev->wmem_alloc) < rfcomm_room(dev->dlc)) {
-		struct sk_buff *skb = alloc_skb(size, priority);
-		if (skb) {
-			rfcomm_set_owner_w(skb, dev);
-			return skb;
-		}
-	}
-	return NULL;
+	struct sk_buff *skb = alloc_skb(size, priority);
+	if (skb)
+		rfcomm_set_owner_w(skb, dev);
+	return skb;
 }
 
 /* ---- Device IOCTLs ---- */
@@ -786,7 +782,7 @@ static int rfcomm_tty_write(struct tty_struct *tty, const unsigned char *buf, in
 	struct rfcomm_dev *dev = (struct rfcomm_dev *) tty->driver_data;
 	struct rfcomm_dlc *dlc = dev->dlc;
 	struct sk_buff *skb;
-	int err = 0, sent = 0, size;
+	int sent = 0, size;
 
 	BT_DBG("tty %p count %d", tty, count);
 
@@ -794,7 +790,6 @@ static int rfcomm_tty_write(struct tty_struct *tty, const unsigned char *buf, in
 		size = min_t(uint, count, dlc->mtu);
 
 		skb = rfcomm_wmalloc(dev, size + RFCOMM_SKB_RESERVE, GFP_ATOMIC);
-
 		if (!skb)
 			break;
 
@@ -802,17 +797,13 @@ static int rfcomm_tty_write(struct tty_struct *tty, const unsigned char *buf, in
 
 		memcpy(skb_put(skb, size), buf + sent, size);
 
-		err = rfcomm_dlc_send(dlc, skb);
-		if (err < 0) {
-			kfree_skb(skb);
-			break;
-		}
+		rfcomm_dlc_send_noerror(dlc, skb);
 
 		sent  += size;
 		count -= size;
 	}
 
-	return sent ? sent : err;
+	return sent;
 }
 
 static int rfcomm_tty_write_room(struct tty_struct *tty)
-- 
1.8.1.2

[PATCH 23/24] Bluetooth: Refactor write_room() calculation

[Peter Hurley]

Compute the amount of space available for a single write()
within rfcomm_room(); clamp to 0 for negative values. Note
this patch does not change the result of the computation.

Report the amount of room returned in the debug printk.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index af775f3..3f44195 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -348,11 +348,18 @@ static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
 }
 
 /* ---- Send buffer ---- */
-static inline unsigned int rfcomm_room(struct rfcomm_dlc *dlc)
+static inline unsigned int rfcomm_room(struct rfcomm_dev *dev)
 {
-	/* We can't let it be zero, because we don't get a callback
-	   when tx_credits becomes nonzero, hence we'd never wake up */
-	return dlc->mtu * (dlc->tx_credits?:1);
+	struct rfcomm_dlc *dlc = dev->dlc;
+
+	/* The limit is bogus; the number of packets which can
+	 * currently be sent by the krfcommd thread has no relevance
+	 * to the number of packets which can be queued on the dlc's
+	 * tx queue.
+	 */
+	int limit = dlc->mtu * (dlc->tx_credits?:1);
+
+	return max(0, limit - atomic_read(&dev->wmem_alloc));
 }
 
 static void rfcomm_wfree(struct sk_buff *skb)
@@ -809,16 +816,12 @@ static int rfcomm_tty_write(struct tty_struct *tty, const unsigned char *buf, in
 static int rfcomm_tty_write_room(struct tty_struct *tty)
 {
 	struct rfcomm_dev *dev = (struct rfcomm_dev *) tty->driver_data;
-	int room;
+	int room = 0;
 
-	BT_DBG("tty %p", tty);
-
-	if (!dev || !dev->dlc)
-		return 0;
+	if (dev && dev->dlc)
+		room = rfcomm_room(dev);
 
-	room = rfcomm_room(dev->dlc) - atomic_read(&dev->wmem_alloc);
-	if (room < 0)
-		room = 0;
+	BT_DBG("tty %p room %d", tty, room);
 
 	return room;
 }
-- 
1.8.1.2

[PATCH 24/24] Bluetooth: Fix write_room() calculation

[Peter Hurley]

The skb truesize of a 12-byte payload with a 10-byte head/tail
reserve is 768 bytes. Consequently, even with 40 tx_credits, at
most 6 packets could be queued at any one time:

  40 tx_credits * 127-byte mtu < 768-byte truesize * 7

This error could also cause the tx queue to apparently stall if
credit flow control is disabled (where tx_credits is fixed at 5),
or if the receiver only granted a limited number of tx credits
(eg., less than 7).

Instead, track the outstanding number of queued packets not yet sent
in wmem_alloc and allow for a maximum of 40 queued packets. Report
the space avail for a single write() as the mtu * number of packets
left before reaching the maximum.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 3f44195..403ec09 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -352,20 +352,16 @@ static inline unsigned int rfcomm_room(struct rfcomm_dev *dev)
 {
 	struct rfcomm_dlc *dlc = dev->dlc;
 
-	/* The limit is bogus; the number of packets which can
-	 * currently be sent by the krfcommd thread has no relevance
-	 * to the number of packets which can be queued on the dlc's
-	 * tx queue.
-	 */
-	int limit = dlc->mtu * (dlc->tx_credits?:1);
+	/* Limit the outstanding number of packets not yet sent to 40 */
+	int pending = 40 - atomic_read(&dev->wmem_alloc);
 
-	return max(0, limit - atomic_read(&dev->wmem_alloc));
+	return max(0, pending) * dlc->mtu;
 }
 
 static void rfcomm_wfree(struct sk_buff *skb)
 {
 	struct rfcomm_dev *dev = (void *) skb->sk;
-	atomic_sub(skb->truesize, &dev->wmem_alloc);
+	atomic_dec(&dev->wmem_alloc);
 	if (test_bit(RFCOMM_TTY_ATTACHED, &dev->flags))
 		tty_port_tty_wakeup(&dev->port);
 	tty_port_put(&dev->port);
@@ -374,7 +370,7 @@ static void rfcomm_wfree(struct sk_buff *skb)
 static void rfcomm_set_owner_w(struct sk_buff *skb, struct rfcomm_dev *dev)
 {
 	tty_port_get(&dev->port);
-	atomic_add(skb->truesize, &dev->wmem_alloc);
+	atomic_inc(&dev->wmem_alloc);
 	skb->sk = (void *) dev;
 	skb->destructor = rfcomm_wfree;
 }
-- 
1.8.1.2

Re: [PATCH 00/24] rfcomm fixes

[Marcel Holtmann]

Hi Peter,

> This patch series addresses a number of previously unknown issues
> with the RFCOMM tty device implementation, in addition to
> addressing the locking regression recently reported [1].
> 
> As Gianluca suggested and I agree, this series first reverts
> 3 of the 4 patches of 3.14-rc1 for bluetooth/rfcomm/tty.c.

so for 3.14 we should revert 3 patches. And then the other 21 are intended for 3.15 merge window.

I realize that we still have to deal with some breakage, but we do not want regressions and I clearly not going to take 24 patches for 3.14 at this point in time.

What I can do is take all 24 patches into bluetooth-next and let them sit for 1 week and have people test them. And then we go ahead with reverting 3 patches from 3.14. Does that make sense?

Regards

Marcel

Re: [PATCH 00/24] rfcomm fixes

[Marcel Holtmann]

Hi Peter,

> This patch series addresses a number of previously unknown issues
> with the RFCOMM tty device implementation, in addition to
> addressing the locking regression recently reported [1].
> 
> As Gianluca suggested and I agree, this series first reverts
> 3 of the 4 patches of 3.14-rc1 for bluetooth/rfcomm/tty.c.

so for 3.14 we should revert 3 patches. And then the other 21 are intended for 3.15 merge window.

I realize that we still have to deal with some breakage, but we do not want regressions and I clearly not going to take 24 patches for 3.14 at this point in time.

What I can do is take all 24 patches into bluetooth-next and let them sit for 1 week and have people test them. And then we go ahead with reverting 3 patches from 3.14. Does that make sense?

Regards

Marcel

Re: [PATCH 00/24] rfcomm fixes

[Peter Hurley]

Hi Marcel,

On 02/10/2014 05:09 PM, Marcel Holtmann wrote:
> Hi Peter,
>
>> This patch series addresses a number of previously unknown issues
>> with the RFCOMM tty device implementation, in addition to
>> addressing the locking regression recently reported [1].
>>
>> As Gianluca suggested and I agree, this series first reverts
>> 3 of the 4 patches of 3.14-rc1 for bluetooth/rfcomm/tty.c.
>
> so for 3.14 we should revert 3 patches. And then the other 21 are
 > intended for 3.15 merge window.

Yep, this is probably best. At least 3.13 & 3.14 will behave the
same wrt rfcomm.

> I realize that we still have to deal with some breakage, but we
 > do not want regressions and I clearly not going to take 24 patches
 > for 3.14 at this point in time.

Yeah, I wasn't expecting you to.

> What I can do is take all 24 patches into bluetooth-next and let
 > them sit for 1 week and have people test them. And then we go ahead
 > with reverting 3 patches from 3.14. Does that make sense?

Yep, that's fine with me. Thanks.

Regards,
Peter Hurley

Re: [PATCH 00/24] rfcomm fixes

[Peter Hurley]

Hi Marcel,

On 02/10/2014 05:09 PM, Marcel Holtmann wrote:
> Hi Peter,
>
>> This patch series addresses a number of previously unknown issues
>> with the RFCOMM tty device implementation, in addition to
>> addressing the locking regression recently reported [1].
>>
>> As Gianluca suggested and I agree, this series first reverts
>> 3 of the 4 patches of 3.14-rc1 for bluetooth/rfcomm/tty.c.
>
> so for 3.14 we should revert 3 patches. And then the other 21 are
 > intended for 3.15 merge window.

Yep, this is probably best. At least 3.13 & 3.14 will behave the
same wrt rfcomm.

> I realize that we still have to deal with some breakage, but we
 > do not want regressions and I clearly not going to take 24 patches
 > for 3.14 at this point in time.

Yeah, I wasn't expecting you to.

> What I can do is take all 24 patches into bluetooth-next and let
 > them sit for 1 week and have people test them. And then we go ahead
 > with reverting 3 patches from 3.14. Does that make sense?

Yep, that's fine with me. Thanks.

Regards,
Peter Hurley

Re: [PATCH 00/24] rfcomm fixes

[Sander Eikelenboom]

Monday, February 10, 2014, 11:09:38 PM, you wrote:

> Hi Peter,

>> This patch series addresses a number of previously unknown issues
>> with the RFCOMM tty device implementation, in addition to
>> addressing the locking regression recently reported [1].
>> 
>> As Gianluca suggested and I agree, this series first reverts
>> 3 of the 4 patches of 3.14-rc1 for bluetooth/rfcomm/tty.c.

> so for 3.14 we should revert 3 patches. And then the other 21 are intended for 3.15 merge window.

> I realize that we still have to deal with some breakage, but we do not want regressions and I clearly not going to take 24 patches for 3.14 at this point in time.

> What I can do is take all 24 patches into bluetooth-next and let them sit for 1 week and have people test them. And then we go ahead with reverting 3 patches from 3.14. Does that make sense?

Reverting those 3 patches works for me.

--
Sander

> Regards

> Marcel

Re: [PATCH 00/24] rfcomm fixes

[Sander Eikelenboom]

Monday, February 10, 2014, 11:09:38 PM, you wrote:

> Hi Peter,

>> This patch series addresses a number of previously unknown issues
>> with the RFCOMM tty device implementation, in addition to
>> addressing the locking regression recently reported [1].
>> 
>> As Gianluca suggested and I agree, this series first reverts
>> 3 of the 4 patches of 3.14-rc1 for bluetooth/rfcomm/tty.c.

> so for 3.14 we should revert 3 patches. And then the other 21 are intended for 3.15 merge window.

> I realize that we still have to deal with some breakage, but we do not want regressions and I clearly not going to take 24 patches for 3.14 at this point in time.

> What I can do is take all 24 patches into bluetooth-next and let them sit for 1 week and have people test them. And then we go ahead with reverting 3 patches from 3.14. Does that make sense?

Reverting those 3 patches works for me.

--
Sander

> Regards

> Marcel

Re: [PATCH 00/24] rfcomm fixes

[Marcel Holtmann]

Hi Peter,

>>> This patch series addresses a number of previously unknown issues
>>> with the RFCOMM tty device implementation, in addition to
>>> addressing the locking regression recently reported [1].
>>> 
>>> As Gianluca suggested and I agree, this series first reverts
>>> 3 of the 4 patches of 3.14-rc1 for bluetooth/rfcomm/tty.c.
>> 
>> so for 3.14 we should revert 3 patches. And then the other 21 are
> > intended for 3.15 merge window.
> 
> Yep, this is probably best. At least 3.13 & 3.14 will behave the
> same wrt rfcomm.
> 
>> I realize that we still have to deal with some breakage, but we
> > do not want regressions and I clearly not going to take 24 patches
> > for 3.14 at this point in time.
> 
> Yeah, I wasn't expecting you to.
> 
>> What I can do is take all 24 patches into bluetooth-next and let
> > them sit for 1 week and have people test them. And then we go ahead
> > with reverting 3 patches from 3.14. Does that make sense?
> 
> Yep, that's fine with me. Thanks.

we might also want to add some end-to-end test cases to rfcomm-tester that covers this behavior.

Regards

Marcel

Re: [PATCH 00/24] rfcomm fixes

[Marcel Holtmann]

Hi Peter,

>>> This patch series addresses a number of previously unknown issues
>>> with the RFCOMM tty device implementation, in addition to
>>> addressing the locking regression recently reported [1].
>>> 
>>> As Gianluca suggested and I agree, this series first reverts
>>> 3 of the 4 patches of 3.14-rc1 for bluetooth/rfcomm/tty.c.
>> 
>> so for 3.14 we should revert 3 patches. And then the other 21 are
> > intended for 3.15 merge window.
> 
> Yep, this is probably best. At least 3.13 & 3.14 will behave the
> same wrt rfcomm.
> 
>> I realize that we still have to deal with some breakage, but we
> > do not want regressions and I clearly not going to take 24 patches
> > for 3.14 at this point in time.
> 
> Yeah, I wasn't expecting you to.
> 
>> What I can do is take all 24 patches into bluetooth-next and let
> > them sit for 1 week and have people test them. And then we go ahead
> > with reverting 3 patches from 3.14. Does that make sense?
> 
> Yep, that's fine with me. Thanks.

we might also want to add some end-to-end test cases to rfcomm-tester that covers this behavior.

Regards

Marcel

Re: [PATCH 00/24] rfcomm fixes

[Peter Hurley]

Hi Marcel,

On 02/12/2014 05:58 PM, Marcel Holtmann wrote:
> we might also want to add some end-to-end test cases to rfcomm-tester that covers this behavior.

Are there some docs for the linux-bluetooth test harness?

I have some unit tests that I can port but I should probably understand
the bluetooth test framework first. A lot of my unit tests are designed
to test races with the ioctl interface, device creation and initialization,
teardown, tty open, close, etc.

Would that kind of unit test fit in the test harness design?

Regards,
Peter Hurley

Re: [PATCH 00/24] rfcomm fixes

[Peter Hurley]

Hi Marcel,

On 02/12/2014 05:58 PM, Marcel Holtmann wrote:
> we might also want to add some end-to-end test cases to rfcomm-tester that covers this behavior.

Are there some docs for the linux-bluetooth test harness?

I have some unit tests that I can port but I should probably understand
the bluetooth test framework first. A lot of my unit tests are designed
to test races with the ioctl interface, device creation and initialization,
teardown, tty open, close, etc.

Would that kind of unit test fit in the test harness design?

Regards,
Peter Hurley

Re: [PATCH 04/24] tty: Fix ref counting for port krefs

[Greg Kroah-Hartman]

On Sun, Feb 09, 2014 at 08:59:04PM -0500, Peter Hurley wrote:
> The tty core supports two models for handling tty_port lifetimes;
> the tty_port can use the kref supplied by tty_port (which will
> automatically destruct the tty_port when the ref count drops to
> zero) or it can destruct the tty_port manually.
> 
> For tty drivers that choose to use the port kref to manage the
> tty_port lifetime, it is not possible to safely acquire a port
> reference conditionally. If the last reference is released after
> evaluating the condition but before acquiring the reference, a
> bogus reference will be held while the tty_port destruction
> commences.
> 
> Rather, only acquire a port reference if the ref count is non-zero
> and allow the caller to distinguish if a reference has successfully
> been acquired.
> 
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 00/24] rfcomm fixes

[Alexander Holler]

Am 10.02.2014 02:59, schrieb Peter Hurley:
> Marcel,
>
> This patch series addresses a number of previously unknown issues
> with the RFCOMM tty device implementation, in addition to
> addressing the locking regression recently reported [1].
>
> As Gianluca suggested and I agree, this series first reverts
> 3 of the 4 patches of 3.14-rc1 for bluetooth/rfcomm/tty.c.

Oh, looks like you've spend quiet some time to fix rfcomm.

Thanks a lot for that!

I've just tested this series on top of 3.13.2 where I already have 
Gianluca 4 patches applied. Test machines were one AMD (x86_64) (bluez 
4.x) and one ARM(v5) (bluez 5.x) box and I could not find any problems 
during my short tests (disconnecting/turning of both local and remote 
and reconnecting).

I haven't had any debug options turned on and haven't looked at the 
patches at all, but maybe this is already enough to add a

Tested-By: Alexander Holler <holler@ahsoftware.de>

So feel free to add that if someone thinks it makes sense based on my 
short test description above and it isn't already too late.

Thanks again,

Alexander Holler

Re: [PATCH 00/24] rfcomm fixes

[Alexander Holler]

Am 13.02.2014 01:38, schrieb Peter Hurley:
> Hi Marcel,
>
> On 02/12/2014 05:58 PM, Marcel Holtmann wrote:
>> we might also want to add some end-to-end test cases to rfcomm-tester
>> that covers this behavior.

Sounds great. Such would have found the problem with disappearing remote 
bt (rfcomm) devices since 3.8 likely earlier than I did (which was 
around 3.10 if I remember correctly).

Regards,

Alexander Holler

Re: [PATCH 00/24] rfcomm fixes

[Alexander Holler]

Am 13.02.2014 01:38, schrieb Peter Hurley:
> Hi Marcel,
>
> On 02/12/2014 05:58 PM, Marcel Holtmann wrote:
>> we might also want to add some end-to-end test cases to rfcomm-tester
>> that covers this behavior.

Sounds great. Such would have found the problem with disappearing remote 
bt (rfcomm) devices since 3.8 likely earlier than I did (which was 
around 3.10 if I remember correctly).

Regards,

Alexander Holler

Re: [PATCH 00/24] rfcomm fixes

[Marcel Holtmann]

Hi Peter,

> This patch series addresses a number of previously unknown issues
> with the RFCOMM tty device implementation, in addition to
> addressing the locking regression recently reported [1].
> 
> As Gianluca suggested and I agree, this series first reverts
> 3 of the 4 patches of 3.14-rc1 for bluetooth/rfcomm/tty.c.
> 
> The reasoning is detailed in the changelog for
>  Revert "Bluetooth: Always wait for a connection on RFCOMM open()"
> but the short answer is that it re-implements a long-standing
> bug by blocking on a non-blocking open.
> 
> This patch series corrects the reported regressions from 3.13
> (to the extent that correction is required). Specifically,
> the ModemManager regression reported by Gianluca Anzolin [2]
> and the rfcomm bind with wvdial reported by Andrey Vihrov [3].
> 
> tty: Fix ref counting for port krefs
> Bluetooth: Fix racy acquire of rfcomm_dev reference
> Bluetooth: Exclude released devices from RFCOMMGETDEVLIST ioctl
> Bluetooth: Release rfcomm_dev only once
> Bluetooth: Fix unreleased rfcomm_dev reference
>   These first 5 patches after the reverts
>   fix 4 different rfcomm_dev ref count mishandling bugs.
> 
> Bluetooth: Fix RFCOMM tty teardown race and
> Bluetooth: Serialize RFCOMMCREATEDEV and RFCOMMRELEASEDEV ioctls
>   Fix races which occur due to the design of the rfcomm ioctls
>   (note that buses don't have these kinds of races).
> 
> Bluetooth: Verify dlci not in use before rfcomm_dev create
> Bluetooth: Simplify RFCOMM session state eval
> Bluetooth: Refactor deferred setup test in rfcomm_dlc_close()
> Bluetooth: Refactor dlc disconnect logic in rfcomm_dlc_close()
> Bluetooth: Directly close dlc for not yet started RFCOMM session
>   These 5 patches fix issues with reusing the dlci after
>   closing the tty (found by unit test).
> 
> Bluetooth: Fix unsafe RFCOMM device parenting
> Bluetooth: Fix RFCOMM parent device for reused dlc
>   These 2 patches fix the ModemManager regression.
> 
> Bluetooth: Refactor rfcomm_dev_add()
> Bluetooth: Cleanup RFCOMM device registration error handling
>   These 2 patches fix an unreleased module reference while
>   error handling.
> 
> Bluetooth: Rename __rfcomm_dev_get() to __rfcomm_dev_lookup()
>   This is a trivial naming patch with no functional impact.
> 
> Bluetooth: Force -EIO from tty read/write if .activate() fails
>   The tty core provides an existing mechanism for failing
>   reads/writes if device activation fails (like an error
>   allocating the dlc).
> 
> Bluetooth: Don't fail RFCOMM tty writes
>   This patch implements buffered writes even if the device
>   is not connected.
> 
> While unit testing this, I discovered a serious defect in
> the way available space is computed that under-utilizes
> rfcomm i/o and may even halt further tx on that link, which
> is fixed by:
>  Bluetooth: Refactor write_room() calculation
>  Bluetooth: Fix write_room() calculation
> 
> 
> Note that this series does not fix the naively inefficient
> method of packetizing tty output; packetizing should be
> done on the krfcommd thread to take advantage of aggregating
> multiple tty writes into 1 or more packets. Look at any
> line-by-line console output to realize how under-utilized
> the rfcomm tty packeting is.
> 
> [1] http://www.spinics.net/lists/linux-wireless/msg117818.html
> [2] http://www.spinics.net/lists/linux-bluetooth/msg42075.html
> [3] http://www.spinics.net/lists/linux-bluetooth/msg42057.html
> 
> 
> Regards,
> 
> 
> Peter Hurley (24):
>  Revert "Bluetooth: Remove rfcomm_carrier_raised()"
>  Revert "Bluetooth: Always wait for a connection on RFCOMM open()"
>  Revert "Bluetooth: Move rfcomm_get_device() before
>    rfcomm_dev_activate()”

these 3 patches are in bluetooth-next to give them extra testing. We will get them into 3.14-rc4 after an extra week of them being in linux-next.

>  tty: Fix ref counting for port krefs

I am taking this one with Greg’s ack through bluetooth-next. Unless someone objects.

>  Bluetooth: Fix racy acquire of rfcomm_dev reference
>  Bluetooth: Exclude released devices from RFCOMMGETDEVLIST ioctl
>  Bluetooth: Release rfcomm_dev only once
>  Bluetooth: Fix unreleased rfcomm_dev reference
>  Bluetooth: Fix RFCOMM tty teardown race
>  Bluetooth: Verify dlci not in use before rfcomm_dev create
>  Bluetooth: Simplify RFCOMM session state eval
>  Bluetooth: Refactor deferred setup test in rfcomm_dlc_close()
>  Bluetooth: Refactor dlc disconnect logic in rfcomm_dlc_close()
>  Bluetooth: Directly close dlc for not yet started RFCOMM session
>  Bluetooth: Fix unsafe RFCOMM device parenting
>  Bluetooth: Fix RFCOMM parent device for reused dlc
>  Bluetooth: Rename __rfcomm_dev_get() to __rfcomm_dev_lookup()
>  Bluetooth: Serialize RFCOMMCREATEDEV and RFCOMMRELEASEDEV ioctls
>  Bluetooth: Refactor rfcomm_dev_add()
>  Bluetooth: Cleanup RFCOMM device registration error handling
>  Bluetooth: Force -EIO from tty read/write if .activate() fails
>  Bluetooth: Don't fail RFCOMM tty writes
>  Bluetooth: Refactor write_room() calculation
>  Bluetooth: Fix write_room() calculation
> 
> include/linux/tty.h            |   6 +-
> include/net/bluetooth/rfcomm.h |   9 +-
> net/bluetooth/rfcomm/core.c    |  88 ++++++++++----
> net/bluetooth/rfcomm/tty.c     | 262 ++++++++++++++++++++++-------------------
> 4 files changed, 223 insertions(+), 142 deletions(-)

all patches have been applied to bluetooth-next tree.

Regards

Marcel

Re: [PATCH 00/24] rfcomm fixes

[Marcel Holtmann]

Hi Peter,

> This patch series addresses a number of previously unknown issues
> with the RFCOMM tty device implementation, in addition to
> addressing the locking regression recently reported [1].
> 
> As Gianluca suggested and I agree, this series first reverts
> 3 of the 4 patches of 3.14-rc1 for bluetooth/rfcomm/tty.c.
> 
> The reasoning is detailed in the changelog for
>  Revert "Bluetooth: Always wait for a connection on RFCOMM open()"
> but the short answer is that it re-implements a long-standing
> bug by blocking on a non-blocking open.
> 
> This patch series corrects the reported regressions from 3.13
> (to the extent that correction is required). Specifically,
> the ModemManager regression reported by Gianluca Anzolin [2]
> and the rfcomm bind with wvdial reported by Andrey Vihrov [3].
> 
> tty: Fix ref counting for port krefs
> Bluetooth: Fix racy acquire of rfcomm_dev reference
> Bluetooth: Exclude released devices from RFCOMMGETDEVLIST ioctl
> Bluetooth: Release rfcomm_dev only once
> Bluetooth: Fix unreleased rfcomm_dev reference
>   These first 5 patches after the reverts
>   fix 4 different rfcomm_dev ref count mishandling bugs.
> 
> Bluetooth: Fix RFCOMM tty teardown race and
> Bluetooth: Serialize RFCOMMCREATEDEV and RFCOMMRELEASEDEV ioctls
>   Fix races which occur due to the design of the rfcomm ioctls
>   (note that buses don't have these kinds of races).
> 
> Bluetooth: Verify dlci not in use before rfcomm_dev create
> Bluetooth: Simplify RFCOMM session state eval
> Bluetooth: Refactor deferred setup test in rfcomm_dlc_close()
> Bluetooth: Refactor dlc disconnect logic in rfcomm_dlc_close()
> Bluetooth: Directly close dlc for not yet started RFCOMM session
>   These 5 patches fix issues with reusing the dlci after
>   closing the tty (found by unit test).
> 
> Bluetooth: Fix unsafe RFCOMM device parenting
> Bluetooth: Fix RFCOMM parent device for reused dlc
>   These 2 patches fix the ModemManager regression.
> 
> Bluetooth: Refactor rfcomm_dev_add()
> Bluetooth: Cleanup RFCOMM device registration error handling
>   These 2 patches fix an unreleased module reference while
>   error handling.
> 
> Bluetooth: Rename __rfcomm_dev_get() to __rfcomm_dev_lookup()
>   This is a trivial naming patch with no functional impact.
> 
> Bluetooth: Force -EIO from tty read/write if .activate() fails
>   The tty core provides an existing mechanism for failing
>   reads/writes if device activation fails (like an error
>   allocating the dlc).
> 
> Bluetooth: Don't fail RFCOMM tty writes
>   This patch implements buffered writes even if the device
>   is not connected.
> 
> While unit testing this, I discovered a serious defect in
> the way available space is computed that under-utilizes
> rfcomm i/o and may even halt further tx on that link, which
> is fixed by:
>  Bluetooth: Refactor write_room() calculation
>  Bluetooth: Fix write_room() calculation
> 
> 
> Note that this series does not fix the naively inefficient
> method of packetizing tty output; packetizing should be
> done on the krfcommd thread to take advantage of aggregating
> multiple tty writes into 1 or more packets. Look at any
> line-by-line console output to realize how under-utilized
> the rfcomm tty packeting is.
> 
> [1] http://www.spinics.net/lists/linux-wireless/msg117818.html
> [2] http://www.spinics.net/lists/linux-bluetooth/msg42075.html
> [3] http://www.spinics.net/lists/linux-bluetooth/msg42057.html
> 
> 
> Regards,
> 
> 
> Peter Hurley (24):
>  Revert "Bluetooth: Remove rfcomm_carrier_raised()"
>  Revert "Bluetooth: Always wait for a connection on RFCOMM open()"
>  Revert "Bluetooth: Move rfcomm_get_device() before
>    rfcomm_dev_activate()”

these 3 patches are in bluetooth-next to give them extra testing. We will get them into 3.14-rc4 after an extra week of them being in linux-next.

>  tty: Fix ref counting for port krefs

I am taking this one with Greg’s ack through bluetooth-next. Unless someone objects.

>  Bluetooth: Fix racy acquire of rfcomm_dev reference
>  Bluetooth: Exclude released devices from RFCOMMGETDEVLIST ioctl
>  Bluetooth: Release rfcomm_dev only once
>  Bluetooth: Fix unreleased rfcomm_dev reference
>  Bluetooth: Fix RFCOMM tty teardown race
>  Bluetooth: Verify dlci not in use before rfcomm_dev create
>  Bluetooth: Simplify RFCOMM session state eval
>  Bluetooth: Refactor deferred setup test in rfcomm_dlc_close()
>  Bluetooth: Refactor dlc disconnect logic in rfcomm_dlc_close()
>  Bluetooth: Directly close dlc for not yet started RFCOMM session
>  Bluetooth: Fix unsafe RFCOMM device parenting
>  Bluetooth: Fix RFCOMM parent device for reused dlc
>  Bluetooth: Rename __rfcomm_dev_get() to __rfcomm_dev_lookup()
>  Bluetooth: Serialize RFCOMMCREATEDEV and RFCOMMRELEASEDEV ioctls
>  Bluetooth: Refactor rfcomm_dev_add()
>  Bluetooth: Cleanup RFCOMM device registration error handling
>  Bluetooth: Force -EIO from tty read/write if .activate() fails
>  Bluetooth: Don't fail RFCOMM tty writes
>  Bluetooth: Refactor write_room() calculation
>  Bluetooth: Fix write_room() calculation
> 
> include/linux/tty.h            |   6 +-
> include/net/bluetooth/rfcomm.h |   9 +-
> net/bluetooth/rfcomm/core.c    |  88 ++++++++++----
> net/bluetooth/rfcomm/tty.c     | 262 ++++++++++++++++++++++-------------------
> 4 files changed, 223 insertions(+), 142 deletions(-)

all patches have been applied to bluetooth-next tree.

Regards

Marcel

Re: [PATCH 00/24] rfcomm fixes

[Sander Eikelenboom]

Wednesday, February 12, 2014, 12:06:44 PM, you wrote:

> Monday, February 10, 2014, 11:09:38 PM, you wrote:

>> Hi Peter,

>>> This patch series addresses a number of previously unknown issues
>>> with the RFCOMM tty device implementation, in addition to
>>> addressing the locking regression recently reported [1].
>>> 
>>> As Gianluca suggested and I agree, this series first reverts
>>> 3 of the 4 patches of 3.14-rc1 for bluetooth/rfcomm/tty.c.

>> so for 3.14 we should revert 3 patches. And then the other 21 are intended for 3.15 merge window.

>> I realize that we still have to deal with some breakage, but we do not want regressions and I clearly not going to take 24 patches for 3.14 at this point in time.

>> What I can do is take all 24 patches into bluetooth-next and let them sit for 1 week and have people test them. And then we go ahead with reverting 3 patches from 3.14. Does that make sense?

> Reverting those 3 patches works for me.

> --
> Sander

>> Regards

>> Marcel

Hi Marcel,

Ping... it seems these 3 reverts are still not in 3.14-rc5 to fix the regressions ?

--
Sander

Re: [PATCH 00/24] rfcomm fixes

[Sander Eikelenboom]

Wednesday, February 12, 2014, 12:06:44 PM, you wrote:

> Monday, February 10, 2014, 11:09:38 PM, you wrote:

>> Hi Peter,

>>> This patch series addresses a number of previously unknown issues
>>> with the RFCOMM tty device implementation, in addition to
>>> addressing the locking regression recently reported [1].
>>> 
>>> As Gianluca suggested and I agree, this series first reverts
>>> 3 of the 4 patches of 3.14-rc1 for bluetooth/rfcomm/tty.c.

>> so for 3.14 we should revert 3 patches. And then the other 21 are intended for 3.15 merge window.

>> I realize that we still have to deal with some breakage, but we do not want regressions and I clearly not going to take 24 patches for 3.14 at this point in time.

>> What I can do is take all 24 patches into bluetooth-next and let them sit for 1 week and have people test them. And then we go ahead with reverting 3 patches from 3.14. Does that make sense?

> Reverting those 3 patches works for me.

> --
> Sander

>> Regards

>> Marcel

Hi Marcel,

Ping... it seems these 3 reverts are still not in 3.14-rc5 to fix the regressions ?

--
Sander

[RC6 Bell Chime] Re: [PATCH 00/24] rfcomm fixes

[Sander Eikelenboom]

Hi all,

Since:
- 3.14-RC6 has been cut
- this regression is known and reported since the merge window
- the fix (revert of 3 patches) is known for over a month now
- but it's still not in mainline
- my polite ping request from last week seems to have provoked exactly 0 (zero) response.

IT'S TIME TO CHIME SOME BELLS :-)

Hope that WILL be heard somewhere ...

--
Sander

PS. on the informative side the 3 commits to be reverted are:

f86772af6a0f643d3e13eb3f4f9213ae0c333ee4 Bluetooth: Remove rfcomm_carrier_raised()
4a2fb3ecc7467c775b154813861f25a0ddc11aa0 Bluetooth: Always wait for a connection on RFCOMM open()
e228b63390536f5b737056059a9a04ea016b1abf Bluetooth: Move rfcomm_get_device() before rfcomm_dev_activate()


Monday, March 3, 2014, 8:38:53 PM, you wrote:


> Wednesday, February 12, 2014, 12:06:44 PM, you wrote:

>> Monday, February 10, 2014, 11:09:38 PM, you wrote:

>>> Hi Peter,

>>>> This patch series addresses a number of previously unknown issues
>>>> with the RFCOMM tty device implementation, in addition to
>>>> addressing the locking regression recently reported [1].
>>>> 
>>>> As Gianluca suggested and I agree, this series first reverts
>>>> 3 of the 4 patches of 3.14-rc1 for bluetooth/rfcomm/tty.c.

>>> so for 3.14 we should revert 3 patches. And then the other 21 are intended for 3.15 merge window.

>>> I realize that we still have to deal with some breakage, but we do not want regressions and I clearly not going to take 24 patches for 3.14 at this point in time.

>>> What I can do is take all 24 patches into bluetooth-next and let them sit for 1 week and have people test them. And then we go ahead with reverting 3 patches from 3.14. Does that make sense?

>> Reverting those 3 patches works for me.

>> --
>> Sander

>>> Regards

>>> Marcel

> Hi Marcel,

> Ping... it seems these 3 reverts are still not in 3.14-rc5 to fix the regressions ?

> --
> Sander



-- 
Best regards,
 Sander                            mailto:linux@eikelenboom.it

[RC6 Bell Chime] Re: [PATCH 00/24] rfcomm fixes

[Sander Eikelenboom]

Hi all,

Since:
- 3.14-RC6 has been cut
- this regression is known and reported since the merge window
- the fix (revert of 3 patches) is known for over a month now
- but it's still not in mainline
- my polite ping request from last week seems to have provoked exactly 0 (zero) response.

IT'S TIME TO CHIME SOME BELLS :-)

Hope that WILL be heard somewhere ...

--
Sander

PS. on the informative side the 3 commits to be reverted are:

f86772af6a0f643d3e13eb3f4f9213ae0c333ee4 Bluetooth: Remove rfcomm_carrier_raised()
4a2fb3ecc7467c775b154813861f25a0ddc11aa0 Bluetooth: Always wait for a connection on RFCOMM open()
e228b63390536f5b737056059a9a04ea016b1abf Bluetooth: Move rfcomm_get_device() before rfcomm_dev_activate()


Monday, March 3, 2014, 8:38:53 PM, you wrote:


> Wednesday, February 12, 2014, 12:06:44 PM, you wrote:

>> Monday, February 10, 2014, 11:09:38 PM, you wrote:

>>> Hi Peter,

>>>> This patch series addresses a number of previously unknown issues
>>>> with the RFCOMM tty device implementation, in addition to
>>>> addressing the locking regression recently reported [1].
>>>> 
>>>> As Gianluca suggested and I agree, this series first reverts
>>>> 3 of the 4 patches of 3.14-rc1 for bluetooth/rfcomm/tty.c.

>>> so for 3.14 we should revert 3 patches. And then the other 21 are intended for 3.15 merge window.

>>> I realize that we still have to deal with some breakage, but we do not want regressions and I clearly not going to take 24 patches for 3.14 at this point in time.

>>> What I can do is take all 24 patches into bluetooth-next and let them sit for 1 week and have people test them. And then we go ahead with reverting 3 patches from 3.14. Does that make sense?

>> Reverting those 3 patches works for me.

>> --
>> Sander

>>> Regards

>>> Marcel

> Hi Marcel,

> Ping... it seems these 3 reverts are still not in 3.14-rc5 to fix the regressions ?

> --
> Sander



-- 
Best regards,
 Sander                            mailto:linux@eikelenboom.it

Re: [RC6 Bell Chime] Re: [PATCH 00/24] rfcomm fixes

[John W. Linville]

On Mon, Mar 10, 2014 at 09:38:43AM +0100, Sander Eikelenboom wrote:
> Hi all,
> 
> Since:
> - 3.14-RC6 has been cut
> - this regression is known and reported since the merge window
> - the fix (revert of 3 patches) is known for over a month now
> - but it's still not in mainline
> - my polite ping request from last week seems to have provoked exactly 0 (zero) response.
> 
> IT'S TIME TO CHIME SOME BELLS :-)
> 
> Hope that WILL be heard somewhere ...
> 
> --
> Sander
> 
> PS. on the informative side the 3 commits to be reverted are:
> 
> f86772af6a0f643d3e13eb3f4f9213ae0c333ee4 Bluetooth: Remove rfcomm_carrier_raised()
> 4a2fb3ecc7467c775b154813861f25a0ddc11aa0 Bluetooth: Always wait for a connection on RFCOMM open()
> e228b63390536f5b737056059a9a04ea016b1abf Bluetooth: Move rfcomm_get_device() before rfcomm_dev_activate()

Gustavo, should I be expecting a pull request?

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [RC6 Bell Chime] Re: [PATCH 00/24] rfcomm fixes

[John W. Linville]

On Mon, Mar 10, 2014 at 09:38:43AM +0100, Sander Eikelenboom wrote:
> Hi all,
> 
> Since:
> - 3.14-RC6 has been cut
> - this regression is known and reported since the merge window
> - the fix (revert of 3 patches) is known for over a month now
> - but it's still not in mainline
> - my polite ping request from last week seems to have provoked exactly 0 (zero) response.
> 
> IT'S TIME TO CHIME SOME BELLS :-)
> 
> Hope that WILL be heard somewhere ...
> 
> --
> Sander
> 
> PS. on the informative side the 3 commits to be reverted are:
> 
> f86772af6a0f643d3e13eb3f4f9213ae0c333ee4 Bluetooth: Remove rfcomm_carrier_raised()
> 4a2fb3ecc7467c775b154813861f25a0ddc11aa0 Bluetooth: Always wait for a connection on RFCOMM open()
> e228b63390536f5b737056059a9a04ea016b1abf Bluetooth: Move rfcomm_get_device() before rfcomm_dev_activate()

Gustavo, should I be expecting a pull request?

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [RC6 Bell Chime] [PATCH 00/24] rfcomm fixes

[Marcel Holtmann]

Hi John,

>> Since:
>> - 3.14-RC6 has been cut
>> - this regression is known and reported since the merge window
>> - the fix (revert of 3 patches) is known for over a month now
>> - but it's still not in mainline
>> - my polite ping request from last week seems to have provoked exactly 0 (zero) response.
>> 
>> IT'S TIME TO CHIME SOME BELLS :-)
>> 
>> Hope that WILL be heard somewhere ...
>> 
>> --
>> Sander
>> 
>> PS. on the informative side the 3 commits to be reverted are:
>> 
>> f86772af6a0f643d3e13eb3f4f9213ae0c333ee4 Bluetooth: Remove rfcomm_carrier_raised()
>> 4a2fb3ecc7467c775b154813861f25a0ddc11aa0 Bluetooth: Always wait for a connection on RFCOMM open()
>> e228b63390536f5b737056059a9a04ea016b1abf Bluetooth: Move rfcomm_get_device() before rfcomm_dev_activate()
> 
> Gustavo, should I be expecting a pull request?

you have all 3 reverts already in wireless-next as part of a larger RFCOMM change from Peter that we had to do to get this bug fixed. The whole series ended up as 24 patches and was way to late in the -rc stage. In addition we did not have enough exposure from people running that patch set. I did not wanted to end up in a ping-pong situation with apply + revert over and over again until we gave this some test exposure.

I think it is pretty safe to revert these 3 patches from 3.14-rc6 since they broke more than they actually fixed. However everybody has to be aware of that the real fix only comes in 3.15 since the change unfortunately is large. As far as we can tell, only users of RFCOMM TTY are affected. All RFCOMM socket users are fine.

So I do not know what the best way of getting these reverts merged. Gustavo has a tree ready for you to pull from. Or would it be better if they get cherry picked from wireless-next tree.

Regards

Marcel

Re: [RC6 Bell Chime] [PATCH 00/24] rfcomm fixes

[Marcel Holtmann]

Hi John,

>> Since:
>> - 3.14-RC6 has been cut
>> - this regression is known and reported since the merge window
>> - the fix (revert of 3 patches) is known for over a month now
>> - but it's still not in mainline
>> - my polite ping request from last week seems to have provoked exactly 0 (zero) response.
>> 
>> IT'S TIME TO CHIME SOME BELLS :-)
>> 
>> Hope that WILL be heard somewhere ...
>> 
>> --
>> Sander
>> 
>> PS. on the informative side the 3 commits to be reverted are:
>> 
>> f86772af6a0f643d3e13eb3f4f9213ae0c333ee4 Bluetooth: Remove rfcomm_carrier_raised()
>> 4a2fb3ecc7467c775b154813861f25a0ddc11aa0 Bluetooth: Always wait for a connection on RFCOMM open()
>> e228b63390536f5b737056059a9a04ea016b1abf Bluetooth: Move rfcomm_get_device() before rfcomm_dev_activate()
> 
> Gustavo, should I be expecting a pull request?

you have all 3 reverts already in wireless-next as part of a larger RFCOMM change from Peter that we had to do to get this bug fixed. The whole series ended up as 24 patches and was way to late in the -rc stage. In addition we did not have enough exposure from people running that patch set. I did not wanted to end up in a ping-pong situation with apply + revert over and over again until we gave this some test exposure.

I think it is pretty safe to revert these 3 patches from 3.14-rc6 since they broke more than they actually fixed. However everybody has to be aware of that the real fix only comes in 3.15 since the change unfortunately is large. As far as we can tell, only users of RFCOMM TTY are affected. All RFCOMM socket users are fine.

So I do not know what the best way of getting these reverts merged. Gustavo has a tree ready for you to pull from. Or would it be better if they get cherry picked from wireless-next tree.

Regards

Marcel

Re: [RC6 Bell Chime] [PATCH 00/24] rfcomm fixes

[Sander Eikelenboom]

Tuesday, March 11, 2014, 4:14:38 PM, you wrote:

> Hi John,

>>> Since:
>>> - 3.14-RC6 has been cut
>>> - this regression is known and reported since the merge window
>>> - the fix (revert of 3 patches) is known for over a month now
>>> - but it's still not in mainline
>>> - my polite ping request from last week seems to have provoked exactly 0 (zero) response.
>>> 
>>> IT'S TIME TO CHIME SOME BELLS :-)
>>> 
>>> Hope that WILL be heard somewhere ...
>>> 
>>> --
>>> Sander
>>> 
>>> PS. on the informative side the 3 commits to be reverted are:
>>> 
>>> f86772af6a0f643d3e13eb3f4f9213ae0c333ee4 Bluetooth: Remove rfcomm_carrier_raised()
>>> 4a2fb3ecc7467c775b154813861f25a0ddc11aa0 Bluetooth: Always wait for a connection on RFCOMM open()
>>> e228b63390536f5b737056059a9a04ea016b1abf Bluetooth: Move rfcomm_get_device() before rfcomm_dev_activate()
>> 
>> Gustavo, should I be expecting a pull request?

> you have all 3 reverts already in wireless-next as part of a larger RFCOMM change from Peter that we had to do to get this bug fixed. The whole series ended up as 24 patches and was way to late in the -rc stage. In addition we did not have enough exposure from people running that patch set. I did not wanted to end up in a ping-pong situation with apply + revert over and over again until we gave this some test exposure.

> I think it is pretty safe to revert these 3 patches from 3.14-rc6 since they broke more than they actually fixed. However everybody has to be aware of that the real fix only comes in 3.15 since the change unfortunately is large. As far as we can tell, only users of RFCOMM TTY are affected. All RFCOMM socket users are fine.

> So I do not know what the best way of getting these reverts merged. Gustavo has a tree ready for you to pull from. Or would it be better if they get cherry picked from wireless-next tree.

> Regards

> Marcel


<sarcasm on>
Is it just me .. or is this going at the speed of about a bluetooth connection ..
and probably missing the boot for 3.14 ? (for no good reason IMHO)
<sarcasm off>

(it was not in John's nor Dave's last pull request, although it seems to be reverted in the bluetooth tree now .. i didn't
see any formal pull request from that .. to get it even *starting* to traverse all the trees up to Linus ... )

--
Sander

Re: [RC6 Bell Chime] [PATCH 00/24] rfcomm fixes

[Sander Eikelenboom]

Tuesday, March 11, 2014, 4:14:38 PM, you wrote:

> Hi John,

>>> Since:
>>> - 3.14-RC6 has been cut
>>> - this regression is known and reported since the merge window
>>> - the fix (revert of 3 patches) is known for over a month now
>>> - but it's still not in mainline
>>> - my polite ping request from last week seems to have provoked exactly 0 (zero) response.
>>> 
>>> IT'S TIME TO CHIME SOME BELLS :-)
>>> 
>>> Hope that WILL be heard somewhere ...
>>> 
>>> --
>>> Sander
>>> 
>>> PS. on the informative side the 3 commits to be reverted are:
>>> 
>>> f86772af6a0f643d3e13eb3f4f9213ae0c333ee4 Bluetooth: Remove rfcomm_carrier_raised()
>>> 4a2fb3ecc7467c775b154813861f25a0ddc11aa0 Bluetooth: Always wait for a connection on RFCOMM open()
>>> e228b63390536f5b737056059a9a04ea016b1abf Bluetooth: Move rfcomm_get_device() before rfcomm_dev_activate()
>> 
>> Gustavo, should I be expecting a pull request?

> you have all 3 reverts already in wireless-next as part of a larger RFCOMM change from Peter that we had to do to get this bug fixed. The whole series ended up as 24 patches and was way to late in the -rc stage. In addition we did not have enough exposure from people running that patch set. I did not wanted to end up in a ping-pong situation with apply + revert over and over again until we gave this some test exposure.

> I think it is pretty safe to revert these 3 patches from 3.14-rc6 since they broke more than they actually fixed. However everybody has to be aware of that the real fix only comes in 3.15 since the change unfortunately is large. As far as we can tell, only users of RFCOMM TTY are affected. All RFCOMM socket users are fine.

> So I do not know what the best way of getting these reverts merged. Gustavo has a tree ready for you to pull from. Or would it be better if they get cherry picked from wireless-next tree.

> Regards

> Marcel


<sarcasm on>
Is it just me .. or is this going at the speed of about a bluetooth connection ..
and probably missing the boot for 3.14 ? (for no good reason IMHO)
<sarcasm off>

(it was not in John's nor Dave's last pull request, although it seems to be reverted in the bluetooth tree now .. i didn't
see any formal pull request from that .. to get it even *starting* to traverse all the trees up to Linus ... )

--
Sander

Re: [RC6 Bell Chime] [PATCH 00/24] rfcomm fixes

[Marcel Holtmann]

Hi Sander,

>>>> Since:
>>>> - 3.14-RC6 has been cut
>>>> - this regression is known and reported since the merge window
>>>> - the fix (revert of 3 patches) is known for over a month now
>>>> - but it's still not in mainline
>>>> - my polite ping request from last week seems to have provoked exactly 0 (zero) response.
>>>> 
>>>> IT'S TIME TO CHIME SOME BELLS :-)
>>>> 
>>>> Hope that WILL be heard somewhere ...
>>>> 
>>>> --
>>>> Sander
>>>> 
>>>> PS. on the informative side the 3 commits to be reverted are:
>>>> 
>>>> f86772af6a0f643d3e13eb3f4f9213ae0c333ee4 Bluetooth: Remove rfcomm_carrier_raised()
>>>> 4a2fb3ecc7467c775b154813861f25a0ddc11aa0 Bluetooth: Always wait for a connection on RFCOMM open()
>>>> e228b63390536f5b737056059a9a04ea016b1abf Bluetooth: Move rfcomm_get_device() before rfcomm_dev_activate()
>>> 
>>> Gustavo, should I be expecting a pull request?
> 
>> you have all 3 reverts already in wireless-next as part of a larger RFCOMM change from Peter that we had to do to get this bug fixed. The whole series ended up as 24 patches and was way to late in the -rc stage. In addition we did not have enough exposure from people running that patch set. I did not wanted to end up in a ping-pong situation with apply + revert over and over again until we gave this some test exposure.
> 
>> I think it is pretty safe to revert these 3 patches from 3.14-rc6 since they broke more than they actually fixed. However everybody has to be aware of that the real fix only comes in 3.15 since the change unfortunately is large. As far as we can tell, only users of RFCOMM TTY are affected. All RFCOMM socket users are fine.
> 
>> So I do not know what the best way of getting these reverts merged. Gustavo has a tree ready for you to pull from. Or would it be better if they get cherry picked from wireless-next tree.
> 
> 
> <sarcasm on>
> Is it just me .. or is this going at the speed of about a bluetooth connection ..
> and probably missing the boot for 3.14 ? (for no good reason IMHO)
> <sarcasm off>
> 
> (it was not in John's nor Dave's last pull request, although it seems to be reverted in the bluetooth tree now .. i didn't
> see any formal pull request from that .. to get it even *starting* to traverse all the trees up to Linus … )

the whole set is now in net-next so it is really up to John or Dave if they either want to cherry pick these 3 reverts or if they want to pull from bluetooth.git tree.

Regards

Marcel

Re: [RC6 Bell Chime] [PATCH 00/24] rfcomm fixes

[Marcel Holtmann]

Hi Sander,

>>>> Since:
>>>> - 3.14-RC6 has been cut
>>>> - this regression is known and reported since the merge window
>>>> - the fix (revert of 3 patches) is known for over a month now
>>>> - but it's still not in mainline
>>>> - my polite ping request from last week seems to have provoked exactly 0 (zero) response.
>>>> 
>>>> IT'S TIME TO CHIME SOME BELLS :-)
>>>> 
>>>> Hope that WILL be heard somewhere ...
>>>> 
>>>> --
>>>> Sander
>>>> 
>>>> PS. on the informative side the 3 commits to be reverted are:
>>>> 
>>>> f86772af6a0f643d3e13eb3f4f9213ae0c333ee4 Bluetooth: Remove rfcomm_carrier_raised()
>>>> 4a2fb3ecc7467c775b154813861f25a0ddc11aa0 Bluetooth: Always wait for a connection on RFCOMM open()
>>>> e228b63390536f5b737056059a9a04ea016b1abf Bluetooth: Move rfcomm_get_device() before rfcomm_dev_activate()
>>> 
>>> Gustavo, should I be expecting a pull request?
> 
>> you have all 3 reverts already in wireless-next as part of a larger RFCOMM change from Peter that we had to do to get this bug fixed. The whole series ended up as 24 patches and was way to late in the -rc stage. In addition we did not have enough exposure from people running that patch set. I did not wanted to end up in a ping-pong situation with apply + revert over and over again until we gave this some test exposure.
> 
>> I think it is pretty safe to revert these 3 patches from 3.14-rc6 since they broke more than they actually fixed. However everybody has to be aware of that the real fix only comes in 3.15 since the change unfortunately is large. As far as we can tell, only users of RFCOMM TTY are affected. All RFCOMM socket users are fine.
> 
>> So I do not know what the best way of getting these reverts merged. Gustavo has a tree ready for you to pull from. Or would it be better if they get cherry picked from wireless-next tree.
> 
> 
> <sarcasm on>
> Is it just me .. or is this going at the speed of about a bluetooth connection ..
> and probably missing the boot for 3.14 ? (for no good reason IMHO)
> <sarcasm off>
> 
> (it was not in John's nor Dave's last pull request, although it seems to be reverted in the bluetooth tree now .. i didn't
> see any formal pull request from that .. to get it even *starting* to traverse all the trees up to Linus … )

the whole set is now in net-next so it is really up to John or Dave if they either want to cherry pick these 3 reverts or if they want to pull from bluetooth.git tree.

Regards

Marcel

Re: [RC6 Bell Chime] [PATCH 00/24] rfcomm fixes

[Peter Hurley]

Hi Sander,

On 03/13/2014 08:49 PM, Sander Eikelenboom wrote:
> <sarcasm on>
> Is it just me .. or is this going at the speed of about a bluetooth connection ..
> and probably missing the boot for 3.14 ? (for no good reason IMHO)
> <sarcasm off>
>
> (it was not in John's nor Dave's last pull request, although it seems to be reverted in the bluetooth tree now .. i didn't
> see any formal pull request from that .. to get it even *starting* to traverse all the trees up to Linus ... )

Known bugs sometimes roll out into mainline release because the
alternative can be worse.

As I explained in the follow-up to my patch series, I would
not have expected Marcel to pick up any of the fixes for 3.14.
There are a lot of moving parts in usb + bluetooth + rfcomm + tty,
and the unfortunate reality is that -next doesn't get as much
testing as it should.

The fault is mine because Gianluca let me know about the
problems with the conversion to tty_port, but the holidays really
interfered with my ability to put this work first, and I'm sorry
for that.

I know the breakage around RFCOMM is frustrating but I think the
worst is behind us. After 3.15 gets some -rc testing, I will
be happy to cherry-pick the critical fixes for -stable inclusion.

Regards,
Peter Hurley

Re: [RC6 Bell Chime] [PATCH 00/24] rfcomm fixes

[Peter Hurley]

Hi Sander,

On 03/13/2014 08:49 PM, Sander Eikelenboom wrote:
> <sarcasm on>
> Is it just me .. or is this going at the speed of about a bluetooth connection ..
> and probably missing the boot for 3.14 ? (for no good reason IMHO)
> <sarcasm off>
>
> (it was not in John's nor Dave's last pull request, although it seems to be reverted in the bluetooth tree now .. i didn't
> see any formal pull request from that .. to get it even *starting* to traverse all the trees up to Linus ... )

Known bugs sometimes roll out into mainline release because the
alternative can be worse.

As I explained in the follow-up to my patch series, I would
not have expected Marcel to pick up any of the fixes for 3.14.
There are a lot of moving parts in usb + bluetooth + rfcomm + tty,
and the unfortunate reality is that -next doesn't get as much
testing as it should.

The fault is mine because Gianluca let me know about the
problems with the conversion to tty_port, but the holidays really
interfered with my ability to put this work first, and I'm sorry
for that.

I know the breakage around RFCOMM is frustrating but I think the
worst is behind us. After 3.15 gets some -rc testing, I will
be happy to cherry-pick the critical fixes for -stable inclusion.

Regards,
Peter Hurley

Re: [RC6 Bell Chime] [PATCH 00/24] rfcomm fixes

[Sander Eikelenboom]

Friday, March 14, 2014, 2:29:43 AM, you wrote:

> Hi Sander,

> On 03/13/2014 08:49 PM, Sander Eikelenboom wrote:
>> <sarcasm on>
>> Is it just me .. or is this going at the speed of about a bluetooth connection ..
>> and probably missing the boot for 3.14 ? (for no good reason IMHO)
>> <sarcasm off>
>>
>> (it was not in John's nor Dave's last pull request, although it seems to be reverted in the bluetooth tree now .. i didn't
>> see any formal pull request from that .. to get it even *starting* to traverse all the trees up to Linus ... )

> Known bugs sometimes roll out into mainline release because the
> alternative can be worse.

> As I explained in the follow-up to my patch series, I would
> not have expected Marcel to pick up any of the fixes for 3.14.
> There are a lot of moving parts in usb + bluetooth + rfcomm + tty,
> and the unfortunate reality is that -next doesn't get as much
> testing as it should.

> The fault is mine because Gianluca let me know about the
> problems with the conversion to tty_port, but the holidays really
> interfered with my ability to put this work first, and I'm sorry
> for that.

> I know the breakage around RFCOMM is frustrating but I think the
> worst is behind us. After 3.15 gets some -rc testing, I will
> be happy to cherry-pick the critical fixes for -stable inclusion.

Ok but the breakage/regression was known since around the merge window.
I thought the "standard policy" when things cause new regression and are not fixable (too intrusive, too time consuming you
name the reason), was to revert .. (and possibly ASAP so the reverts also get some test coverage in mainline RC's).
That's also why i tested the revert ASAP to let you know that that worked  .. at least for me.

But no big deal for mee .. the reverts are simple enough to be privately applied.

> Regards,
> Peter Hurley

Re: [RC6 Bell Chime] [PATCH 00/24] rfcomm fixes

[Sander Eikelenboom]

Friday, March 14, 2014, 2:29:43 AM, you wrote:

> Hi Sander,

> On 03/13/2014 08:49 PM, Sander Eikelenboom wrote:
>> <sarcasm on>
>> Is it just me .. or is this going at the speed of about a bluetooth connection ..
>> and probably missing the boot for 3.14 ? (for no good reason IMHO)
>> <sarcasm off>
>>
>> (it was not in John's nor Dave's last pull request, although it seems to be reverted in the bluetooth tree now .. i didn't
>> see any formal pull request from that .. to get it even *starting* to traverse all the trees up to Linus ... )

> Known bugs sometimes roll out into mainline release because the
> alternative can be worse.

> As I explained in the follow-up to my patch series, I would
> not have expected Marcel to pick up any of the fixes for 3.14.
> There are a lot of moving parts in usb + bluetooth + rfcomm + tty,
> and the unfortunate reality is that -next doesn't get as much
> testing as it should.

> The fault is mine because Gianluca let me know about the
> problems with the conversion to tty_port, but the holidays really
> interfered with my ability to put this work first, and I'm sorry
> for that.

> I know the breakage around RFCOMM is frustrating but I think the
> worst is behind us. After 3.15 gets some -rc testing, I will
> be happy to cherry-pick the critical fixes for -stable inclusion.

Ok but the breakage/regression was known since around the merge window.
I thought the "standard policy" when things cause new regression and are not fixable (too intrusive, too time consuming you
name the reason), was to revert .. (and possibly ASAP so the reverts also get some test coverage in mainline RC's).
That's also why i tested the revert ASAP to let you know that that worked  .. at least for me.

But no big deal for mee .. the reverts are simple enough to be privately applied.

> Regards,
> Peter Hurley

Re: [RC6 Bell Chime] [PATCH 00/24] rfcomm fixes

[Linus Torvalds]

On Sat, Mar 15, 2014 at 6:51 AM, Sander Eikelenboom
<linux@eikelenboom.it> wrote:
>
>
> Ok but the breakage/regression was known since around the merge window.
> I thought the "standard policy" when things cause new regression and are not fixable (too intrusive, too time consuming you
> name the reason), was to revert ..

It is indeed.

Guys, why is this being discussed? We should not release a
known-broken 3.14 just because the fix is too big for rc. Things
should be reverted.

                   Linus

Re: [RC6 Bell Chime] [PATCH 00/24] rfcomm fixes

[Linus Torvalds]

On Sat, Mar 15, 2014 at 6:51 AM, Sander Eikelenboom
<linux@eikelenboom.it> wrote:
>
>
> Ok but the breakage/regression was known since around the merge window.
> I thought the "standard policy" when things cause new regression and are not fixable (too intrusive, too time consuming you
> name the reason), was to revert ..

It is indeed.

Guys, why is this being discussed? We should not release a
known-broken 3.14 just because the fix is too big for rc. Things
should be reverted.

                   Linus

Re: [RC6 Bell Chime] [PATCH 00/24] rfcomm fixes

[Peter Hurley]

On 03/15/2014 01:53 PM, Linus Torvalds wrote:
> Guys, why is this being discussed?

FWIW, the 'known breakage' for 3.14 is a (valid) lockdep report.

This regression was introduced by a small patchset added to -next
over the holidays that was intended to address 2 bug reports
stemming from a long-overdue overhaul of the RFCOMM tty driver by
Gianluca Anzolin (which fixed numerous problems and several hangs
reported since 3.8).

It is the bulk of that small patchset which is being reverted.

The point of this brief history is that:
1) the 3.13 state of rfcomm is just as broken as 3.14-rcX, but
    in a different way
2) there are plenty of serious defects in both versions regardless.

I mean for this to be informative and not argumentative --
either outcome is ok with me. In fact, I'm ok if you want to
pick my entire 24-patch series that addresses the bugs in 3.13
and 3.14, plus a bunch of other problems that I found at the time.

Regards,
Peter Hurley

Re: [RC6 Bell Chime] [PATCH 00/24] rfcomm fixes

[Peter Hurley]

On 03/15/2014 01:53 PM, Linus Torvalds wrote:
> Guys, why is this being discussed?

FWIW, the 'known breakage' for 3.14 is a (valid) lockdep report.

This regression was introduced by a small patchset added to -next
over the holidays that was intended to address 2 bug reports
stemming from a long-overdue overhaul of the RFCOMM tty driver by
Gianluca Anzolin (which fixed numerous problems and several hangs
reported since 3.8).

It is the bulk of that small patchset which is being reverted.

The point of this brief history is that:
1) the 3.13 state of rfcomm is just as broken as 3.14-rcX, but
    in a different way
2) there are plenty of serious defects in both versions regardless.

I mean for this to be informative and not argumentative --
either outcome is ok with me. In fact, I'm ok if you want to
pick my entire 24-patch series that addresses the bugs in 3.13
and 3.14, plus a bunch of other problems that I found at the time.

Regards,
Peter Hurley

Re: [RC6 Bell Chime] [PATCH 00/24] rfcomm fixes

[Sander Eikelenboom]

Saturday, March 15, 2014, 9:45:03 PM, you wrote:

> On 03/15/2014 01:53 PM, Linus Torvalds wrote:
>> Guys, why is this being discussed?

> FWIW, the 'known breakage' for 3.14 is a (valid) lockdep report.

Hmm .. whoops you are right .. i remembered it as being an "oops",
but you are right it was merely an lockdep warning.
Should have checked that before putting up my big mouth .. sorry for that!

> This regression was introduced by a small patchset added to -next
> over the holidays that was intended to address 2 bug reports
> stemming from a long-overdue overhaul of the RFCOMM tty driver by
> Gianluca Anzolin (which fixed numerous problems and several hangs
> reported since 3.8).

Ok if what went in for 3.14 fixes some hangs that should probably outweigh
the lockdep warning regression.

> It is the bulk of that small patchset which is being reverted.

> The point of this brief history is that:
> 1) the 3.13 state of rfcomm is just as broken as 3.14-rcX, but
>     in a different way
> 2) there are plenty of serious defects in both versions regardless.

> I mean for this to be informative and not argumentative --
> either outcome is ok with me. In fact, I'm ok if you want to
> pick my entire 24-patch series that addresses the bugs in 3.13
> and 3.14, plus a bunch of other problems that I found at the time.

> Regards,
> Peter Hurley

Re: [RC6 Bell Chime] [PATCH 00/24] rfcomm fixes

[Sander Eikelenboom]

Saturday, March 15, 2014, 9:45:03 PM, you wrote:

> On 03/15/2014 01:53 PM, Linus Torvalds wrote:
>> Guys, why is this being discussed?

> FWIW, the 'known breakage' for 3.14 is a (valid) lockdep report.

Hmm .. whoops you are right .. i remembered it as being an "oops",
but you are right it was merely an lockdep warning.
Should have checked that before putting up my big mouth .. sorry for that!

> This regression was introduced by a small patchset added to -next
> over the holidays that was intended to address 2 bug reports
> stemming from a long-overdue overhaul of the RFCOMM tty driver by
> Gianluca Anzolin (which fixed numerous problems and several hangs
> reported since 3.8).

Ok if what went in for 3.14 fixes some hangs that should probably outweigh
the lockdep warning regression.

> It is the bulk of that small patchset which is being reverted.

> The point of this brief history is that:
> 1) the 3.13 state of rfcomm is just as broken as 3.14-rcX, but
>     in a different way
> 2) there are plenty of serious defects in both versions regardless.

> I mean for this to be informative and not argumentative --
> either outcome is ok with me. In fact, I'm ok if you want to
> pick my entire 24-patch series that addresses the bugs in 3.13
> and 3.14, plus a bunch of other problems that I found at the time.

> Regards,
> Peter Hurley

Re: [RC6 Bell Chime] [PATCH 00/24] rfcomm fixes

[Linus Torvalds]

On Sat, Mar 15, 2014 at 1:45 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>
> FWIW, the 'known breakage' for 3.14 is a (valid) lockdep report.

Ok, if it's just lockdep and unlikely to hit in real life and getting
fixed later, I guess I don't really care all that deeply for 3.14

                  Linus

Re: [RC6 Bell Chime] [PATCH 00/24] rfcomm fixes

[Linus Torvalds]

On Sat, Mar 15, 2014 at 1:45 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>
> FWIW, the 'known breakage' for 3.14 is a (valid) lockdep report.

Ok, if it's just lockdep and unlikely to hit in real life and getting
fixed later, I guess I don't really care all that deeply for 3.14

                  Linus