Problematic BT commit in Linux 5.7

Problematic BT commit in Linux 5.7

[Rafael J. Wysocki]

Hi Marcel,

Unfortunately, we are observing system suspend failures on multiple
lab machines as reported in the BZ entry at

https://bugzilla.kernel.org/show_bug.cgi?id=207629

which is due to the following commit:

commit dd522a7429b07e4441871ae75ebbfcf53635bdd4
Author: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Date:   Wed Mar 11 08:54:02 2020 -0700

    Bluetooth: Handle LE devices during suspend

Ostensibly, this is because the BT firmware on the machines in
question does not match the new kernel code, but the firmware update
requirement is not entirely obvious to the users and the steps to take
in order to upgrade the firmware are not clear.

Apart from the above, failing system suspend for a reason like a
protocol timeout isn't really user-friendly, because the user may just
have closed the lid of a laptop and is expecting the system to be
suspended (so it may go into a backpack or similar).  Yes, the driver
may not be able to suspend its device gracefully, but failing the
entire system suspend really is a big deal and should be treated as a
last-resort type of action.

As stated in the BZ above, reverting the above commit along with
"Bluetooth: Pause discovery and advertising during suspend"
(4867bd007d25a8dfd4ffc558534f7aec8b361789) makes the issue go away, so
can you please consider doing that?

Alternatively, would it be possible to address the issue in a way that
would not require many users to update firmware on their systems?

Cheers,
Rafael

Re: Problematic BT commit in Linux 5.7

[Abhishek Pandit-Subedi]

Hi Rafael,

I left a comment on the bugzilla post as well but I agree with you now
that it's probably bad to fail suspend when this occurs.

At https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/net/bluetooth/hci_core.c#n3363,
we try to recover from a failed suspend by restoring running state and
returning an -EBUSY.
I think if we change this logic to not attempt to restore until
resume, log the error and allow suspend, that would probably be
preferable from a suspend perspective.
I will make this change, test it out and send the patch today.

Thanks
Abhishek

On Wed, Jun 3, 2020 at 9:16 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> Hi Marcel,
>
> Unfortunately, we are observing system suspend failures on multiple
> lab machines as reported in the BZ entry at
>
> https://bugzilla.kernel.org/show_bug.cgi?id=207629
>
> which is due to the following commit:
>
> commit dd522a7429b07e4441871ae75ebbfcf53635bdd4
> Author: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> Date:   Wed Mar 11 08:54:02 2020 -0700
>
>     Bluetooth: Handle LE devices during suspend
>
> Ostensibly, this is because the BT firmware on the machines in
> question does not match the new kernel code, but the firmware update
> requirement is not entirely obvious to the users and the steps to take
> in order to upgrade the firmware are not clear.
>
> Apart from the above, failing system suspend for a reason like a
> protocol timeout isn't really user-friendly, because the user may just
> have closed the lid of a laptop and is expecting the system to be
> suspended (so it may go into a backpack or similar).  Yes, the driver
> may not be able to suspend its device gracefully, but failing the
> entire system suspend really is a big deal and should be treated as a
> last-resort type of action.
>
> As stated in the BZ above, reverting the above commit along with
> "Bluetooth: Pause discovery and advertising during suspend"
> (4867bd007d25a8dfd4ffc558534f7aec8b361789) makes the issue go away, so
> can you please consider doing that?
>
> Alternatively, would it be possible to address the issue in a way that
> would not require many users to update firmware on their systems?
>
> Cheers,
> Rafael

Re: Problematic BT commit in Linux 5.7

[Rafael J. Wysocki]

Hi Abhishek,

On Wed, Jun 3, 2020 at 6:29 PM Abhishek Pandit-Subedi
<abhishekpandit@chromium.org> wrote:
>
> Hi Rafael,
>
> I left a comment on the bugzilla post as well but I agree with you now
> that it's probably bad to fail suspend when this occurs.
>
> At https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/net/bluetooth/hci_core.c#n3363,
> we try to recover from a failed suspend by restoring running state and
> returning an -EBUSY.
> I think if we change this logic to not attempt to restore until
> resume, log the error and allow suspend, that would probably be
> preferable from a suspend perspective.
> I will make this change, test it out and send the patch today.

Thank you!

Please CC linux-pm as well as Len Brown and Todd Brandt (both in the
CC list of this message) on that patch.

Thanks!


