Re: [patch net] devlink: convert occ_get op to separate registration

Re: [patch net] devlink: convert occ_get op to separate registration

[Sasha Levin]

Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: d9f9b9a4d05f devlink: Add support for resource abstraction.

The bot has tested the following trees: v4.16.1.

v4.16.1: Failed to apply! Possible dependencies:
    37923ed6b8ce ("netdevsim: Add simple FIB resource controller via devlink")
    3ed898e8cd05 ("mlxsw: spectrum_kvdl: Make some functions static")
    4f4bbf7c4e3d ("devlink: Perform cleanup of resource_set cb")
    51d3c08e3371 ("mlxsw: spectrum_kvdl: Add support for linear division resources")
    7f47b19bd744 ("mlxsw: spectrum_kvdl: Add support for per part occupancy")
    88d2fbcda145 ("mlxsw: spectrum: Pass mlxsw_core as arg of mlxsw_sp_kvdl_resources_register()")
    c8276dd250e9 ("mlxsw: spectrum_kvdl: Fix handling of resource_size_param")
    f9b9120119ca ("mlxsw: Constify devlink_resource_ops")


--
Thanks,
Sasha

Re: [patch net] devlink: convert occ_get op to separate registration

[Jiri Pirko]

Tue, Apr 10, 2018 at 03:49:40PM CEST, Alexander.Levin@microsoft.com wrote:
>Hi,
>
>[This is an automated email]
>
>This commit has been processed because it contains a "Fixes:" tag,
>fixing commit: d9f9b9a4d05f devlink: Add support for resource abstraction.

I don't think we need this fix in stable. Thanks.


>
>The bot has tested the following trees: v4.16.1.
>
>v4.16.1: Failed to apply! Possible dependencies:
>    37923ed6b8ce ("netdevsim: Add simple FIB resource controller via devlink")
>    3ed898e8cd05 ("mlxsw: spectrum_kvdl: Make some functions static")
>    4f4bbf7c4e3d ("devlink: Perform cleanup of resource_set cb")
>    51d3c08e3371 ("mlxsw: spectrum_kvdl: Add support for linear division resources")
>    7f47b19bd744 ("mlxsw: spectrum_kvdl: Add support for per part occupancy")
>    88d2fbcda145 ("mlxsw: spectrum: Pass mlxsw_core as arg of mlxsw_sp_kvdl_resources_register()")
>    c8276dd250e9 ("mlxsw: spectrum_kvdl: Fix handling of resource_size_param")
>    f9b9120119ca ("mlxsw: Constify devlink_resource_ops")
>
>
>--
>Thanks,
>Sasha

Re: [patch net] devlink: convert occ_get op to separate registration

[David Miller]

From: Sasha Levin <Alexander.Levin@microsoft.com>
Date: Tue, 10 Apr 2018 13:49:40 +0000

> This commit has been processed because it contains a "Fixes:" tag,
> fixing commit: d9f9b9a4d05f devlink: Add support for resource abstraction.
> 
> The bot has tested the following trees: v4.16.1.

This is nice, but it's becomming noise.

I pick and choose specific networking changes to queue up for -stable
and whether it has a Fixes tag or applies cleanly are not the primary
considerations I take into account when deciding what goes to stable
or not.

What matters primarily are two attributes:

1) What is the impact of the bug?

2) How risky is the change?

3) Did someone explicitly ask for the backport?  Why?

These automated emails tell me nothing about either attribute.

I figure out what Fixes: tags are involved and whether the patch
applies cleanly when I process my stable queue of networking patches.

For various reasons I have to do this all manually anyways.  I always
do a full analysis of where the bug came from, and also do manual
code inspection to find the cases where a Fixes: tag indicates a commit
that was also put into stable branches and what impact that has on
the backport of the stable fix for a particular stable tree.

So I would like to kindly ask that you stop sending these automated
emails, they really are not helping my networking stable backport
process at all, and instead are just making more emails I have to
delete from my inbox every day.

Thank you very much for your kind consideration in this matter.

Re: [patch net] devlink: convert occ_get op to separate registration

[David Miller]

From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 10 Apr 2018 16:13:09 +0200

