Re: [PATCH 4.4 009/115] Bluetooth: btusb: driver to enable the usb-wakeup feature

[Guenter Roeck]

On Thu, Dec 21, 2017 at 09:20:51AM -0800, Brian Norris wrote:
> On Thu, Dec 21, 2017 at 12:30 AM, gregkh@xxxxxxxxxxxxxxxxxxx
> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > As Linus's tree is also broken, being bug-compatible here is good,
> > right?  I can just apply the revert/fix patch when it lands in that
> > tree, and all will be ok.
> >
> > Or is Linus's tree not broken now?  Sorry, this whole thread has been
> > really confusing...
> 
> Linus' tree is broken in 2 ways:
> 
> 1. It includes this patch:
> "Bluetooth: btusb: fix QCA Rome suspend/resume"
> which is wrong, and we're on our way to reverting it upstream and
> backporting that to -stable.
> 
> 2. It includes $subject patch. I'm not quite so sure, but I believe
> it's not 100% "wrong"; it's just tougher for user space to deal with,
> since now by default, all sorts of BT devices are set to be wakeup
> sources. We can account for that in user space by being more careful
> with initiating BT activity before suspend, but we don't currently do
> that (at least not with the BlueZ in ChromeOS).
> A related portion of this problem is that we briefly thought that this
> patch resolved regressions with 1. In the end, it might mask some, but
> it does not actually fix the problem. But then, you only included this
> patch because somebody suggested it could resolve 1...
> 
> In the end, I'd say that #2 never belonged in -stable at all, and #1
> was just "buggy" (so we're reverting it and letting the revert trickle
> into -stable). I'm not sure I have a strong reason to revert #2
> upstream, but I think I have an argument for not including it in
> -stable.
> 
> I'm not sure what you mean by "bug-compatible"; if I wanted to use a
> buggy kernel, I'd use Linus' tree :)
> 
> Or as I think I understand your point: the key point is that #2 might
> not be actually a "bug", but a feature that user space has to be more
> careful with. That may be a candidate for mainline, but not for
> -stable, as I understand the current rules.
> 

I absolutely agree with Brian here. #2, or commit a0085f2510e8 ("Bluetooth:
btusb: driver to enable the usb-wakeup feature") does not fix a bug, was
not tagged for stable, and should not have been applied to stable releases
in the first place. That it has a potential to cause trouble is not even
relevant here.

I am personally not sure what to do with #2 in -stable at this point.
We reverted it from the merge of v4.4.107 into chromeos-4.4, so it does
not immediately affect us. I'll leave it up to Greg to decide what to do
with it in v4.4.y.

I am much more concerned how to avoid such problems going forward, as it
does negatively affect the value of stable releases. How can we prevent
patches like this from ending up in -stable in the first place ?

Regarding "being bug-compatible here is good" - maybe I am missing some
context but, no, not at all for stable releases. Quite he opposite,
actually. If stable wants to be bug-compatible with upstream, there
would be no reason to have stable releases in the first place.

Thanks,
Guenter