On Sun, 12 Apr 2020 10:10:22 +0300 Or Gerlitz wrote:

On Sun, Apr 12, 2020 at 2:16 AM Sasha Levin sashal@kernel.org wrote:

...

[ Upstream commit 6783e8b29f636383af293a55336f036bc7ad5619 ]

Sasha,

This was pushed to net-next without a fixes tag, and there're probably reasons for that. As you can see the possible null deref is not even reproducible without another patch which for itself was also net-next and not net one.

If a team is not pushing patch to net nor putting a fixes that, I don't think it's correct to go and pick that into stable and from there to customer production kernels.

Alsom, I am not sure what's the idea behind the auto-selection concept, e.g for mlx5 the maintainer is specifically pointing which patches should go to stable and to what releases there and this is done with care and thinking ahead, why do we want to add on that? and why this can be something which is just automatic selection?

We have customers running production system with LTS 4.4.x and 4.9.y (along with 4.14.z and 4.19.w) kernels, we put lots of care thinking if/what should go there, I don't see a benefit from adding auto-selection, the converse.

FWIW I had the same thoughts about the nfp driver, and I indicated to Sasha to skip it in the auto selection, which AFAICT worked nicely.

Maybe we should communicate more clearly that maintainers who carefully select patches for stable should opt out of auto-selection?

On Sun, Apr 12, 2020 at 10:59:35AM -0700, Jakub Kicinski wrote:

On Sun, 12 Apr 2020 10:10:22 +0300 Or Gerlitz wrote:

...

On Sun, Apr 12, 2020 at 2:16 AM Sasha Levin sashal@kernel.org wrote:

...

[ Upstream commit 6783e8b29f636383af293a55336f036bc7ad5619 ]

Sasha,

This was pushed to net-next without a fixes tag, and there're probably reasons for that. As you can see the possible null deref is not even reproducible without another patch which for itself was also net-next and not net one.

If a team is not pushing patch to net nor putting a fixes that, I don't think it's correct

While it's great that you're putting the effort into adding a fixes tag to your commits, I'm not sure what a fixes tag has to do with inclusion in a stable tree.

It's a great help when we look into queueing something up, but on it's own it doesn't imply anything.

...

to go and pick that into stable and from there to customer production kernels.

This mail is your two week warning that this patch might get queued to stable, nothing was actually queued just yet.

...

Alsom, I am not sure what's the idea behind the auto-selection concept, e.g for mlx5 the maintainer is specifically pointing which patches should go to stable and

I'm curious, how does this process work? Is it on a mailing list somewhere?

...

to what releases there and this is done with care and thinking ahead, why do we want to add on that? and why this can be something which is just automatic selection?

We have customers running production system with LTS 4.4.x and 4.9.y (along with 4.14.z and 4.19.w) kernels, we put lots of care thinking if/what should go there, I don't see a benefit from adding auto-selection, the converse.

FWIW I had the same thoughts about the nfp driver, and I indicated to Sasha to skip it in the auto selection, which AFAICT worked nicely.

Maybe we should communicate more clearly that maintainers who carefully select patches for stable should opt out of auto-selection?

I've added drivers/net/ethernet/mellanox/ to my blacklist for auto selection. It's very easy to opt out, just ask... I've never argued with anyone around this - the maintainers of any given subsystem know about it way better than me.

On Tue, Apr 14, 2020 at 4:56 AM Sasha Levin sashal@kernel.org wrote:

On Sun, Apr 12, 2020 at 10:59:35AM -0700, Jakub Kicinski wrote:

...

On Sun, 12 Apr 2020 10:10:22 +0300 Or Gerlitz wrote:

...

On Sun, Apr 12, 2020 at 2:16 AM Sasha Levin sashal@kernel.org wrote:

...

[ Upstream commit 6783e8b29f636383af293a55336f036bc7ad5619 ]

Sasha,

This was pushed to net-next without a fixes tag, and there're probably reasons for that. As you can see the possible null deref is not even reproducible without another patch which for itself was also net-next and not net one.

If a team is not pushing patch to net nor putting a fixes that, I don't think it's correct

While it's great that you're putting the effort into adding a fixes tag to your commits, I'm not sure what a fixes tag has to do with inclusion in a stable tree.

It's a great help when we look into queueing something up, but on it's own it doesn't imply anything.

...

...

to go and pick that into stable and from there to customer production kernels.

This mail is your two week warning that this patch might get queued to stable, nothing was actually queued just yet.

...

...

Alsom, I am not sure what's the idea behind the auto-selection concept, e.g for mlx5 the maintainer is specifically pointing which patches should go to stable and

I'm curious, how does this process work? Is it on a mailing list somewhere?

...

...

to what releases there and this is done with care and thinking ahead, why do we want to add on that? and why this can be something which is just automatic selection?

We have customers running production system with LTS 4.4.x and 4.9.y (along with 4.14.z and 4.19.w) kernels, we put lots of care thinking if/what should go there, I don't see a benefit from adding auto-selection, the converse.

FWIW I had the same thoughts about the nfp driver, and I indicated to Sasha to skip it in the auto selection, which AFAICT worked nicely.

Maybe we should communicate more clearly that maintainers who carefully select patches for stable should opt out of auto-selection?

I've added drivers/net/ethernet/mellanox/ to my blacklist for auto selection. It's very easy to opt out, just ask... I've never argued with anyone around this - the maintainers of any given subsystem know about it way better than me.

Just to make sure, does this excluding of mlx5 happens immediately, that is, applies also to all non committed patches that you already posted?

IMHO - I think it should be the other way around, you should get approval from sub-system maintainers to put their code in charge into auto-selection, unless there's kernel summit decision that says otherwise, is this documented anywhere?

On Tue, Apr 14, 2020 at 2:09 PM Greg KH gregkh@linuxfoundation.org wrote:

On Tue, Apr 14, 2020 at 01:22:59PM +0300, Or Gerlitz wrote:

...

IMHO - I think it should be the other way around, you should get approval from sub-system maintainers to put their code in charge into auto-selection, unless there's kernel summit decision that says otherwise, is this documented anywhere?

No, we can't get make this a "only take if I agree" as there are _many_ subsystem maintainers who today never mark anything for stable trees, as they just can't be bothered. And that's fine, stable trees should not take up any extra maintainer time if they do not want to do so. So it's simpler to do an opt-out when asked for.

OK, but I must say I am worried from the comment made here:

"I'm not sure what a fixes tag has to do with inclusion in a stable tree"

This patch

(A) was pushed to -next and not -rc kernel

(B) doesn't have fixes tag

(C) the change log state clearly that what's being "fixed" can't be reproduced on any earlier kernel [..] "only possible to reproduce with next commit in this series"

but it was selected for -stable -- at least if the fixes tag was used as gating criteria, this wrong stable inclusion could have been eliminated

On 14/04/2020 16:16, Sasha Levin wrote:

Are you suggesting that a commit without a fixes tag is never a fix?

Because fixes are much more likely than non-fixes to have a Fixes tag, the absence of a fixes tag is Bayesian evidence that a commit is not a fix. It's of course not incontrovertible evidence, since (as you note) some fixes do not have a Fixes tag, but it does increase the amount of countervailing evidence needed to conclude a commit is a fix. In this case it looks as if the only such evidence was that the commit message included the phrase "NULL pointer dereference".

Fixes can (and should) come in during a merge window as well. They are not put on hold until the -rc releases.