> On Wed, Jun 3, 2020 at 9:16 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > Hi Marcel,
> >
> > Unfortunately, we are observing system suspend failures on multiple
> > lab machines as reported in the BZ entry at
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=207629
> >
> > which is due to the following commit:
> >
> > commit dd522a7429b07e4441871ae75ebbfcf53635bdd4
> > Author: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > Date:   Wed Mar 11 08:54:02 2020 -0700
> >
> >     Bluetooth: Handle LE devices during suspend
> >
> > Ostensibly, this is because the BT firmware on the machines in
> > question does not match the new kernel code, but the firmware update
> > requirement is not entirely obvious to the users and the steps to take
> > in order to upgrade the firmware are not clear.
> >
> > Apart from the above, failing system suspend for a reason like a
> > protocol timeout isn't really user-friendly, because the user may just
> > have closed the lid of a laptop and is expecting the system to be
> > suspended (so it may go into a backpack or similar).  Yes, the driver
> > may not be able to suspend its device gracefully, but failing the
> > entire system suspend really is a big deal and should be treated as a
> > last-resort type of action.
> >
> > As stated in the BZ above, reverting the above commit along with
> > "Bluetooth: Pause discovery and advertising during suspend"
> > (4867bd007d25a8dfd4ffc558534f7aec8b361789) makes the issue go away, so
> > can you please consider doing that?
> >
> > Alternatively, would it be possible to address the issue in a way that
> > would not require many users to update firmware on their systems?
> >
> > Cheers,
> > Rafael

Re: Problematic BT commit in Linux 5.7

[Abhishek Pandit-Subedi]

I've sent a patch to fix this titled: [PATCH] Bluetooth: Allow suspend
even when preparation has failed
https://patchwork.kernel.org/patch/11586223/

Please take a look.

Thanks
Abhishek

On Wed, Jun 3, 2020 at 9:32 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> Hi Abhishek,
>
> On Wed, Jun 3, 2020 at 6:29 PM Abhishek Pandit-Subedi
> <abhishekpandit@chromium.org> wrote:
> >
> > Hi Rafael,
> >
> > I left a comment on the bugzilla post as well but I agree with you now
> > that it's probably bad to fail suspend when this occurs.
> >
> > At https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/net/bluetooth/hci_core.c#n3363,
> > we try to recover from a failed suspend by restoring running state and
> > returning an -EBUSY.
> > I think if we change this logic to not attempt to restore until
> > resume, log the error and allow suspend, that would probably be
> > preferable from a suspend perspective.
> > I will make this change, test it out and send the patch today.
>
> Thank you!
>
> Please CC linux-pm as well as Len Brown and Todd Brandt (both in the
> CC list of this message) on that patch.
>
> Thanks!
>
>
> > On Wed, Jun 3, 2020 at 9:16 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
> > >
> > > Hi Marcel,
> > >
> > > Unfortunately, we are observing system suspend failures on multiple
> > > lab machines as reported in the BZ entry at
> > >
> > > https://bugzilla.kernel.org/show_bug.cgi?id=207629
> > >
> > > which is due to the following commit:
> > >
> > > commit dd522a7429b07e4441871ae75ebbfcf53635bdd4
> > > Author: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > > Date:   Wed Mar 11 08:54:02 2020 -0700
> > >
> > >     Bluetooth: Handle LE devices during suspend
> > >
> > > Ostensibly, this is because the BT firmware on the machines in
> > > question does not match the new kernel code, but the firmware update
> > > requirement is not entirely obvious to the users and the steps to take
> > > in order to upgrade the firmware are not clear.
> > >
> > > Apart from the above, failing system suspend for a reason like a
> > > protocol timeout isn't really user-friendly, because the user may just
> > > have closed the lid of a laptop and is expecting the system to be
> > > suspended (so it may go into a backpack or similar).  Yes, the driver
> > > may not be able to suspend its device gracefully, but failing the
> > > entire system suspend really is a big deal and should be treated as a
> > > last-resort type of action.
> > >
> > > As stated in the BZ above, reverting the above commit along with
> > > "Bluetooth: Pause discovery and advertising during suspend"
> > > (4867bd007d25a8dfd4ffc558534f7aec8b361789) makes the issue go away, so
> > > can you please consider doing that?
> > >
> > > Alternatively, would it be possible to address the issue in a way that
> > > would not require many users to update firmware on their systems?
> > >
> > > Cheers,
> > > Rafael