Bluetooth: Introduce separate pool for discardable events

Johan Hedberg · jhedberg · commit 0d9dab300eaf · 2019-07-01T16:36:15.000+03:00
Introduce a separate buffer pool for events which the HCI driver
considers discardable. Examples of such events could be e.g.
Advertising Reports. The benefit of having such a pool means that the
if there is a heavy inflow of such events it will not cause the
allocation for other critical events to block and may even eliminate
deadlocks in some cases.

Also update all mesh samples not to specify explicit RX buffer counts
anymore. Instead, create appropriate defaults in Kconfig so that we
only need to override this in the app for cases like the bbc:microbit
with limited memory.

Signed-off-by: Johan Hedberg &lt;johan.hedberg@intel.com&gt;
diff --git a/drivers/bluetooth/hci/h4.c b/drivers/bluetooth/hci/h4.c
@@ -162,7 +162,7 @@ static struct net_buf *get_rx(int timeout)
 	BT_DBG("type 0x%02x, evt 0x%02x", rx.type, rx.evt.evt);
 
 	if (rx.type == H4_EVT) {
-		return bt_buf_get_evt(rx.evt.evt, timeout);
+		return bt_buf_get_evt(rx.evt.evt, rx.discardable, timeout);
 	}
 
 	return bt_buf_get_rx(BT_BUF_ACL_IN, timeout);
diff --git a/drivers/bluetooth/hci/h5.c b/drivers/bluetooth/hci/h5.c
@@ -408,7 +408,7 @@ static inline struct net_buf *get_evt_buf(u8_t evt)
 {
 	struct net_buf *buf;
 
-	buf = bt_buf_get_evt(evt, K_NO_WAIT);
+	buf = bt_buf_get_evt(evt, false, K_NO_WAIT);
 	if (buf) {
 		net_buf_add_u8(h5.rx_buf, evt);
 	}
diff --git a/drivers/bluetooth/hci/ipm_stm32wb.c b/drivers/bluetooth/hci/ipm_stm32wb.c
@@ -115,7 +115,8 @@ void TM_EvtReceivedCb(TL_EvtPacket_t *hcievt)
 			       hcievt->evtserial.evt.evtcode);
 			goto out;
 		default:
-			buf = bt_buf_get_evt(evtserial.evt.evtcode, K_FOREVER);
+			buf = bt_buf_get_evt(evtserial.evt.evtcode, false,
+					     K_FOREVER);
 			break;
 		}
 		net_buf_add_mem(buf, &hcievt->evtserial.evt,
diff --git a/drivers/bluetooth/hci/spi.c b/drivers/bluetooth/hci/spi.c
@@ -355,7 +355,7 @@ static void bt_spi_rx_thread(void)
 					continue;
 				default:
 					buf = bt_buf_get_evt(rxmsg[EVT_HEADER_EVENT],
-							     K_FOREVER);
+							     false, K_FOREVER);
 					break;
 				}
 