> Tue, Apr 10, 2018 at 03:49:40PM CEST, Alexander.Levin@microsoft.com wrote:
>>Hi,
>>
>>[This is an automated email]
>>
>>This commit has been processed because it contains a "Fixes:" tag,
>>fixing commit: d9f9b9a4d05f devlink: Add support for resource abstraction.
> 
> I don't think we need this fix in stable. Thanks.

And as I stated in another email, I don't think we need these automated
things either.

Re: [patch net] devlink: convert occ_get op to separate registration

[Sasha Levin]

On Tue, Apr 10, 2018 at 10:35:07AM -0400, David Miller wrote:
>From: Sasha Levin <Alexander.Levin@microsoft.com>
>Date: Tue, 10 Apr 2018 13:49:40 +0000
>
>> This commit has been processed because it contains a "Fixes:" tag,
>> fixing commit: d9f9b9a4d05f devlink: Add support for resource abstraction.
>>
>> The bot has tested the following trees: v4.16.1.
>
>This is nice, but it's becomming noise.

Thank you, and sorry. I've blacklisted net/ and drivers/net/.

>I pick and choose specific networking changes to queue up for -stable
>and whether it has a Fixes tag or applies cleanly are not the primary
>considerations I take into account when deciding what goes to stable
>or not.
>
>What matters primarily are two attributes:
>
>1) What is the impact of the bug?
>
>2) How risky is the change?
>
>3) Did someone explicitly ask for the backport?  Why?

When I started the autoselection project, I found it interesting that
the networking subsystem has a unique workflow for stable commits where
a single person would review every single patch that gets merged, and
would select patches purely based on whether the patch follows the
stable rules or not, without relying on stable tags.

For the first few iterations of the neural network, I thought that the
networking commit set is valuable since if I tried to learn on commits
that were tagged for stable, the neural network would develop a bias
towards the tag which will prevent it from detecting bug fixing patches
that do not contain such tags.

However, during validation, I found that the quality of the selection on
a random set of commits wasn't what I expected it to be. I reviewed a
random sample of networking commits and found that a high percentage of
commits that are clearly "bug fixing", were not included in any stable
tree.

This changed my view on how the stable process works for different
subsystems; I realized that there are 3 main things we look for in
stable trees:

 1. They have all bug fixing commits that are relevant to that tree.
 2. There are none non-bug-fixing commits in the tree.
 3. The commits are backported correctly to that tree.

And since subsystems have different workflows for stable, I could see
how different approaches balanced these goals. For networking, it was
easy to see that a high quality of review by the maintainer prevented
and non-bug-fixing commits from creeping in, and the error rates for
backports were much smaller than any other subsystem I looked at.

What was missing was the first goal. My hypothesis was that after all,
we are only humans; with the high rate of commits that flow into a
subsystem, and the opt-in nature of the process, we are bound to
incorrectly miss commits that would otherwise be in a stable tree.

I could back up my theory with experiences other folks had [1][2], 
so one of my biggest hopes was to see how the autoselection would could
improve the quality of selection and build on the biggest advantage the
networking subsystem over other subsystems: a dedicated maintainer who
actually gives a crap and reviews these patches himself.

When the work on the neural network itself quieted down, and the first
results started trickling in, I was delighted that we can now help
select commits that were missed by manual inspection, but are clearly
bug fixing. For example, [3][4][5].

>These automated emails tell me nothing about either attribute.

The attributes you've listed are subjective, and are the final step of
selecting patches for stable trees. Consider the following additional
attributes you undoubtfully look at when going through commits:

 1. Is it fixing a bug?
 2. Is it relevant to any of the stable trees?