In networking-land, fixes generally go through David's 'net' tree, rather than 'net-next'; the only times a fix goes to net-next are when a) the code it's fixing is only in net-next; i.e. it's a fix to a previous patch from the same merge window. In this case the fix should not be backported, since the code it's fixing will not appear in stable kernels. b) the code has changed enough between net and net-next that different fixes are appropriate for the two trees. In this case, only the fix that went to 'net' should be backported (since it's the one that's appropriate for net, it's probably more appropriate for stable trees too); the fix that went to 'net-next' should not. Or's original phrasing was that this patch "was pushed to net-next", which is not quite exactly the same thing as -next vs. -rc (though it's similar because of David's system of closing net-next for the duration of the merge window). And this, again, is quite strong Bayesian evidence that the patch should not be selected for stable.

To be honest, that this needs to be explained to you does not inspire confidence in the quality of your autoselection process...

GregKH wrote:

we can't get make this a "only take if I agree" as there are _many_ subsystem maintainers who today never mark anything for stable trees

The complaint seems to be that Sasha's threshold for "this looks stable- worthy" is too low; maybe setting it that low should be opt-in, while patches for subsystems that haven't opted in should need to be "obviously" fixes for them to get autoselected, on the grounds that having to watch the autosel messages and pounce on the ones to which the answer is "no no no, applying that to stable kernels will break them horribly" is _also_ extra work for a maintainer. (Especially if, a month after NACKing them, the same patch shows up again in another autosel batch, which — unless my memory is playing tricks on me — has happened to me at least once.)

-ed

On Tue, 14 Apr 2020 20:37:18 +0300 Leon Romanovsky wrote:

On Tue, Apr 14, 2020 at 04:49:20PM +0100, Edward Cree wrote:

...

On 14/04/2020 16:16, Sasha Levin wrote:

...

Are you suggesting that a commit without a fixes tag is never a fix?

Because fixes are much more likely than non-fixes to have a Fixes tag, the absence of a fixes tag is Bayesian evidence that a commit is not a fix. It's of course not incontrovertible evidence, since (as you note) some fixes do not have a Fixes tag, but it does increase the amount of countervailing evidence needed to conclude a commit is a fix. In this case it looks as if the only such evidence was that the commit message included the phrase "NULL pointer dereference".

...

Fixes can (and should) come in during a merge window as well. They are not put on hold until the -rc releases.

In networking-land, fixes generally go through David's 'net' tree, rather than 'net-next'; the only times a fix goes to net-next are when a) the code it's fixing is only in net-next; i.e. it's a fix to a previous patch from the same merge window. In this case the fix should not be backported, since the code it's fixing will not appear in stable kernels. b) the code has changed enough between net and net-next that different fixes are appropriate for the two trees. In this case, only the fix that went to 'net' should be backported (since it's the one that's appropriate for net, it's probably more appropriate for stable trees too); the fix that went to 'net-next' should not. Or's original phrasing was that this patch "was pushed to net-next", which is not quite exactly the same thing as -next vs. -rc (though it's similar because of David's system of closing net-next for the duration of the merge window). And this, again, is quite strong Bayesian evidence that the patch should not be selected for stable.

To be honest, that this needs to be explained to you does not inspire confidence in the quality of your autoselection process...

It is a little bit harsh to say that.

The autoselection process works good enough for everything outside of netdev community. The amount of bugs in those stable@ trees is not such high if you take into account the amount of fixes automatically brought in.

I think that all Fedora users are indirectly use those stable@ trees.

+1

I think folks how mark things for stable explicitly and carefully have an obvious bias because they only see the false positives of auto-sel and never the benefits.

On Tue, Apr 14, 2020 at 08:37:18PM +0300, Leon Romanovsky wrote:

The autoselection process works good enough for everything outside of netdev community.

That's very far from true. I have seen and heard many complaints about AUTOSEL and inflation of stable trees in general, both in private and in public lists. It was also discussed on Kernel Summit few times - with little success.

Just for fun, I suggest everyone to read first section of Documentation/process/stable-kernel-rules.rst and compare with today's reality. Of course, rules can change over time but keeping that document in kernel tree as a memento is rather sad - I went through the rules now and there are only three which are not broken on a regular basis these days.

Michal Kubecek

On Wed, Apr 15, 2020 at 12:50:09AM +0200, Michal Kubecek wrote:

On Tue, Apr 14, 2020 at 08:37:18PM +0300, Leon Romanovsky wrote:

...

The autoselection process works good enough for everything outside of netdev community.

That's very far from true. I have seen and heard many complaints about

How about some actionable data? What do you expect us to do with "seen and heard"?

Do you have evidence that AUTOSEL introduces more regressions over the stable tagged commits? Do you have evidence showing that AUTOSEL isn't performing properly? Great, let's go over it.

I have always measured the results of this work and compared it to the "regular" flow of stable tagged commits, I'm keeping track of how the quality of it compares to those stable tagged commits, and quite a few times I took action to deal with specific issues that come up with the process based on data.

There's not much I can do with "seen and heard".

AUTOSEL and inflation of stable trees in general, both in private and in

I'm puzzled about your objections to "inflation"? The stable tree was missing a bunch of fixes before, and now it doesn't.

In parallel to "inflating" the stable trees I'm also driving efforts to improve the testing of these trees so that we could reliably say that we didn't introduce any regressions into the stable tree. "inflation" on it's own doesn't increase the rate of regressions in stable trees, and that's based on historical data.

public lists. It was also discussed on Kernel Summit few times - with little success.

Define success? We're adding a bunch of missing fixes and not causing any more regressions than before, you don't call that a success?

Let me also point out that I've tried to bring up this process (as well as the -stable process in general) at every kernel summit/maintainer's summit (here's the one from last year: https://lwn.net/Articles/799166/) and attempted to address any feedback that came back from it. It seemed to me that there's an agreement that this process was going well (at the very least Linus didn't yell at me), so I'm still puzzled by your own success criteria.

Firstly, let me apologise: my previous email was too harsh and too  assertiveabout things that were really more uncertain and unclear.

On 14/04/2020 21:57, Sasha Levin wrote:

I've pointed out that almost 50% of commits tagged for stable do not have a fixes tag, and yet they are fixes. You really deduce things based on coin flip probability?

Yes, but far less than 50% of commits *not* tagged for stable have a fixes  tag.  It's not about hard-and-fast Aristotelian "deductions", like "this  doesn't have Fixes:, therefore it is not a stable candidate", it's about  probabilistic "induction".

"it does increase the amount of countervailing evidence needed to conclude a commit is a fix" - Please explain this argument given the above.

Are you familiar with Bayesian statistics?  If not, I'd suggest reading  something like http://yudkowsky.net/rational/bayes/ which explains it. There's a big difference between a coin flip and a _correlated_ coin flip.

This is great, but the kernel is more than just net/. Note that I also do not look at net/ itself, but rather drivers/net/ as those end up with a bunch of missed fixes.

drivers/net/ goes through the same DaveM net/net-next trees, with the  same rules.

...

To be honest, that this needs to be explained to you does not inspire confidence in the quality of your autoselection process...

Nothing like a personal attack or two to try and make a point?

It wasn't meant as a personal attack, more as an "it's worrying that this  is not known to the people doing the stable selection".  But I did agonise  over whether to say that and now wish I hadn't; sorry.  (I'm not exactly  my best self right now, what with all the lockdown cabin fever.)

-ed

On Wed, 2020-04-15 at 20:00 -0400, Sasha Levin wrote:

On Wed, Apr 15, 2020 at 05:18:38PM +0100, Edward Cree wrote:

...

Firstly, let me apologise: my previous email was too harsh and too assertiveabout things that were really more uncertain and unclear.

On 14/04/2020 21:57, Sasha Levin wrote:

...

I've pointed out that almost 50% of commits tagged for stable do not have a fixes tag, and yet they are fixes. You really deduce things based on coin flip probability?

Yes, but far less than 50% of commits *not* tagged for stable have a fixes tag. It's not about hard-and-fast Aristotelian "deductions", like "this doesn't have Fixes:, therefore it is not a stable candidate", it's about probabilistic "induction".

...

"it does increase the amount of countervailing evidence needed to conclude a commit is a fix" - Please explain this argument given the above.

Are you familiar with Bayesian statistics? If not, I'd suggest reading something like http://yudkowsky.net/rational/bayes/ which explains it. There's a big difference between a coin flip and a _correlated_ coin flip.

I'd maybe point out that the selection process is based on a neural network which knows about the existence of a Fixes tag in a commit.

It does exactly what you're describing, but also taking a bunch more factors into it's desicion process ("panic"? "oops"? "overflow"? etc).

I am not against AUTOSEL in general, as long as the decision to know how far back it is allowed to take a patch is made deterministically and not statistically based on some AI hunch.

Any auto selection for a patch without a Fixes tags can be catastrophic .. imagine a patch without a Fixes Tag with a single line that is fixing some "oops", such patch can be easily applied cleanly to stable- v.x and stable-v.y .. while it fixes the issue on v.x it might have catastrophic results on v.y ..

if you want these factors to keep playing a role in the autosel process, then a human factor or some deterministic testing/code coverage step must take action before backporting such patch on the blind.

What i would suggest here: For patches that are missing a Fixes tag, they should be considered as a "candidate" for autosel, and don't actually apply them until an explicit ACK from some human/regression test factor is received.

...

...

This is great, but the kernel is more than just net/. Note that I also do not look at net/ itself, but rather drivers/net/ as those end up with a bunch of missed fixes.

drivers/net/ goes through the same DaveM net/net-next trees, with the same rules.

Let me put my Microsoft employee hat on here. We have driver/net/hyperv/ which definitely wasn't getting all the fixes it should have been getting without AUTOSEL.

until some patch which shouldn't get backported slips through, believe me this will happen, just give it some time ..

While net/ is doing great, drivers/net/ is not. If it's indeed following the same rules then we need to talk about how we get done right.

both net and drivers/net are managed by the same maitainer and follow the same rules, can you elaborate on the difference ?

I really have no objection to not looking in drivers/net/, it's just that the experience I had with the process suggests that it's not following the same process as net/.

On Thu, Apr 16, 2020 at 08:24:09AM +0300, Leon Romanovsky wrote:

On Thu, Apr 16, 2020 at 04:08:10AM +0000, Saeed Mahameed wrote:

...

On Wed, 2020-04-15 at 20:00 -0400, Sasha Levin wrote:

...

On Wed, Apr 15, 2020 at 05:18:38PM +0100, Edward Cree wrote:

...

Firstly, let me apologise: my previous email was too harsh and too assertiveabout things that were really more uncertain and unclear.

On 14/04/2020 21:57, Sasha Levin wrote:

...

I've pointed out that almost 50% of commits tagged for stable do not have a fixes tag, and yet they are fixes. You really deduce things based on coin flip probability?

Yes, but far less than 50% of commits *not* tagged for stable have a fixes tag. It's not about hard-and-fast Aristotelian "deductions", like "this doesn't have Fixes:, therefore it is not a stable candidate", it's about probabilistic "induction".

...

"it does increase the amount of countervailing evidence needed to conclude a commit is a fix" - Please explain this argument given the above.

Are you familiar with Bayesian statistics? If not, I'd suggest reading something like http://yudkowsky.net/rational/bayes/ which explains it. There's a big difference between a coin flip and a _correlated_ coin flip.

I'd maybe point out that the selection process is based on a neural network which knows about the existence of a Fixes tag in a commit.

It does exactly what you're describing, but also taking a bunch more factors into it's desicion process ("panic"? "oops"? "overflow"? etc).

I am not against AUTOSEL in general, as long as the decision to know how far back it is allowed to take a patch is made deterministically and not statistically based on some AI hunch.

Any auto selection for a patch without a Fixes tags can be catastrophic .. imagine a patch without a Fixes Tag with a single line that is fixing some "oops", such patch can be easily applied cleanly to stable- v.x and stable-v.y .. while it fixes the issue on v.x it might have catastrophic results on v.y ..

I tried to imagine such flow and failed to do so. Are you talking about anything specific or imaginary case?

It happens, rarely, but it does. However, all the cases I can think of happened with a stable tagged commit without a fixes where it's backport to an older tree caused unintended behavior (local denial of service in one case).

The scenario you have in mind is true for both stable and non-stable tagged patches, so it you want to restrict how we deal with commits that don't have a fixes tag shouldn't it be true for *all* commits?

<...>

...

...

Let me put my Microsoft employee hat on here. We have driver/net/hyperv/ which definitely wasn't getting all the fixes it should have been getting without AUTOSEL.

until some patch which shouldn't get backported slips through, believe me this will happen, just give it some time ..

Bugs are inevitable, I don't see many differences between bugs introduced by manually cherry-picking or automatically one.

Oh bugs slip in, that's why I track how many bugs slipped via stable tagged commits vs non-stable tagged ones, and the statistic may surprise you.

The solution here is to beef up your testing infrastructure rather than taking less patches; we still want to have *all* the fixes, right?

Of course, it is true if this automatically cherry-picking works as expected and evolving.

...

...

While net/ is doing great, drivers/net/ is not. If it's indeed following the same rules then we need to talk about how we get done right.

both net and drivers/net are managed by the same maitainer and follow the same rules, can you elaborate on the difference ?

The main reason is a difference in a volume between net and drivers/net. While net/* patches are watched by many eyes and carefully selected to be ported to stable@, most of the drivers/net patches are not.

Except 3-5 the most active drivers, rest of the driver patches almost never asked to be backported.

Right, that's exactly my point: If you're not Mellanox, e1000*, etc you won't see it, but the smaller drivers aren't getting the same handling as the big ones.

I think that we all love the work DaveM does with net/ - it makes our lives a lot easier, and if the same thing would happen with drivers/net/ I'll happily go away and never AUTOSEL a *net* commit, but looking at how our Hyper-V drivers look like it's clearly not there yet.

On Thu, Apr 16, 2020 at 07:07:13PM +0000, Saeed Mahameed wrote:

On Thu, 2020-04-16 at 09:30 -0400, Sasha Levin wrote:

...

On Thu, Apr 16, 2020 at 08:24:09AM +0300, Leon Romanovsky wrote:

...

On Thu, Apr 16, 2020 at 04:08:10AM +0000, Saeed Mahameed wrote:

...

On Wed, 2020-04-15 at 20:00 -0400, Sasha Levin wrote:

...

On Wed, Apr 15, 2020 at 05:18:38PM +0100, Edward Cree wrote:

...

Firstly, let me apologise: my previous email was too harsh and too assertiveabout things that were really more uncertain and unclear.

On 14/04/2020 21:57, Sasha Levin wrote: > I've pointed out that almost 50% of commits tagged for > stable do > not > have a fixes tag, and yet they are fixes. You really deduce > things based > on coin flip probability? Yes, but far less than 50% of commits *not* tagged for stable have a fixes tag. It's not about hard-and-fast Aristotelian "deductions", like "this doesn't have Fixes:, therefore it is not a stable candidate", it's about probabilistic "induction".

> "it does increase the amount of countervailing evidence > needed to > conclude a commit is a fix" - Please explain this argument > given > the > above. Are you familiar with Bayesian statistics? If not, I'd suggest reading something like http://yudkowsky.net/rational/bayes/ which explains it. There's a big difference between a coin flip and a _correlated_ coin flip.

I'd maybe point out that the selection process is based on a neural network which knows about the existence of a Fixes tag in a commit.

It does exactly what you're describing, but also taking a bunch more factors into it's desicion process ("panic"? "oops"? "overflow"? etc).

I am not against AUTOSEL in general, as long as the decision to know how far back it is allowed to take a patch is made deterministically and not statistically based on some AI hunch.

Any auto selection for a patch without a Fixes tags can be catastrophic .. imagine a patch without a Fixes Tag with a single line that is fixing some "oops", such patch can be easily applied cleanly to stable- v.x and stable-v.y .. while it fixes the issue on v.x it might have catastrophic results on v.y ..

I tried to imagine such flow and failed to do so. Are you talking about anything specific or imaginary case?

It happens, rarely, but it does. However, all the cases I can think of happened with a stable tagged commit without a fixes where it's backport to an older tree caused unintended behavior (local denial of service in one case).

The scenario you have in mind is true for both stable and non-stable tagged patches, so it you want to restrict how we deal with commits that don't have a fixes tag shouldn't it be true for *all* commits?

All commits? even the ones without "oops" in them ? where does this stop ? :) We _must_ have a hard and deterministic cut for how far back to take a patch based on a human decision.. unless we are 100% positive autoselection AI can never make a mistake.

Humans are allowed to make mistakes, AI is not.

Oh I'm reviewing all patches myself after the bot does it's selection, you can blame me for these screw ups.

If a Fixes tag is wrong, then a human will be blamed, and that is perfectly fine, but if we have some statistical model that we know it is going to be wrong 0.001% of the time.. and we still let it run.. then something needs to be done about this.

I know there are benefits to autosel, but overtime, if this is not being audited, many pieces of the kernel will get broken unnoticed until some poor distro decides to upgrade their kernel version.

Quite a few distros are always running on the latest LTS releases, Android isn't that far behind either at this point.

There are actually very few non-LTS users at this point...

...

...

<...>

...

...

Let me put my Microsoft employee hat on here. We have driver/net/hyperv/ which definitely wasn't getting all the fixes it should have been getting without AUTOSEL.

until some patch which shouldn't get backported slips through, believe me this will happen, just give it some time ..

Bugs are inevitable, I don't see many differences between bugs introduced by manually cherry-picking or automatically one.

Oh bugs slip in, that's why I track how many bugs slipped via stable tagged commits vs non-stable tagged ones, and the statistic may surprise you.

Statistics do not matter here, what really matters is that there is a possibility of a non-human induced error, this should be a no no. or at least make it an opt-in thing for those who want to take their chances and keep a close eye on it..

Hrm, why? Pretend that the bot is a human sitting somewhere sending mails out, how does it change anything?

...

The solution here is to beef up your testing infrastructure rather than

So please let me opt-in until I beef up my testing infra.

Already did :)

...

taking less patches; we still want to have *all* the fixes, right?

if you can be sure 100% it is the right thing to do, then yes, please don't hesitate to take that patch, even without asking anyone !!

Again, Humans are allowed to make mistakes.. AI is not.

Again, why?

On Thu, Apr 16, 2020 at 09:08:06PM +0000, Saeed Mahameed wrote:

On Thu, 2020-04-16 at 15:58 -0400, Sasha Levin wrote:

...

On Thu, Apr 16, 2020 at 07:07:13PM +0000, Saeed Mahameed wrote:

...

On Thu, 2020-04-16 at 09:30 -0400, Sasha Levin wrote:

...

On Thu, Apr 16, 2020 at 08:24:09AM +0300, Leon Romanovsky wrote:

...

On Thu, Apr 16, 2020 at 04:08:10AM +0000, Saeed Mahameed wrote:

...

On Wed, 2020-04-15 at 20:00 -0400, Sasha Levin wrote: > On Wed, Apr 15, 2020 at 05:18:38PM +0100, Edward Cree > wrote: > > Firstly, let me apologise: my previous email was too > > harsh > > and too > > assertiveabout things that were really more uncertain > > and > > unclear. > > > > On 14/04/2020 21:57, Sasha Levin wrote: > > > I've pointed out that almost 50% of commits tagged for > > > stable do > > > not > > > have a fixes tag, and yet they are fixes. You really > > > deduce > > > things based > > > on coin flip probability? > > Yes, but far less than 50% of commits *not* tagged for > > stable > > have > > a fixes > > tag. It's not about hard-and-fast Aristotelian > > "deductions", like > > "this > > doesn't have Fixes:, therefore it is not a stable > > candidate", it's > > about > > probabilistic "induction". > > > > > "it does increase the amount of countervailing evidence > > > needed to > > > conclude a commit is a fix" - Please explain this > > > argument > > > given > > > the > > > above. > > Are you familiar with Bayesian statistics? If not, I'd > > suggest > > reading > > something like http://yudkowsky.net/rational/bayes/ > > which > > explains > > it. > > There's a big difference between a coin flip and a > > _correlated_ > > coin flip. > > I'd maybe point out that the selection process is based on > a > neural > network which knows about the existence of a Fixes tag in a > commit. > > It does exactly what you're describing, but also taking a > bunch > more > factors into it's desicion process ("panic"? "oops"? > "overflow"? > etc). >

I am not against AUTOSEL in general, as long as the decision to know how far back it is allowed to take a patch is made deterministically and not statistically based on some AI hunch.

Any auto selection for a patch without a Fixes tags can be catastrophic .. imagine a patch without a Fixes Tag with a single line that is fixing some "oops", such patch can be easily applied cleanly to stable- v.x and stable-v.y .. while it fixes the issue on v.x it might have catastrophic results on v.y ..

I tried to imagine such flow and failed to do so. Are you talking about anything specific or imaginary case?

It happens, rarely, but it does. However, all the cases I can think of happened with a stable tagged commit without a fixes where it's backport to an older tree caused unintended behavior (local denial of service in one case).

The scenario you have in mind is true for both stable and non- stable tagged patches, so it you want to restrict how we deal with commits that don't have a fixes tag shouldn't it be true for *all* commits?

All commits? even the ones without "oops" in them ? where does this stop ? :) We _must_ have a hard and deterministic cut for how far back to take a patch based on a human decision.. unless we are 100% positive autoselection AI can never make a mistake.

Humans are allowed to make mistakes, AI is not.

Oh I'm reviewing all patches myself after the bot does it's selection, you can blame me for these screw ups.

...

If a Fixes tag is wrong, then a human will be blamed, and that is perfectly fine, but if we have some statistical model that we know it is going to be wrong 0.001% of the time.. and we still let it run.. then something needs to be done about this.

I know there are benefits to autosel, but overtime, if this is not being audited, many pieces of the kernel will get broken unnoticed until some poor distro decides to upgrade their kernel version.

Quite a few distros are always running on the latest LTS releases, Android isn't that far behind either at this point.

There are actually very few non-LTS users at this point...

...

...

...

<...>

...

> Let me put my Microsoft employee hat on here. We have > driver/net/hyperv/ > which definitely wasn't getting all the fixes it should > have > been > getting without AUTOSEL. >

until some patch which shouldn't get backported slips through, believe me this will happen, just give it some time ..

Bugs are inevitable, I don't see many differences between bugs introduced by manually cherry-picking or automatically one.

Oh bugs slip in, that's why I track how many bugs slipped via stable tagged commits vs non-stable tagged ones, and the statistic may surprise you.

Statistics do not matter here, what really matters is that there is a possibility of a non-human induced error, this should be a no no. or at least make it an opt-in thing for those who want to take their chances and keep a close eye on it..

Hrm, why? Pretend that the bot is a human sitting somewhere sending mails out, how does it change anything?

If i know a bot might do something wrong, i Fix it and make sure it will never do it again. For humans i just can't do that, can I ? :) so this is the difference and why we all have jobs ..

...

...

...

The solution here is to beef up your testing infrastructure rather than

So please let me opt-in until I beef up my testing infra.

Already did :)

No you didn't :), I received more than 5 AUTOSEL emails only today and yesterday.

Please don't opt mlx5 out just yet ;-), i need to do some more research and make up my mind..

...

...

...

taking less patches; we still want to have *all* the fixes, right?

if you can be sure 100% it is the right thing to do, then yes, please don't hesitate to take that patch, even without asking anyone !!

Again, Humans are allowed to make mistakes.. AI is not.

Again, why?

Because AI is not there yet.. and this is a very big philosophical question.

Let me simplify: there is a bug in the AI, where it can choose a wrong patch, let's fix it.

You do realize that there are at least 2 steps in this "AI" where people are involved. The first is when Sasha goes thorough the patches and weeds out all of the "bad ones".

The second is when you, the maintainer, is asked if you think there is a problem if the patch is to be merged.

Then there's also the third, when again, I send out emails for the -rc process with the patches involved, and you are cc:ed on it.

This isn't an unchecked process here running with no human checks at all in it, so please don't speak of it like it is.

thanks,

greg k-h

On Fri, Apr 17, 2020 at 10:23:37PM +0000, Saeed Mahameed wrote:

On Fri, 2020-04-17 at 10:28 +0200, gregkh@linuxfoundation.org wrote:

...

...

Let me simplify: there is a bug in the AI, where it can choose a wrong patch, let's fix it.

You do realize that there are at least 2 steps in this "AI" where people are involved. The first is when Sasha goes thorough the patches and weeds out all of the "bad ones".

The second is when you, the maintainer, is asked if you think there is a problem if the patch is to be merged.

Then there's also the third, when again, I send out emails for the -rc process with the patches involved, and you are cc:ed on it.

This isn't an unchecked process here running with no human checks at all in it, so please don't speak of it like it is.

Sure I understand,

But with all do respect to Sasha and i know he is doing a great job, he just can't sign-off on all of the patches on all of the linux kernel and determine just by himself if a patch is good or not.. and the maintainer review is what actually matters here.

The maintainer review already happened when the patch went into Linus's tree.

But the maintainer ack is an optional thing, and I bet that the vast majority don't even look at these e-mails.

That is true.

My vision is that we make this an opt-in thing, and we somehow force all active and important kernel subsystems to opt-in, and make it the maintainer responsibility if something goes wrong.

That's a nice vision, and I too want a pony :)

Seriously, the first rule of the stable trees being created was that it was not going to cause any extra work for a maintainer to do, given that even 18 years ago, our maintainers were overloaded. Now that didn't totally happen, as I do ask for a cc: stable line to be added to patches to give me a hint as to what to apply.

Now some maintainers really don't care about stable trees, and don't even put those lines, which is fine for them, but not fine for me in wanting to make stable trees that contain the needed fixes for them that are going into Linus's tree. So over the years we have come up with tools to dig these patches out of Linus's tree. The latest version of that is this AUTOSEL tool, and after a number of maintainers complained that being notified at the last possible second (i.e. during a -rc cycle) was not early enough to stop some AUTOSEL patches from being merged, Sasha started up the process you see now, giving maintainers a few _weeks_ to object.

For the maintainers that don't care, fine, they just write a nice procmail rule and send the email to the round filing cabinet. For the maintainers that do care, they get a chance to object. For the maintainers that insist they are doing this all right and marking things correctly and don't want AUTOSEL running on their subsystems, they too have that option, which a few have already taken.

So that's where we are today, a process that has evolved over the decades into the one that at the moment, is producing pretty solid and good stable kernels as per the review of external parties. Yes, we can do better and find more patches that need to be backported, and are working on that. But to stop and try to go to an opt-in-only process would cause us to go backwards in the ability for us to provide kernels with useful bugfixes to users.

I understand from your statistics that this system is working very well, so i believe eventually every maintainer with a code that matters will come on board.

Sorry, that just will not happen. As proof of that, look at all of the maintainers today that are not "on board" with just a simple "cc: stable". And as I say above, that's fine, I am not going to ask them to do extra work that they do not want to do. And because of that, I, and others like Sasha, are going to have to do _extra_ work to make a better stable kernel release, and that's fine, we are the crazy ones here.

this way we don't risk it for inactive and less important subsystems/drivers.. and we guarantee the whole thing is properly audited with the maintainers on-board..

Have you met these maintainers that you can tell what to do despite not being their manager? If you think you can get them on board, then please, try to get them to do the simple thing first (cc: stable) and then we can talk :)

good luck!

greg k-h

On Fri, 2020-04-17 at 09:21 -0400, Sasha Levin wrote:

On Thu, Apr 16, 2020 at 09:08:06PM +0000, Saeed Mahameed wrote:

...

On Thu, 2020-04-16 at 15:58 -0400, Sasha Levin wrote:

...

Hrm, why? Pretend that the bot is a human sitting somewhere sending mails out, how does it change anything?

If i know a bot might do something wrong, i Fix it and make sure it will never do it again. For humans i just can't do that, can I ? :) so this is the difference and why we all have jobs ..

It's tricky because there's no one true value here. Humans are constantly wrong about whether a patch is a fix or not, so how can I train my bot to be 100% right?

...

...

...

...

The solution here is to beef up your testing infrastructure rather than

So please let me opt-in until I beef up my testing infra.

Already did :)

No you didn't :), I received more than 5 AUTOSEL emails only today and yesterday.

Appologies, this is just a result of how my process goes - patch selection happened a few days ago (which is when blacklists are applied), it's been running through my tests since, and mails get sent out only after tests.

No worries, as you see i am not really against this AI .. i am just worried about it being an opt-out thing :)

...

Please don't opt mlx5 out just yet ;-), i need to do some more research and make up my mind..

Alrighty. Keep in mind you can always reply with just a "no" to AUTOSEL mails, you don't have to explain why you don't want it included to keep it easy.

Sure ! thanks .

...

...

...

...

taking less patches; we still want to have *all* the fixes, right?

if you can be sure 100% it is the right thing to do, then yes, please don't hesitate to take that patch, even without asking anyone !!

Again, Humans are allowed to make mistakes.. AI is not.

Again, why?

Because AI is not there yet.. and this is a very big philosophical question.

Let me simplify: there is a bug in the AI, where it can choose a wrong patch, let's fix it.

But we don't know if it's wrong or not, so how can we teach it to be 100% right?

I keep retraining the NN based on previous results which improves it's accuracy, but it'll never be 100%.

The NN claims we're at ~95% with regards to past results.

I didn't really mean for you to fix it..

I am just against using un-audited AI. because i know it can never reach 100%.

Just out of curiosity : what are these 5% failure rate, what types of failures ? how are they identified and how are they feedback into the NN re-training ?

On Thu, Apr 16, 2020 at 04:40:31PM +0300, Or Gerlitz wrote:

On Thu, Apr 16, 2020 at 3:00 AM Sasha Levin sashal@kernel.org wrote:

...

I'd maybe point out that the selection process is based on a neural network which knows about the existence of a Fixes tag in a commit.

It does exactly what you're describing, but also taking a bunch more factors into it's desicion process ("panic"? "oops"? "overflow"? etc).

As Saeed commented, every extra line in stable / production kernel is wrong. IMHO it doesn't make any sense to take into stable automatically any patch that doesn't have fixes line. Do you have 1/2/3/4/5 concrete examples from your (referring to your Microsoft employee hat comment below) or other's people production environment where patches proved to be necessary but they lacked the fixes tag - would love to see them.

Sure, here are 5 from the past few months:

e5e884b42639 ("libertas: Fix two buffer overflows at parsing bss descriptor") 3d94a4a8373b ("mwifiex: fix possible heap overflow in mwifiex_process_country_ie()") 39d170b3cb62 ("ath6kl: fix a NULL-ptr-deref bug in ath6kl_usb_alloc_urb_from_pipe()") 8b51dc729147 ("rsi: fix a double free bug in rsi_91x_deinit()") 5146f95df782 ("USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data")

5 Different drivers, neither has a stable tag or a Fixes: tag, all 5 have a CVE number assigned to them.

We've been coaching new comers for years during internal and on-list code reviews to put proper fixes tag. This serves (A) for the upstream human review of the patch and (B) reasonable human stable considerations.

Thanks for doing it - we do see more and more fixes tags.

You are practically saying that for cases we screwed up stage (A) you can somehow still get away with good results on stage (B) - I don't accept it. BTW - during my reviews I tend to ask/require developers to skip the word panic, and instead better explain the nature of the problem / result.

Humans are still humans, and humans make mistakes. Fixes tags get forgotten, stable tags get forgotten.

I very much belive you that the mellanox stuff are in good shape thanks to your efforts, but the kernel world is bigger than a few select drivers.

...

...

...

This is great, but the kernel is more than just net/. Note that I also do not look at net/ itself, but rather drivers/net/ as those end up with a bunch of missed fixes.

...

...

drivers/net/ goes through the same DaveM net/net-next trees, with the same rules.

you ignored this comment, any more specific complaints?

See above (the example commits). The drivers/net/ stuff don't work as well as net/ sadly.

...

Let me put my Microsoft employee hat on here. We have driver/net/hyperv/ which definitely wasn't getting all the fixes it should have been getting without AUTOSEL.

...

While net/ is doing great, drivers/net/ is not. If it's indeed following the same rules then we need to talk about how we get done right.

I never [1] saw -stable push requests being ignored here in netdev. Your drivers have four listed maintainers and it's common habit by commercial companies to have paid && human (non autosel robots) maintainers that take care of their open source drivers. As in commercial SW products, Linux has a current, next and past (stable) releases, so something sounds as missing to me in your care matrix.

How come? DaveM is specifically asking not to add stable tags because he will do the selection himself, right? So the hyperv stuff indeed don't include a stable tag, but all fixes should have a proper Fixes: tag.

So why don't they end up in a stable tree?

If we need to send a mail saying which commits should go to stable, we might as well tag them for stable to begin with, right?

[1] actually I do remember that once or twice out of the 2020 times we asked, a patch was not sent to -stable by the sub-system maintainer mistake which he fixed(..) later

On Thu, Apr 16, 2020 at 05:17:09PM +0300, Or Gerlitz wrote:

On Thu, Apr 16, 2020 at 5:04 PM Sasha Levin sashal@kernel.org wrote:

...

On Thu, Apr 16, 2020 at 04:40:31PM +0300, Or Gerlitz wrote:

...

On Thu, Apr 16, 2020 at 3:00 AM Sasha Levin sashal@kernel.org wrote:

...

I'd maybe point out that the selection process is based on a neural network which knows about the existence of a Fixes tag in a commit.

It does exactly what you're describing, but also taking a bunch more factors into it's desicion process ("panic"? "oops"? "overflow"? etc).

As Saeed commented, every extra line in stable / production kernel is wrong. IMHO it doesn't make any sense to take into stable automatically any patch that doesn't have fixes line. Do you have 1/2/3/4/5 concrete examples from your (referring to your Microsoft employee hat comment below) or other's people production environment where patches proved to be necessary but they lacked the fixes tag - would love to see them.

Sure, here are 5 from the past few months:

e5e884b42639 ("libertas: Fix two buffer overflows at parsing bss descriptor") 3d94a4a8373b ("mwifiex: fix possible heap overflow in mwifiex_process_country_ie()") 39d170b3cb62 ("ath6kl: fix a NULL-ptr-deref bug in ath6kl_usb_alloc_urb_from_pipe()") 8b51dc729147 ("rsi: fix a double free bug in rsi_91x_deinit()") 5146f95df782 ("USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data")

5 Different drivers, neither has a stable tag or a Fixes: tag, all 5 have a CVE number assigned to them.

CVE number sounds good enough to me to AI around and pull to stable, BUT only to where relevant, and nothing beyond (see below).

But this is true with so many other things... This example was easy because I could grep for CVE, but what about commits that don't get assigned a CVE number but are just as severe?

Here are few without a CVE ID:

f700546682a6 ("rsi: fix nommu_map_sg overflow kernel panic") 3c68b8fffb48 ("dpaa_eth: FMan erratum A050385 workaround") 536dc5df2808 ("hv_netvsc: Fix memory leak when removing rndis device")

Could we agree here that important commits from drivers/net/ don't have a fixes tag but still should end up in stable?

...

...

We've been coaching new comers for years during internal and on-list code reviews to put proper fixes tag. This serves (A) for the upstream human review of the patch and (B) reasonable human stable considerations.

Thanks for doing it - we do see more and more fixes tags.

...

You are practically saying that for cases we screwed up stage (A) you can somehow still get away with good results on stage (B) - I don't accept it. BTW - during my reviews I tend to ask/require developers to skip the word panic, and instead better explain the nature of the problem / result.

Humans are still humans, and humans make mistakes. Fixes tags get forgotten

In the netdev land, the very much usual habit is for -rc patches to have Fixes tag, so maybe some RCA comment here would be to enforce that better across the kermel land?

I'd be very happy to see better tagging (not just fixes, but also reported-by/tested-by/etc).

...

stable tags get forgotten.

An easy AI would be to deduce the -stable tag from the -fixes tag, just find out where the offending patch was accepted and never/ever (please!) push a fix beyond that kernel.

This is tricky because not all commits with a fixes tag need to go to stable, right?

We have a bunch of commits that "fix" a documentation typo for example.

...

I very much belive you that the mellanox stuff are in good shape thanks to your efforts, but the kernel world is bigger than a few select drivers.

...

...

...

...

...

This is great, but the kernel is more than just net/. Note that I also do not look at net/ itself, but rather drivers/net/ as those end up with a bunch of missed fixes.

...

...

...

...

drivers/net/ goes through the same DaveM net/net-next trees, with the same rules.

...

...

you ignored this comment, any more specific complaints?

...

See above (the example commits). The drivers/net/ stuff don't work as well as net/ sadly.

Disagree.

If there is a problem it is due to some driver/net/ driver maintainers not doing their job good enough.

Maybe it's just me missing something here, but I thought that drivers/net/ aren't supposed to add a stable tag, they're just supposed to write good commit message, add a fixes tag if necessary, and the netdev maintainers will do the rest?

""" Q: How can I tell what patches are queued up for backporting to the various stable releases? A: Normally Greg Kroah-Hartman collects stable commits himself, but for networking, Dave collects up patches he deems critical for the networking subsystem, and then hands them off to Greg. """

There are 2 things I understood from this:

1. "networking subsystem" == net/ 2. Dave does patch selection.

...

...

...

Let me put my Microsoft employee hat on here. We have driver/net/hyperv/ which definitely wasn't getting all the fixes it should have been getting without AUTOSEL.

...

While net/ is doing great, drivers/net/ is not. If it's indeed following the same rules then we need to talk about how we get done right.

I never [1] saw -stable push requests being ignored here in netdev. Your drivers have four listed maintainers and it's common habit by commercial companies to have paid && human (non autosel robots) maintainers that take care of their open source drivers. As in commercial SW products, Linux has a current, next and past (stable) releases, so something sounds as missing to me in your care matrix.

How come? DaveM is specifically asking not to add stable tags because he will do the selection himself, right? So the hyperv stuff indeed don't include a stable tag, but all fixes should have a proper Fixes: tag.

Ask/tell your maintainer to note that in their cover letters, it will be taken.

Or allow them to tag stuff for stable? :)

...

So why don't they end up in a stable tree?

...

If we need to send a mail saying which commits should go to stable, we might as well tag them for stable to begin with, right?

so how about you go and argue with the netdev maintainer and settle your complaints instead of WA your disagreements using autosel?

So we had this discussion a while back and what I thought the outcome was that I'll ignore net/ and so I did.

I've mentioned it before - I'm not stuck on doing what I'm doing now for no reason. If the netdev maintainers want this done in a different way, or if you can show me data that what I'm doing is wrong, I'll be happy to change my workflow.

On Thu, 2020-04-16 at 19:20 +0200, Greg KH wrote:

On Thu, Apr 16, 2020 at 04:40:31PM +0300, Or Gerlitz wrote:

...

On Thu, Apr 16, 2020 at 3:00 AM Sasha Levin sashal@kernel.org wrote:

...

I'd maybe point out that the selection process is based on a neural network which knows about the existence of a Fixes tag in a commit.

It does exactly what you're describing, but also taking a bunch more factors into it's desicion process ("panic"? "oops"? "overflow"? etc).

As Saeed commented, every extra line in stable / production kernel is wrong.

What? On what do you base that crazy statement on? I have 18+ years of direct experience of that being the exact opposite.

Oh, I never said such a thing .. :(

I think Or meant to say: every extra line that no one asked for.

And all i wanted to say is that it can have a catastrophic result.. I know in many cases it is working well, and i didn't say it is wrong, I am just worried about it and wanted to show an example of how it can screw up under the radar with a simple single liner patch..

...

IMHO it doesn't make any sense to take into stable automatically any patch that doesn't have fixes line. Do you have 1/2/3/4/5 concrete examples from your (referring to your Microsoft employee hat comment below) or other's people production environment where patches proved to be necessary but they lacked the fixes tag - would love to see them.

Oh wow, where do you want me to start. I have zillions of these.

But wait, don't trust me, trust a 3rd party. Here's what Google's security team said about the last 9 months of 2019:

• 209 known vulnerabilities patched in LTS kernels, most

without CVEs

• 950+ criticial non-security bugs fixes for device XXXX alone with LTS releases

So opt-in for these critical or _always_ in use basic kernel sections. but make the default opt-out..

...

We've been coaching new comers for years during internal and on- list code reviews to put proper fixes tag. This serves (A) for the upstream human review of the patch and (B) reasonable human stable considerations.

If your driver/subsystem is doing this, wonderful, just opt-out of the autosel process and you will never be bothered again.

There are many legacy devices in the kernel that are not well maintained and being rarely fixed from random users.. if a fix will be picked up to the wrong kernel, it can go unnoticed for years..

But, trust me, I think I know a bit about tagging stuff for stable kernels, and yet the AUTOSEL tool keeps finding patches that _I_ forgot to tag as such. So, don't be so sure of yourself, it's humbling :)

Let the AUTOSEL tool run, and if it finds things you don't agree with, a simple "No, please do not include this" email is all you need to do to keep it out of a stable kernel.

So far the AUTOSEL tool has found so many real bugfixes that it isn't funny. If you don't like it, fine, but it has proven itself _way_ beyond my wildest hopes already, and it just keeps getting better.

Now i really don't know what the right balance here, in on one hand, autosel is doing a great job, on the other hand we know it can screw up in some cases, and we know it will.

So we decided to make sacrifices for the greater good ? :)

greg k-h

On Thu, 2020-04-16 at 15:53 -0400, Sasha Levin wrote:

On Thu, Apr 16, 2020 at 07:31:25PM +0000, Saeed Mahameed wrote:

...

On Thu, 2020-04-16 at 19:20 +0200, Greg KH wrote:

...

So far the AUTOSEL tool has found so many real bugfixes that it isn't funny. If you don't like it, fine, but it has proven itself _way_ beyond my wildest hopes already, and it just keeps getting better.

Now i really don't know what the right balance here, in on one hand, autosel is doing a great job, on the other hand we know it can screw up in some cases, and we know it will.

So we decided to make sacrifices for the greater good ? :)

autosel is going to screw up, I'm going to screw up, you're going to screw up, and Linus is going screw up. The existence of the stable trees and a "Fixes:" tag is an admission we all screw up, right?

Right, so fix this AI and we get one less reason to screw up ?

If you're willing to accept that we all make mistakes, you should also accept that we're making mistakes everywhere: we write buggy code, we fail at reviews, we forget tags, and we suck at backporting patches.

If we agree so far, then why do you assume that the same people who do the above also perfectly tag their commits, and do perfect selection of patches for stable? "I'm always right except when I'm wrong".

I am welling to accept people making mistakes, but not the AI..

I am not saying people should be 100% flawless, but i am assuming AI is 100% flawless. if I find a bug in AI I fix. same goes for autosel AI, it must get fixed.

What you are really saying: I don't like bugs, so i wrote an AI that fixes bugs but also can make bugs itself.

My view of the the path forward with stable trees is that we have to beef up our validation and testing story to be able to catch these issues better, rather than place arbitrary limitations on parts of the process. To me your suggestions around the Fixes: tag sound like "Never use kmalloc() because people often forget to free memory!" will it prevent memory leaks? sure, but it'll also prevent useful patches from coming it...

No, I will let people do what people do best (make more bugs) this is more than fine.

if it is necessary and we have a magical solution, i will write good AI with no false positives to fix or help avoid memleacks.

BUT if i can't achieve 100% success rate, and i might end up introducing memleack with my AI, then I wouldn't use AI at all.

We have different views on things.. if i know AI is using kmalloc wrongly, I fix it, end of story :).

fact: Your AI is broken, can introduce _new_ un-called for bugs, even it is very very very good 99.99% of the cases.

You are welling to roll the dice, i am not ..

Here's my suggestion: give us a test rig we can run our stable release candidates through. Something that simulates "real" load that customers are using. We promise that we won't release a stable kernel if your tests are failing.

I will be more than glad to do so, is there a formal process for such thing ?

On Thu, 2020-04-16 at 19:23 -0400, Sasha Levin wrote:

On Thu, Apr 16, 2020 at 09:32:47PM +0000, Saeed Mahameed wrote:

...

On Thu, 2020-04-16 at 15:53 -0400, Sasha Levin wrote:

...

If we agree so far, then why do you assume that the same people who do the above also perfectly tag their commits, and do perfect selection of patches for stable? "I'm always right except when I'm wrong".

I am welling to accept people making mistakes, but not the AI..

This is where we disagree. If I can have an AI that performs on par with an "average" kernel engineer - I'm happy with it.

The way I see AUTOSEL now is an "average" kernel engineer who does patch sorting for me to review.

Given I review everything that it spits out at me, it's technically a human error (mine), rather than a problem with the AI, right?

...

if it is necessary and we have a magical solution, i will write good AI with no false positives to fix or help avoid memleacks.

Easier said than done :)

I think that the "Intelligence" in AI suggests that it can be making mistakes.

...

BUT if i can't achieve 100% success rate, and i might end up introducing memleack with my AI, then I wouldn't use AI at all.

We have different views on things.. if i know AI is using kmalloc wrongly, I fix it, end of story :).

fact: Your AI is broken, can introduce _new_ un-called for bugs, even it is very very very good 99.99% of the cases.

People are broken too, they introduce new bugs, so why are we accepting new commits into the kernel?

My point is that everything is broken, you can't have 100% perfect anything.

...

...

Here's my suggestion: give us a test rig we can run our stable release candidates through. Something that simulates "real" load that customers are using. We promise that we won't release a stable kernel if your tests are failing.

I will be more than glad to do so, is there a formal process for such thing ?

I'd love to work with you on this if you're interested. There are a few options:

1. Send us a mail when you detect a push to a stable-rc branch. Most

people/bots reply to Greg's announce mail with pass/fail.

Sounds like our best option for now, as we already have our own testing infra that knows how to watch for external changes in mailing lists.

2. Integrate your tests into kernelci (kernelci.org) - this means

that you'll run a "lab" on prem, and kernelci will schedule builds and tests on it's own, sending reports to us.

3. We're open to other solutions if you had something in mind, the

first two usually work for people but if you have a different requirement we'll be happy to figure it out.

Thanks,

I will have to discuss this with our CI maintainers and see what we prefer.

i will let you know.

On Thu, 2020-04-16 at 13:08 -0700, Jakub Kicinski wrote:

On Thu, 16 Apr 2020 19:31:25 +0000 Saeed Mahameed wrote:

...

...

...

IMHO it doesn't make any sense to take into stable automatically any patch that doesn't have fixes line. Do you have 1/2/3/4/5 concrete examples from your (referring to your Microsoft employee hat comment below) or other's people production environment where patches proved to be necessary but they lacked the fixes tag - would love to see them.

Oh wow, where do you want me to start. I have zillions of these.

But wait, don't trust me, trust a 3rd party. Here's what Google's security team said about the last 9 months of 2019:

• 209 known vulnerabilities patched in LTS kernels, most

without CVEs

• 950+ criticial non-security bugs fixes for device XXXX alone with LTS releases

So opt-in for these critical or _always_ in use basic kernel sections. but make the default opt-out..

But the less attentive/weaker the maintainers the more benefit from autosel they get. The default has to be correct for the group which is more likely to take no action.

or the more exposed they are to false positives :), unnoticed bugs due to wrong patches getting backported.. this could go for years for less attentive weaker modules, until someone steps on it.

On 16/04/2020 01:00, Sasha Levin wrote:

I'd maybe point out that the selection process is based on a neural network which knows about the existence of a Fixes tag in a commit.

It does exactly what you're describing, but also taking a bunch more factors into it's desicion process ("panic"? "oops"? "overflow"? etc).

Yeah, that's why I found it odd that you were responding in a way that  _looked like_ classic confusion of P(A|B) and P(B|A).  I just wanted  to make sure we had that common ground before launching into a long  Bayesian explanation.

So, let's go: Let's imagine that 10% of all commits are stable-worthy, and we have a  threshold that says we autosel a patch if we think there's better than  50% chance that it's stable-worthy.  Then 50% of stable-worthy commits  have a Fixes: tag (whose referent exists in the stable tree already),  whereas some unknown fraction, let's say 5%, of non-stable-worthy  commits have a Fixes: tag. Then P(S|F) = P(F|S)P(S) / (P(F|S)P(S) + P(F|¬S)P(¬S))             = 0.05 / (0.05 + 0.045) = 0.526... That is, a patch with a Fixes: tag is 52.6% likely to be stable-worthy,  *not* 50%.  (The disparity would be bigger if P(F|¬S) were smaller;  conversely, if P(F|¬S) were larger, P(S|F) could be _less than_ 50%.) But also, P(S|¬F) = P(¬F|S)P(S) / (P(¬F|S)P(S) + P(¬F|¬S)P(¬S))                   = 0.05 / (0.05 + 0.855) = 0.055... That is, a patch without a Fixes: tag is only 5.5% likely to be stable-  worthy, which is *less* than the 10% base rate for all patches.  So  now you need to get *more* of the positive evidence (panic/oops/overflow  etc.) before you get pushed over the 40% threshold. Thus "increase the amount of countervailing evidence needed".

most fixes in -stable *don't* have a fixes tag. Shouldn't your argument be the opposite? If a patch has a fixes tag, it's probably not a fix?

I hope it's now clear that this statement confuses P(S|F) with P(F|S).

Let me put my Microsoft employee hat on here. We have driver/net/hyperv/ which definitely wasn't getting all the fixes it should have been getting without AUTOSEL.

While net/ is doing great, drivers/net/ is not. If it's indeed following the same rules then we need to talk about how we get done right.

I really have no objection to not looking in drivers/net/, it's just that the experience I had with the process suggests that it's not following the same process as net/.

Again, I'm not saying "don't look in drivers/net/", I'm saying increase  the probability threshold there: because _some_ of the stable candidates  have already been picked up by our process, the pickings in what's left  are thinner, i.e. the base rate P(S) is lower, so you need _more_  evidence before deciding to autosel something.  (I don't know exactly  how your NN is set up; is it able to use information like "is in  drivers/net/" as an input node?)  Part of the trouble is that the NN is  trained on "did this go to stable eventually", whereas being in  drivers/net/ is (on this theory) only a signal in the case where it  didn't go to stable _initially_ and had to be caught later; is that  information also present in your training data?  The NN would only be  expected to learn about drivers/net/ for itself if that were the case,  otherwise it would have no way of knowing about the lowered base rate. Conversely, if it *did* have that information (was this sent to stable  by maintainer's own processes, or was it found later to have been  missed) in the training data, it could learn these things by itself and  there'd be no need to do anything special for drivers/net/ (or, arguably,  even for net/).

How come? DaveM is specifically asking not to add stable tags because he will do the selection himself, right?

Driver maintainers sending patch series to Dave often include in the  cover letter "please consider patches 4, 7, 8 for stable".  It's *directly*  CCing stable on patch submissions that Dave asks people not to do. And it sounds from your Microsoft-hat like the HyperV maintainers might be  under that same misapprehension, if their stuff isn't making it to stable  as much as it should be.  But I haven't checked.

-ed

On 16/04/2020 19:49, Sasha Levin wrote:

Just a question while I process your explanation (thanks for doing it!): wouldn't this be done by the neural network?

Yes, in the basic case.  (Hopefully we're agreed that this is a long way  from "I'm not sure what a fixes tag has to do with inclusion in a stable  tree.", which is how this whole brouhaha started.)

It learns what a stable worthy commit is (and what isn't), and applies weights based on these findings, right? So if it learns that most non-stable commits don't have a fixes tag, it's likely to use that and "require" other inputs to have enough weight to compensate over a missing fixes tag so that it'll pass the threshold, no?

Yes.  The problem comes when there are other inputs the NN doesn't have,  that ought to screen off some of the information it's using.  This is  probably best illustrated by an unrealistic extreme case... Let's imagine hypothetically that the maintainer of drivers/blub is an  absolutely perfect judge of which patches should go to -stable, and  that the transmission path from him to the stable trees never loses a  patch.  This would mean that every autosel patch in drivers/blub is  necessarily a false positive, because all the 'true positives' it might  have found have already been taken out of the pool, so to speak.  But  if the NN is just trained to discriminate patches on whether they end  up going to stable, it won't see any difference between a drivers/blub  patch that the maintainer sent to stable straight away and a  drivers/wibble patch that the latter's less diligent maintainer didn't  forward and that only got picked up later when a stable kernel user  encountered the bug it was fixing. As long as the NN doesn't have that piece of information, it's going to  either generate lots of false positives in drivers/blub or lots of  false negatives in drivers/wibble. Now obviously drivers/blub doesn't exist, no maintainer is 100% perfect  at -stable submissions; but any difference will produce the same  effect on a smaller scale, with the 'blubbish' maintainers seeing a  high false positive fraction while from the 'wibblesome' maintainer's  point of view autosel is working great.  And since the 'blubs' are the  ones who're putting effort of their own into stable selection already,  they get aggrieved at having to also put effort into catching the  false positives from a system that doesn't seem to be doing much for  them, and everyone ends up shouting at each other as we're seeing here.

(Do you want me to do another worked numerical example demonstrating the  above, or does it make enough sense in words not to need one?)

-ed