diff --git a/drivers/bluetooth/hci/userchan.c b/drivers/bluetooth/hci/userchan.c
@@ -58,7 +58,7 @@ static int bt_dev_index = -1;
 static struct net_buf *get_rx(const u8_t *buf)
 {
 	if (buf[0] == H4_EVT) {
-		return bt_buf_get_evt(buf[1], K_FOREVER);
+		return bt_buf_get_evt(buf[1], false, K_FOREVER);
 	}
 
 	return bt_buf_get_rx(BT_BUF_ACL_IN, K_FOREVER);
diff --git a/include/bluetooth/buf.h b/include/bluetooth/buf.h
@@ -69,12 +69,13 @@ struct net_buf *bt_buf_get_cmd_complete(s32_t timeout);
  *  This will set the buffer type so bt_buf_set_type() does not need to
  *  be explicitly called before bt_recv_prio() or bt_recv().
  *
- *  @param evt     HCI event code
- *  @param timeout Timeout in milliseconds, or one of the special values
- *                 K_NO_WAIT and K_FOREVER.
+ *  @param evt          HCI event code
+ *  @param discardable  Whether the driver considers the event discardable.
+ *  @param timeout      Timeout in milliseconds, or one of the special values
+ *                      K_NO_WAIT and K_FOREVER.
  *  @return A new buffer.
  */
-struct net_buf *bt_buf_get_evt(u8_t evt, s32_t timeout);
+struct net_buf *bt_buf_get_evt(u8_t evt, bool discardable, s32_t timeout);
 
 /** Set the buffer type
  *
diff --git a/include/bluetooth/hci.h b/include/bluetooth/hci.h
@@ -1362,6 +1362,7 @@ struct bt_hci_cp_le_set_privacy_mode {
 
 /* Event definitions */
 
+#define BT_HCI_EVT_UNKNOWN                      0x00
 #define BT_HCI_EVT_VENDOR                       0xff
 
 #define BT_HCI_EVT_INQUIRY_COMPLETE             0x01
diff --git a/samples/bluetooth/mesh/prj.conf b/samples/bluetooth/mesh/prj.conf
@@ -26,7 +26,6 @@ CONFIG_BT_PERIPHERAL=y
 
 CONFIG_BT=y
 CONFIG_BT_TINYCRYPT_ECC=y
-CONFIG_BT_RX_BUF_COUNT=30
 CONFIG_BT_L2CAP_RX_MTU=69
 CONFIG_BT_L2CAP_TX_MTU=69
 CONFIG_BT_L2CAP_TX_BUF_COUNT=5
diff --git a/samples/bluetooth/mesh/prj_bbc_microbit.conf b/samples/bluetooth/mesh/prj_bbc_microbit.conf
@@ -17,6 +17,8 @@ CONFIG_SETTINGS_FCB=y
 CONFIG_BT=y
 CONFIG_BT_TINYCRYPT_ECC=y
 CONFIG_BT_RX_STACK_SIZE=1100
+CONFIG_BT_RX_BUF_COUNT=3
+CONFIG_BT_DISCARDABLE_BUF_COUNT=3
 
 CONFIG_BT_CTLR_DUP_FILTER_LEN=0
 CONFIG_BT_OBSERVER=y
diff --git a/samples/bluetooth/mesh_demo/prj.conf b/samples/bluetooth/mesh_demo/prj.conf
@@ -3,7 +3,6 @@ CONFIG_MAIN_STACK_SIZE=512
 CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE=2048
 
 CONFIG_BT=y
-CONFIG_BT_RX_BUF_COUNT=30
 CONFIG_BT_TINYCRYPT_ECC=y
 #CONFIG_BT_DEBUG_LOG=y
 CONFIG_BT_OBSERVER=y
diff --git a/samples/boards/nrf52/mesh/onoff-app/prj.conf b/samples/boards/nrf52/mesh/onoff-app/prj.conf
@@ -31,7 +31,6 @@ CONFIG_BT_PERIPHERAL=y
 
 CONFIG_BT=y
 CONFIG_BT_TINYCRYPT_ECC=y
-CONFIG_BT_RX_BUF_COUNT=30
 CONFIG_BT_L2CAP_RX_MTU=69
 CONFIG_BT_L2CAP_TX_MTU=69
 CONFIG_BT_L2CAP_TX_BUF_COUNT=8
diff --git a/samples/boards/nrf52/mesh/onoff_level_lighting_vnd_app/prj.conf b/samples/boards/nrf52/mesh/onoff_level_lighting_vnd_app/prj.conf
@@ -26,7 +26,6 @@ CONFIG_BT_CTLR_TX_PWR_PLUS_8=y
 CONFIG_BT_PERIPHERAL=y
 CONFIG_BT=y
 CONFIG_BT_TINYCRYPT_ECC=y
-CONFIG_BT_RX_BUF_COUNT=30
 CONFIG_BT_RX_STACK_SIZE=4096
 CONFIG_BT_L2CAP_RX_MTU=69
 CONFIG_BT_L2CAP_TX_MTU=69
diff --git a/samples/boards/reel_board/mesh_badge/prj.conf b/samples/boards/reel_board/mesh_badge/prj.conf
@@ -19,7 +19,6 @@ CONFIG_BT_MESH_DEBUG=y
 
 CONFIG_BT_OBSERVER=y
 CONFIG_BT_BROADCASTER=y
-CONFIG_BT_RX_BUF_COUNT=30
 CONFIG_BT_L2CAP_RX_MTU=69
 CONFIG_BT_L2CAP_TX_MTU=69
 CONFIG_BT_L2CAP_TX_BUF_COUNT=8
diff --git a/subsys/bluetooth/controller/hci/hci.c b/subsys/bluetooth/controller/hci/hci.c
@@ -120,7 +120,7 @@ void *hci_cmd_complete(struct net_buf **buf, u8_t plen)
 {
 	struct bt_hci_evt_cmd_complete *cc;
 
-	*buf = bt_buf_get_evt(BT_HCI_EVT_CMD_COMPLETE, K_FOREVER);
+	*buf = bt_buf_get_evt(BT_HCI_EVT_CMD_COMPLETE, false, K_FOREVER);
 
 	hci_evt_create(*buf, BT_HCI_EVT_CMD_COMPLETE, sizeof(*cc) + plen);
 
@@ -137,7 +137,7 @@ static struct net_buf *cmd_status(u8_t status)
 	struct bt_hci_evt_cmd_status *cs;
 	struct net_buf *buf;
 
-	buf = bt_buf_get_evt(BT_HCI_EVT_CMD_STATUS, K_FOREVER);
+	buf = bt_buf_get_evt(BT_HCI_EVT_CMD_STATUS, false, K_FOREVER);
 	hci_evt_create(buf, BT_HCI_EVT_CMD_STATUS, sizeof(*cs));
 
 	cs = net_buf_add(buf, sizeof(*cs));
diff --git a/subsys/bluetooth/controller/hci/hci_driver.c b/subsys/bluetooth/controller/hci/hci_driver.c
@@ -91,7 +91,7 @@ static void prio_recv_thread(void *p1, void *p2, void *p3)
 			struct net_buf *buf;
 
 			buf = bt_buf_get_evt(BT_HCI_EVT_NUM_COMPLETED_PACKETS,
-					     K_FOREVER);
+					     false, K_FOREVER);
 			hci_num_cmplt_encode(buf, handle, num_cmplt);
 			BT_DBG("Num Complete: 0x%04x:%u", handle, num_cmplt);
 			bt_recv_prio(buf);
@@ -146,7 +146,8 @@ static inline struct net_buf *encode_node(struct node_rx_pdu *node_rx,
 	case HCI_CLASS_EVT_REQUIRED:
 	case HCI_CLASS_EVT_CONNECTION:
 		if (class == HCI_CLASS_EVT_DISCARDABLE) {
-			buf = bt_buf_get_rx(BT_BUF_EVT, K_NO_WAIT);
+			buf = bt_buf_get_evt(BT_HCI_EVT_UNKNOWN, true,
+					     K_NO_WAIT);
 		} else {
 			buf = bt_buf_get_rx(BT_BUF_EVT, K_FOREVER);
 		}
diff --git a/subsys/bluetooth/host/Kconfig b/subsys/bluetooth/host/Kconfig
@@ -26,6 +26,7 @@ config BT_HCI_CMD_COUNT
 config BT_RX_BUF_COUNT
 	int "Number of HCI RX buffers"
 	default 3 if BT_RECV_IS_RX_THREAD
+	default 20 if (BT_MESH && !BT_DISCARDABLE_BUF_COUNT)
 	default 10
 	range 2 255
 	help
@@ -47,6 +48,20 @@ config BT_RX_BUF_LEN
 	  an L2CAP MTU of 65 bytes. On top of this there's the L2CAP header
 	  (4 bytes) and the ACL header (also 4 bytes) which yields 73 bytes.
 
+config BT_DISCARDABLE_BUF_COUNT
+	int "Number of discardable event buffers"
+	range 1 255
+	default 20 if BT_MESH
+	default 3
+	depends on BT_H4 || BT_CTLR
+	help
+	  Number of buffers in a separate buffer pool for events which
+	  the HCI driver considers discardable. Examples of such events
+	  could be e.g. Advertising Reports. The benefit of having such
+	  a pool means that the if there is a heavy inflow of such events
+	  it will not cause the allocation for other critical events to
+	  block and may even eliminate deadlocks in some cases.
+
 config BT_HCI_TX_STACK_SIZE
 	# NOTE: This value is derived from other symbols and should not be
 	# user-configurable. Do not give it a prompt.
diff --git a/subsys/bluetooth/host/hci_core.c b/subsys/bluetooth/host/hci_core.c
@@ -148,6 +148,11 @@ NET_BUF_POOL_DEFINE(num_complete_pool, 1, BT_BUF_RX_SIZE,
 		    BT_BUF_USER_DATA_MIN, NULL);
 #endif /* CONFIG_BT_CONN */
 
+#if defined(CONFIG_BT_DISCARDABLE_BUF_COUNT)
+NET_BUF_POOL_DEFINE(discardable_pool, CONFIG_BT_DISCARDABLE_BUF_COUNT,
+		    BT_BUF_RX_SIZE, BT_BUF_USER_DATA_MIN, NULL);
+#endif /* CONFIG_BT_DISCARDABLE_BUF_COUNT */
+
 struct event_handler {
 	u8_t event;
 	u8_t min_len;
@@ -5672,7 +5677,7 @@ struct net_buf *bt_buf_get_cmd_complete(s32_t timeout)
 	return bt_buf_get_rx(BT_BUF_EVT, timeout);
 }
 
-struct net_buf *bt_buf_get_evt(u8_t evt, s32_t timeout)
+struct net_buf *bt_buf_get_evt(u8_t evt, bool discardable, s32_t timeout)
 {
 	switch (evt) {
 #if defined(CONFIG_BT_CONN)
@@ -5693,6 +5698,20 @@ struct net_buf *bt_buf_get_evt(u8_t evt, s32_t timeout)
 	case BT_HCI_EVT_CMD_STATUS:
 		return bt_buf_get_cmd_complete(timeout);
 	default:
+#if defined(CONFIG_BT_DISCARDABLE_BUF_COUNT)
+		if (discardable) {
+			struct net_buf *buf;
+
+			buf = net_buf_alloc(&discardable_pool, timeout);
+			if (buf) {
+				net_buf_reserve(buf, CONFIG_BT_HCI_RESERVE);
+				bt_buf_set_type(buf, BT_BUF_EVT);
+			}
+
+			return buf;
+		}
+#endif /* CONFIG_BT_DISCARDABLE_BUF_COUNT */
+
 		return bt_buf_get_rx(BT_BUF_EVT, timeout);
 	}
 }
diff --git a/subsys/bluetooth/host/hci_ecc.c b/subsys/bluetooth/host/hci_ecc.c
@@ -84,7 +84,7 @@ static void send_cmd_status(u16_t opcode, u8_t status)
 
 	BT_DBG("opcode %x status %x", opcode, status);
 
-	buf = bt_buf_get_evt(BT_HCI_EVT_CMD_STATUS, K_FOREVER);
+	buf = bt_buf_get_evt(BT_HCI_EVT_CMD_STATUS, false, K_FOREVER);
 	bt_buf_set_type(buf, BT_BUF_EVT);
 
 	hdr = net_buf_add(buf, sizeof(*hdr));
diff --git a/subsys/bluetooth/host/hci_raw.c b/subsys/bluetooth/host/hci_raw.c
@@ -72,7 +72,7 @@ struct net_buf *bt_buf_get_cmd_complete(s32_t timeout)
 	return buf;
 }
 
-struct net_buf *bt_buf_get_evt(u8_t evt, s32_t timeout)
+struct net_buf *bt_buf_get_evt(u8_t evt, bool discardable, s32_t timeout)
 {
 	struct net_buf *buf;
 
diff --git a/tests/bluetooth/hci_prop_evt/src/main.c b/tests/bluetooth/hci_prop_evt/src/main.c
@@ -53,7 +53,7 @@ static void *cmd_complete(struct net_buf **buf, u8_t plen, u16_t opcode)
 {
 	struct bt_hci_evt_cmd_complete *cc;
 
-	*buf = bt_buf_get_evt(BT_HCI_EVT_CMD_COMPLETE, K_FOREVER);
+	*buf = bt_buf_get_evt(BT_HCI_EVT_CMD_COMPLETE, false, K_FOREVER);
 	evt_create(*buf, BT_HCI_EVT_CMD_COMPLETE, sizeof(*cc) + plen);
 	cc = net_buf_add(*buf, sizeof(*cc));
 	cc->ncmd = 1U;
diff --git a/tests/bluetooth/mesh/dbg.conf b/tests/bluetooth/mesh/dbg.conf
@@ -19,7 +19,6 @@ CONFIG_BT_PERIPHERAL=y
 
 CONFIG_BT=y
 CONFIG_BT_TINYCRYPT_ECC=y
-CONFIG_BT_RX_BUF_COUNT=30
 CONFIG_BT_L2CAP_RX_MTU=69
 CONFIG_BT_L2CAP_TX_MTU=69
 
diff --git a/tests/bluetooth/mesh/friend.conf b/tests/bluetooth/mesh/friend.conf
@@ -17,7 +17,6 @@ CONFIG_BT_CTLR_PRIVACY=n
 
 CONFIG_BT=y
 CONFIG_BT_TINYCRYPT_ECC=y
-CONFIG_BT_RX_BUF_COUNT=30
 
 CONFIG_BT_MESH=y
 CONFIG_BT_MESH_RELAY=y
diff --git a/tests/bluetooth/mesh/gatt.conf b/tests/bluetooth/mesh/gatt.conf
@@ -19,7 +19,6 @@ CONFIG_BT_PERIPHERAL=y
 
 CONFIG_BT=y
 CONFIG_BT_TINYCRYPT_ECC=y
-CONFIG_BT_RX_BUF_COUNT=30
 CONFIG_BT_L2CAP_RX_MTU=69
 CONFIG_BT_L2CAP_TX_MTU=69
 
diff --git a/tests/bluetooth/mesh/lpn.conf b/tests/bluetooth/mesh/lpn.conf
@@ -19,7 +19,6 @@ CONFIG_BT_PERIPHERAL=y
 
 CONFIG_BT=y
 CONFIG_BT_TINYCRYPT_ECC=y
-CONFIG_BT_RX_BUF_COUNT=30
 CONFIG_BT_L2CAP_RX_MTU=69
 CONFIG_BT_L2CAP_TX_MTU=69
 
diff --git a/tests/bluetooth/mesh/pb_gatt.conf b/tests/bluetooth/mesh/pb_gatt.conf
@@ -19,7 +19,6 @@ CONFIG_BT_PERIPHERAL=y
 
 CONFIG_BT=y
 CONFIG_BT_TINYCRYPT_ECC=y
-CONFIG_BT_RX_BUF_COUNT=30
 CONFIG_BT_L2CAP_RX_MTU=69
 CONFIG_BT_L2CAP_TX_MTU=69
 
diff --git a/tests/bluetooth/mesh/prj.conf b/tests/bluetooth/mesh/prj.conf
@@ -19,7 +19,6 @@ CONFIG_BT_PERIPHERAL=y
 
 CONFIG_BT=y
 CONFIG_BT_TINYCRYPT_ECC=y
-CONFIG_BT_RX_BUF_COUNT=30
 CONFIG_BT_L2CAP_RX_MTU=69
 CONFIG_BT_L2CAP_TX_MTU=69
 
diff --git a/tests/bluetooth/mesh/proxy.conf b/tests/bluetooth/mesh/proxy.conf
@@ -19,7 +19,6 @@ CONFIG_BT_PERIPHERAL=y
 
 CONFIG_BT=y
 CONFIG_BT_TINYCRYPT_ECC=y
-CONFIG_BT_RX_BUF_COUNT=30
 CONFIG_BT_L2CAP_RX_MTU=69
 CONFIG_BT_L2CAP_TX_MTU=69
 
diff --git a/tests/bluetooth/mesh_shell/prj.conf b/tests/bluetooth/mesh_shell/prj.conf
@@ -34,7 +34,6 @@ CONFIG_BT_CTLR_MIN_USED_CHAN=n
 CONFIG_BT_CTLR_ADV_EXT=n
 CONFIG_BT_CTLR_PRIVACY=n
 
-CONFIG_BT_RX_BUF_COUNT=30
 CONFIG_BT_L2CAP_RX_MTU=69
 CONFIG_BT_L2CAP_TX_MTU=69
 CONFIG_BT_L2CAP_TX_BUF_COUNT=4
diff --git a/tests/bluetooth/shell/mesh.conf b/tests/bluetooth/shell/mesh.conf
@@ -18,7 +18,6 @@ CONFIG_BT_SHELL=y
 CONFIG_BT_DEVICE_NAME="test shell"
 CONFIG_BT_L2CAP_TX_BUF_COUNT=6
 
-CONFIG_BT_RX_BUF_COUNT=30
 CONFIG_BT_L2CAP_RX_MTU=69
 CONFIG_BT_L2CAP_TX_MTU=69
 

Original file line number	Diff line number	Diff line change
`@@ -162,7 +162,7 @@ static struct net_buf *get_rx(int timeout)`
`162`	`162`	`BT_DBG("type 0x%02x, evt 0x%02x", rx.type, rx.evt.evt);`
`163`	`163`
`164`	`164`	`if (rx.type == H4_EVT) {`
`165`		`- return bt_buf_get_evt(rx.evt.evt, timeout);`
	`165`	`+ return bt_buf_get_evt(rx.evt.evt, rx.discardable, timeout);`
`166`	`166`	`}`
`167`	`167`
`168`	`168`	`return bt_buf_get_rx(BT_BUF_ACL_IN, timeout);`
Original file line number	Diff line number	Diff line change
`@@ -408,7 +408,7 @@ static inline struct net_buf *get_evt_buf(u8_t evt)`
`408`	`408`	`{`
`409`	`409`	`struct net_buf *buf;`
`410`	`410`
`411`		`- buf = bt_buf_get_evt(evt, K_NO_WAIT);`
	`411`	`+ buf = bt_buf_get_evt(evt, false, K_NO_WAIT);`
`412`	`412`	`if (buf) {`
`413`	`413`	`net_buf_add_u8(h5.rx_buf, evt);`
`414`	`414`	`}`
Original file line number	Diff line number	Diff line change
`@@ -355,7 +355,7 @@ static void bt_spi_rx_thread(void)`
`355`	`355`	`continue;`
`356`	`356`	`default:`
`357`	`357`	`buf = bt_buf_get_evt(rxmsg[EVT_HEADER_EVENT],`
`358`		`- K_FOREVER);`
	`358`	`+ false, K_FOREVER);`
`359`	`359`	`break;`
`360`	`360`	`}`
`361`	`361`
Original file line number	Diff line number	Diff line change
`@@ -58,7 +58,7 @@ static int bt_dev_index = -1;`
`58`	`58`	`static struct net_buf get_rx(const u8_t buf)`
`59`	`59`	`{`
`60`	`60`	`if (buf[0] == H4_EVT) {`
`61`		`- return bt_buf_get_evt(buf[1], K_FOREVER);`
	`61`	`+ return bt_buf_get_evt(buf[1], false, K_FOREVER);`
`62`	`62`	`}`
`63`	`63`
`64`	`64`	`return bt_buf_get_rx(BT_BUF_ACL_IN, K_FOREVER);`