Only after deciding on these attributes, it makes sense to look at ones
that require actual thinking (such as the ones you've listed).

>I figure out what Fixes: tags are involved and whether the patch
>applies cleanly when I process my stable queue of networking patches.

This is another thing that doesn't always end up right. When done
manually patches tend to get lost, forgotten, or end up with a partial
backport.

Doing this is a combination of "dumb" manual labor where for each stable
tree we try to find whether the commit we fix exists in it, and which
other commits we may depend on, and "smart" labor where we try to figure
out whether the commit should actually be in that tree.

The bot tries to take the "dumb" part out of your way, by letting
you know from the start which trees this applied/built on and what
dependencies it might have. It comes for free, why not use it?

As an example, a fix for CVE-2017-16939 [6] was backported to v4.9
but not to v4.4 or v3.18 because of missing dependencies.

>For various reasons I have to do this all manually anyways.  I always
>do a full analysis of where the bug came from, and also do manual
>code inspection to find the cases where a Fixes: tag indicates a commit
>that was also put into stable branches and what impact that has on
>the backport of the stable fix for a particular stable tree.

Would you be interested in going over these reasons, and whether the bot
can automate any of these? Things like determining if a Fixes: commit is
already in stable are already automated (excluding the impact analysis,
ofcourse).

>So I would like to kindly ask that you stop sending these automated
>emails, they really are not helping my networking stable backport
>process at all, and instead are just making more emails I have to
>delete from my inbox every day.
>
>Thank you very much for your kind consideration in this matter.

What would be the appropriate way to submit these to the networking
stable branch?

Let's look at a concrete example: I've sent a mail for a dccp fix[7] a
few days ago. Could we discuss why it's not in any stable tree?



[1] https://www.spinics.net/lists/stable/msg173995.html
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c58d6c93680f28ac58984af61d0a7ebf4319c241
[3] https://patchwork.kernel.org/patch/10188235/
[4] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/?h=linux-4.4.y&id=e29066778bc28eff5f63616800c6b60f12c87267
[5] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/?h=linux-4.4.y&id=7c5deeccc664a79d5586df86b4221493f28e9ef9
[6] https://www.spinics.net/lists/netdev/msg469458.html
[7] https://lkml.org/lkml/2018/4/8/762

Re: [patch net] devlink: convert occ_get op to separate registration

[David Miller]

From: Sasha Levin <Alexander.Levin@microsoft.com>
Date: Tue, 10 Apr 2018 19:08:20 +0000

> The bot tries to take the "dumb" part out of your way, by letting
> you know from the start which trees this applied/built on and what
> dependencies it might have. It comes for free, why not use it?

I do this already while I'm processing the -stable queue in patchwork
and it automatically falls right out of the process I use to extract
patches out of Linus's GIT tree.

I manually pull the commit out of Linus's tree, verify that it is
actually the commit I'm interested in, then I do two things:

1) I look at the exact tag that the commit landed in using
   "git describe --contains SHA1_ID"

2) I look at the exact tag that the commit the Fixes: tag
   points at landed using "git describe --contains SHA1_ID"

I double check #2 to see for cases whether the Fixes: tag
itself was backported to stable trees.

I use these two tag values to organize the -stable queue into
subdirectories.  This guides my patch applying in order to minimize
useless work.

So I have to do all of this work anyways.

Even if the bot provided these values, I would still double
check them, every single one of them.

Therefore, the net result from my perspective is that for most
patches fixing bugs on this list, instead of N list postings,
there will now be at least N * 2.

Bots are starting to overwhelm actual content from human beings
on this list, and I want to put my foot on the brake right now
before it gets even more out of control.

Thank you.

Re: [patch net] devlink: convert occ_get op to separate registration

[Sasha Levin]

On Tue, Apr 10, 2018 at 03:22:57PM -0400, David Miller wrote:
>From: Sasha Levin <Alexander.Levin@microsoft.com>
>Date: Tue, 10 Apr 2018 19:08:20 +0000
>
>> The bot tries to take the "dumb" part out of your way, by letting
>> you know from the start which trees this applied/built on and what
>> dependencies it might have. It comes for free, why not use it?
>
>I do this already while I'm processing the -stable queue in patchwork
>and it automatically falls right out of the process I use to extract
>patches out of Linus's GIT tree.
>
>I manually pull the commit out of Linus's tree, verify that it is
>actually the commit I'm interested in, then I do two things:
>
>1) I look at the exact tag that the commit landed in using
>   "git describe --contains SHA1_ID"
>
>2) I look at the exact tag that the commit the Fixes: tag
>   points at landed using "git describe --contains SHA1_ID"
>
>I double check #2 to see for cases whether the Fixes: tag
>itself was backported to stable trees.
>
>I use these two tag values to organize the -stable queue into
>subdirectories.  This guides my patch applying in order to minimize
>useless work.
>
>So I have to do all of this work anyways.
>
>Even if the bot provided these values, I would still double
>check them, every single one of them.
>
>Therefore, the net result from my perspective is that for most
>patches fixing bugs on this list, instead of N list postings,
>there will now be at least N * 2.

