Re: [PATCH v2] Bluetooth: btintel_pcie: Avoid redundant buffer allocation

[Paul Menzel <pmenzel@xxxxxxxxxxxxx>]

Dear Kiran,


Thank you for your patch.


Am 17.04.25 um 05:48 schrieb Kiran K:
> Reuse the skb buffer provided by the PCIe driver to pass it onto the
> stack, instead of copying it to a new skb.

Does `scripts/bloat-o-meter` verify, that the optimization worked?

> Fixes: c2b636b3f788 ("Bluetooth: btintel_pcie: Add support for PCIe transport")
> Signed-off-by: Kiran K <kiran.k@xxxxxxxxx>
> ---
>   drivers/bluetooth/btintel_pcie.c | 33 ++++++++++++--------------------
>   1 file changed, 12 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index e0b50513403f..b73928e38d34 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -947,8 +947,10 @@ static int btintel_pcie_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
>   		/* This is a debug event that comes from IML and OP image when it
>   		 * starts execution. There is no need pass this event to stack.
>   		 */
> -		if (skb->data[2] == 0x97)
> +		if (skb->data[2] == 0x97) {
> +			hci_recv_diag(hdev, skb);
>   			return 0;
> +		}
>   	}
>   
>   	return hci_recv_frame(hdev, skb);
> @@ -964,7 +966,6 @@ static int btintel_pcie_recv_frame(struct btintel_pcie_data *data,
>   	u8 pkt_type;
>   	u16 plen;
>   	u32 pcie_pkt_type;
> -	struct sk_buff *new_skb;
>   	void *pdata;
>   	struct hci_dev *hdev = data->hdev;
>   
> @@ -1041,24 +1042,20 @@ static int btintel_pcie_recv_frame(struct btintel_pcie_data *data,
>   
>   	bt_dev_dbg(hdev, "pkt_type: 0x%2.2x len: %u", pkt_type, plen);
>   
> -	new_skb = bt_skb_alloc(plen, GFP_ATOMIC);
> -	if (!new_skb) {
> -		bt_dev_err(hdev, "Failed to allocate memory for skb of len: %u",
> -			   skb->len);
> -		ret = -ENOMEM;
> -		goto exit_error;
> -	}
> -
> -	hci_skb_pkt_type(new_skb) = pkt_type;
> -	skb_put_data(new_skb, skb->data, plen);
> +	hci_skb_pkt_type(skb) = pkt_type;
>   	hdev->stat.byte_rx += plen;
> +	skb_trim(skb, plen);
>   
>   	if (pcie_pkt_type == BTINTEL_PCIE_HCI_EVT_PKT)
> -		ret = btintel_pcie_recv_event(hdev, new_skb);
> +		ret = btintel_pcie_recv_event(hdev, skb);
>   	else
> -		ret = hci_recv_frame(hdev, new_skb);
> +		ret = hci_recv_frame(hdev, skb);
> +	skb = NULL; /* skb is freed in the callee  */
>   
>   exit_error:
> +	if (skb)
> +		kfree_skb(skb);
> +
>   	if (ret)
>   		hdev->stat.err_rx++;
>   
> @@ -1192,8 +1189,6 @@ static void btintel_pcie_rx_work(struct work_struct *work)
>   	struct btintel_pcie_data *data = container_of(work,
>   					struct btintel_pcie_data, rx_work);
>   	struct sk_buff *skb;
> -	int err;
> -	struct hci_dev *hdev = data->hdev;
>   
>   	if (test_bit(BTINTEL_PCIE_HWEXP_INPROGRESS, &data->flags)) {
>   		/* Unlike usb products, controller will not send hardware
> @@ -1214,11 +1209,7 @@ static void btintel_pcie_rx_work(struct work_struct *work)
>   
>   	/* Process the sk_buf in queue and send to the HCI layer */
>   	while ((skb = skb_dequeue(&data->rx_skb_q))) {
> -		err = btintel_pcie_recv_frame(data, skb);
> -		if (err)
> -			bt_dev_err(hdev, "Failed to send received frame: %d",
> -				   err);
> -		kfree_skb(skb);
> +		btintel_pcie_recv_frame(data, skb);
>   	}
>   }

Reviewed-by: Paul Menzel <pmenzel@xxxxxxxxxxxxx>


Kind regards,

Paul