Hi Gustavo,
Yes, you are right, this is a irq context can't use GFP_KERNEL.
But what if it return error like -NOMEM?
I meet this while debugging hci_ath.c driver.
this the function call chain in hci_ath.c driver when some data receive.
low_level_uart_driver -> hci_ldisc.c:hci_uart_tty_receive() --> ath_recv() --> hci_recv_stream_fragment()
1. If hci_recv_stream_fragment return an error like -NOMEM, the ath_recv() will report a error : Frame Reassembly Failed. but still return the count to hci_uart_tty_receive(). hci_uart_tty_receive() will add this count to his receive statistics.
2. I think it should do some thing like report an error, let upper level know the receive data is got error.
I guess maybe upper level like rfcomm will check the package's checksum ?
This patch is for the #1.
diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c
index bd34406..caf807e 100644
--- a/drivers/bluetooth/hci_ath.c
+++ b/drivers/bluetooth/hci_ath.c
@@ -201,8 +201,12 @@ static struct sk_buff *ath_dequeue(struct hci_uart *hu)
/* Recv data */
static int ath_recv(struct hci_uart *hu, void *data, int count)
{
- if (hci_recv_stream_fragment(hu->hdev, data, count) < 0)
+ int ret;
+ ret = hci_recv_stream_fragment(hu->hdev, data, count);
+ if (ret < 0) {
BT_ERR("Frame Reassembly Failed");
+ return ret;
+ }
return count;
}
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 48ad2a7..320f718 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -359,6 +359,7 @@ static void hci_uart_tty_wakeup(struct tty_struct *tty)
*/
static void hci_uart_tty_receive(struct tty_struct *tty, const u8 *data, char *flags, int count)
{
+ int ret;
struct hci_uart *hu = (void *)tty->disc_data;
if (!hu || tty != hu->tty)
@@ -368,8 +369,9 @@ static void hci_uart_tty_receive(struct tty_struct *tty, const u8 *data, char *f
return;
spin_lock(&hu->rx_lock);
- hu->proto->recv(hu, (void *) data, count);
- hu->hdev->stat.byte_rx += count;
+ ret = hu->proto->recv(hu, (void *) data, count);
+ if (ret > 0)
+ hu->hdev->stat.byte_rx += count;
spin_unlock(&hu->rx_lock);
tty_unthrottle(tty);
________________________________________
From: Gustavo F. Padovan [pao@xxxxxxxxxxxxxx] on behalf of Gustavo F. Padovan [padovan@xxxxxxxxxxxxxx]
Sent: Tuesday, April 05, 2011 5:16 AM
To: Zhang Jiejing-B33651
Cc: suraj@xxxxxxxxxxx; marcel@xxxxxxxxxxxx; linux-bluetooth@xxxxxxxxxxxxxxx
Subject: Re: [PATCH] Bluetooth: change gfp type in hci_recv_stream_fragment()
Hi Zhang,
* Zhang Jiejing <jiejing.zhang@xxxxxxxxxxxxx> [2011-03-31 14:56:56 +0800]:
> change gfp type passed to hci_reassembly(), replace GFP_ATOMIC
> to GFP_KERNEL. Since some HCI_ACLDATA may request 1024+4 bytes
> some time GFP_ATOMIC will failed to allocation memory during
> large file FTP transfer in high baud rate.
>
> Signed-off-by: Zhang Jiejing <jiejing.zhang@xxxxxxxxxxxxx>
> ---
> net/bluetooth/hci_core.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 9c4541b..22b3ded 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1214,7 +1214,7 @@ int hci_recv_stream_fragment(struct hci_dev *hdev, void *data, int count)
> type = bt_cb(skb)->pkt_type;
>
> rem = hci_reassembly(hdev, type, data,
> - count, STREAM_REASSEMBLY, GFP_ATOMIC);
> + count, STREAM_REASSEMBLY, GFP_KERNEL);
We can't use GFP_KERNEL in a interrupt context, we can't sleep here and
GFP_ATOMIC guarantees that.
--
Gustavo F. Padovan
http://profusion.mobi
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