Gotcha. I can keep it out from the regular patch flow. How could we
address commits that might have been missed? Let's say I look at commits
that are older than ~1 month, how can I get those looked at again?

>Bots are starting to overwhelm actual content from human beings
>on this list, and I want to put my foot on the brake right now
>before it gets even more out of control.

I think we're just hitting the limitations of using a mailing list. Bots
aren't around to spam everyone for fun, right?

0day bot is spammy because patches fail to compile, syzbot is spammy
because we have tons of bugs users can hit and the stable bot is
spammy because we miss lots of commits that should be in stable.

As the kernel grows, not only the codebase is expanding but also the
tooling around it. While spammy, bots provide valuable input that in the
past would take blood, sweat and tears to acquire. We just need a better
way to consume it rather than blocking off these inputs.

Thanks!

Re: [patch net] devlink: convert occ_get op to separate registration

[gregkh]

On Tue, Apr 10, 2018 at 08:43:31PM +0000, Sasha Levin wrote:
> >Bots are starting to overwhelm actual content from human beings
> >on this list, and I want to put my foot on the brake right now
> >before it gets even more out of control.
> 
> I think we're just hitting the limitations of using a mailing list. Bots
> aren't around to spam everyone for fun, right?
> 
> 0day bot is spammy because patches fail to compile, syzbot is spammy
> because we have tons of bugs users can hit and the stable bot is
> spammy because we miss lots of commits that should be in stable.
> 
> As the kernel grows, not only the codebase is expanding but also the
> tooling around it. While spammy, bots provide valuable input that in the
> past would take blood, sweat and tears to acquire. We just need a better
> way to consume it rather than blocking off these inputs.

As one of the primary abusers of the "I send too much email" flood of
mess, I'm going to start cutting down on what type of message I respond
to a public vger list from now on.  I'll write more on the stable@ list
about this, but for your individual patches like this, how about just
responding to the developers themselves and not a public list?  I do
that when I commit a patch to my tree, stripping out lists, as it's not
a useful message for anyone other than the person who wrote the patch
and the reviewers.

thanks,

greg k-h

Re: [patch net] devlink: convert occ_get op to separate registration

[Sasha Levin]

On Wed, Apr 11, 2018 at 10:46:00AM +0200, gregkh@linuxfoundation.org wrote:
>On Tue, Apr 10, 2018 at 08:43:31PM +0000, Sasha Levin wrote:
>> >Bots are starting to overwhelm actual content from human beings
>> >on this list, and I want to put my foot on the brake right now
>> >before it gets even more out of control.
>>
>> I think we're just hitting the limitations of using a mailing list. Bots
>> aren't around to spam everyone for fun, right?
>>
>> 0day bot is spammy because patches fail to compile, syzbot is spammy
>> because we have tons of bugs users can hit and the stable bot is
>> spammy because we miss lots of commits that should be in stable.
>>
>> As the kernel grows, not only the codebase is expanding but also the
>> tooling around it. While spammy, bots provide valuable input that in the
>> past would take blood, sweat and tears to acquire. We just need a better
>> way to consume it rather than blocking off these inputs.
>
>As one of the primary abusers of the "I send too much email" flood of
>mess, I'm going to start cutting down on what type of message I respond
>to a public vger list from now on.  I'll write more on the stable@ list
>about this, but for your individual patches like this, how about just
>responding to the developers themselves and not a public list?  I do
>that when I commit a patch to my tree, stripping out lists, as it's not
>a useful message for anyone other than the person who wrote the patch
>and the reviewers.

Sure.

I think we should have a discussion as to the best way to "spam" people.
There are a few alternatives I can think of (a "digest" mail once a
day/week? Web UI? different mailing list?) but we keep using the one
size fits all approach for all our bots.

I don't see a reason why we can't tailor bot output based on
maintainer/author preference? Maybe David would be less upset if he'd
see just one mail per week with commits we ask to review?

We're stuck with mailing lists for kernel development, might as well
make the best out of it.