Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200323194507.90944-1-abhishekpandit@chromium.org>
 <20200323124503.v3.1.I17e2220fd0c0822c76a15ef89b882fb4cfe3fe89@changeid> <7FD50BDC-A4B5-4ED9-8DAB-887039735800@holtmann.org>
In-Reply-To: <7FD50BDC-A4B5-4ED9-8DAB-887039735800@holtmann.org>
From:   Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Date:   Fri, 3 Apr 2020 11:10:23 -0700
Message-ID: <CANFp7mX=LvTzttCHcb14TRF8YukQt_WdMpYzJP5LP_ZXwzQTsQ@mail.gmail.com>
Subject: Re: [PATCH v3 1/1] Bluetooth: Prioritize SCO traffic
To:     Marcel Holtmann <marcel@holtmann.org>
Cc:     Bluetooth Kernel Mailing List <linux-bluetooth@vger.kernel.org>,
        ChromeOS Bluetooth Upstreaming 
        <chromeos-bluetooth-upstreaming@chromium.org>,
        "David S. Miller" <davem@davemloft.net>,
        Johan Hedberg <johan.hedberg@gmail.com>,
        netdev <netdev@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Jakub Kicinski <kuba@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Marcel,

Thanks for merging.

I agree that the distinction between SCO/eSCO and ACL/LE is a bit
concerning for scheduling. I will make some time to revisit this as
part of Audio improvements we are making.

Thanks
Abhishek

Abhishek

On Thu, Apr 2, 2020 at 11:56 PM Marcel Holtmann <marcel@holtmann.org> wrote=
:
>
> Hi Abhishek,
>
> > When scheduling TX packets, send all SCO/eSCO packets first, check for
> > pending SCO/eSCO packets after every ACL/LE packet and send them if any
> > are pending.  This is done to make sure that we can meet SCO deadlines
> > on slow interfaces like UART.
> >
> > If we were to queue up multiple ACL packets without checking for a SCO
> > packet, we might miss the SCO timing. For example:
> >
> > The time it takes to send a maximum size ACL packet (1024 bytes):
> > t =3D 10/8 * 1024 bytes * 8 bits/byte * 1 packet / baudrate
> >        where 10/8 is uart overhead due to start/stop bits per byte
> >
> > Replace t =3D 3.75ms (SCO deadline), which gives us a baudrate of 27306=
66.
> >
> > At a baudrate of 3000000, if we didn't check for SCO packets within 102=
4
> > bytes, we would miss the 3.75ms timing window.
> >
> > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > ---
> >
> > Changes in v3:
> > * Removed hci_sched_sync
> >
> > Changes in v2:
> > * Refactor to check for SCO/eSCO after each ACL/LE packet sent
> > * Enabled SCO priority all the time and removed the sched_limit variabl=
e
> >
> > net/bluetooth/hci_core.c | 106 +++++++++++++++++++++------------------
> > 1 file changed, 57 insertions(+), 49 deletions(-)
>
> patch has been applied to bluetooth-next tree.
>
> However I have been a bit reluctant to apply this right away. I think whe=
n this code was originally written, we only had ACL and SCO packets. The wo=
rld was pretty simple. And right now we also only have two packets types (i=
gnoring ISO packets for now), but we added LE and eSCO as separate scheduli=
ng and thus =E2=80=9Cfake=E2=80=9D packet types.
>
> I have the feeling that this serialized packet processing will get us int=
o trouble since we prioritize BR/EDR packets over LE packets and SCO over e=
SCO. I think we should have looked at all packets based on SO_PRIORITY and =
with ISO packets we have to most likely re-design this. Anyway, just someth=
ing to think about.
>
> Regards
>
> Marcel
>