linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: Sasha Levin <sashal@kernel.org>
Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org
Subject: Re: AUTOSEL process
Date: Tue, 28 Feb 2023 00:32:20 +0000	[thread overview]
Message-ID: <Y/1LlA5WogOAPBNv@gmail.com> (raw)
In-Reply-To: <Y/0wMiOwoeLcFefc@sashalap>

On Mon, Feb 27, 2023 at 05:35:30PM -0500, Sasha Levin wrote:
> > > Note, however, that it's not enough to keep pointing at a tiny set and
> > > using it to suggest that the entire process is broken. How many AUTOSEL
> > > commits introduced a regression? How many -stable tagged ones did? How
> > > many bugs did AUTOSEL commits fix?
> > 
> > So basically you don't accept feedback from individual people, as individual
> > people don't have enough data?
> 
> I'd love to improve the process, but for that we need to figure out
> criteria for what we consider good or bad, collect data, and make
> decisions based on that data.
> 
> What I'm getting from this thread is a few anecdotal examples and
> statements that the process isn't working at all.
> 
> I took Jon's stablefixes script which he used for his previous articles
> around stable kernel regressions (here:
> https://lwn.net/Articles/812231/) and tried running it on the 5.15
> stable tree (just a random pick). I've proceeded with ignoring the
> non-user-visible regressions as Jon defined in his article (basically
> issues that were introduced and fixed in the same releases) and ended up
> with 604 commits that caused a user visible regression.
> 
> Out of those 604 commits:
> 
>  - 170 had an explicit stable tag.
>  - 434 did not have a stable tag.
> 
> Looking at the commits in the 5.15 tree:
> 
> With stable tag:
> 
> 	$ git log --oneline -i --grep "cc.*stable" v5.15..stable/linux-5.15.y | wc -l
> 	3676
> 
> Without stable tag (-96 commits which are version bumps):
> 
> 	$ git log --oneline --invert-grep -i --grep "cc.*stable" v5.15..stable/linux-5.15.y | wc -l
> 	10649
> 
> Regression rate for commits with stable tag: 170 / 3676 = 4.62%
> Regression rate for commits without a stable tag: 434 / 10553 = 4.11%
> 
> Is the analysis flawed somehow? Probably, and I'd happy take feedback on
> how/what I can do better, but this type of analysis is what I look for
> to know if the process is working well or not.

I'm shocked that these are the statistics you use to claim the current AUTOSEL
process is working.  I think they actually show quite the opposite!

First, since many AUTOSEL commits aren't actually fixes but nearly all
stable-tagged commits *are* fixes, the rate of regressions per commit would need
to be lower for AUTOSEL commits than for stable-tagged commits in order for
AUTOSEL commits to have the same rate of regressions *per fix*.  Your numbers
suggest a similar regression rate *per commit*.  Thus, AUTOSEL probably
introduces more regressions *per fix* than stable-tagged commits.

Second, the way you're identifying regression-introducing commits seems to be
excluding one of the most common, maybe *the* most common, cause of AUTOSEL
regressions: missing prerequisite commits.  A very common case that I've seen
repeatedly is AUTOSEL picking just patch 2 or higher of a multi-patch series.
For an example, see the patch that started this thread...  If a missing
prerequisite is backported later, my understanding is that it usually isn't
given a Fixes tag, as the upstream commit didn't have it.  I think such
regressions aren't counted in your statistic, which only looks at Fixes tags.

(Of course, stable-tagged commits sometimes have missing prerequisite bugs too.
But it's expected to be at a lower rate, since the original developers and
maintainers are directly involved in adding the stable tags.  These are the
people who are more familiar than anyone else with prerequisites.)

Third, the category "commits without a stable tag" doesn't include just AUTOSEL
commits, but also non-AUTOSEL commits that people asked to be added to stable
because they fixed a problem for them.  Such commits often have been in mainline
for a long time, so naturally they're expected to have a lower regression rate
than stable-tagged commits due to the longer soak time, on average.  So if the
regression rate of stable-tagged and non-stable-tagged commits is actually
similar, that suggests the regression rate of non-stable-tagged commits is being
brought up artifically by a high regression rate in AUTOSEL commits...

So, I think your statistics actually reflect quite badly on AUTOSEL in its
current form.

By the way, to be clear, AUTOSEL is absolutely needed.  The way you are doing it
currently is not working well, though.  I think it needs to be tuned to select
fewer, higher-confidence fixes, and you need to do some basic checks against
each one, like "does this commit have a pending fix" and "is this commit part of
a multi-patch series, and if so are earlier patches needed as prerequisites".
There also needs to be more soak time in mainline, and more review time.

IMO you also need to take a hard look at whatever neural network thing you are
using, as from what I've seen its results are quite poor...  It does pick up
some obvious fixes, but it seems they could have just as easily been found
through some heuristics with grep.  Beyond those obvious fixes, what it picks up
seems to be barely distinguishable from a random selection.

- Eric

  parent reply	other threads:[~2023-02-28  0:32 UTC|newest]

Thread overview: 102+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-26  3:42 [PATCH AUTOSEL 6.1 01/21] ARM: OMAP2+: omap4-common: Fix refcount leak bug Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 02/21] arm64: dts: qcom: msm8996: Add additional A2NoC clocks Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 03/21] udf: Define EFSCORRUPTED error code Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 04/21] context_tracking: Fix noinstr vs KASAN Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 05/21] exit: Detect and fix irq disabled state in oops Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 06/21] ARM: dts: exynos: Use Exynos5420 compatible for the MIPI video phy Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 07/21] fs: Use CHECK_DATA_CORRUPTION() when kernel bugs are detected Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 08/21] blk-iocost: fix divide by 0 error in calc_lcoefs() Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 09/21] blk-cgroup: dropping parent refcount after pd_free_fn() is done Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 10/21] blk-cgroup: synchronize pd_free_fn() from blkg_free_workfn() and blkcg_deactivate_policy() Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 11/21] trace/blktrace: fix memory leak with using debugfs_lookup() Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 12/21] fs/super.c: stop calling fscrypt_destroy_keyring() from __put_super() Sasha Levin
2023-02-26  4:07   ` Eric Biggers
2023-02-26  5:30     ` Eric Biggers
2023-02-26 19:24       ` Eric Biggers
2023-02-26 19:33         ` Slade Watkins
2023-02-27 14:18         ` Sasha Levin
2023-02-27 17:47           ` AUTOSEL process Eric Biggers
2023-02-27 18:06             ` Eric Biggers
2023-02-27 20:39               ` Sasha Levin
2023-02-27 21:38                 ` Eric Biggers
2023-02-27 22:35                   ` Sasha Levin
2023-02-27 22:59                     ` Matthew Wilcox
2023-02-28  0:52                       ` Sasha Levin
2023-02-28  1:25                         ` Eric Biggers
2023-02-28  4:25                           ` Willy Tarreau
2023-03-30  0:08                         ` Eric Biggers
2023-03-30 14:05                           ` Sasha Levin
2023-03-30 17:22                             ` Eric Biggers
2023-03-30 17:50                               ` Sasha Levin
2023-02-28  0:32                     ` Eric Biggers [this message]
2023-02-28  1:53                       ` Sasha Levin
2023-02-28  3:41                         ` Eric Biggers
2023-02-28 10:41                           ` Amir Goldstein
2023-02-28 11:28                             ` Greg KH
2023-03-01  2:05                               ` Slade Watkins
2023-03-01  5:13                                 ` Eric Biggers
2023-03-01  6:09                                   ` Greg KH
2023-03-01  7:22                                     ` Eric Biggers
2023-03-01  7:40                                       ` Willy Tarreau
2023-03-01  8:31                                         ` Eric Biggers
2023-03-01  8:43                                           ` Greg KH
2023-03-01  6:06                                 ` Greg KH
2023-03-01  7:05                                   ` Eric Biggers
2023-03-01 10:31                               ` Thorsten Leemhuis
2023-03-01 13:26                               ` Mark Brown
2023-02-28 17:03                           ` Sasha Levin
2023-03-10 23:07                           ` Eric Biggers
2023-03-11 13:41                             ` Sasha Levin
2023-03-11 15:54                               ` James Bottomley
2023-03-11 18:07                                 ` Sasha Levin
2023-03-12 19:03                                   ` Theodore Ts'o
2023-03-07 21:18               ` Pavel Machek
2023-03-07 21:45                 ` Eric Biggers
2023-03-11  6:25                   ` Matthew Wilcox
2023-03-11  8:11                     ` Willy Tarreau
2023-03-11 11:45                       ` Pavel Machek
2023-03-11 12:29                         ` Greg KH
2023-03-21 12:41                           ` Maciej W. Rozycki
2023-03-11 14:06                     ` Sasha Levin
2023-03-11 16:16                       ` Theodore Ts'o
2023-03-11 17:48                         ` Eric Biggers
2023-03-11 18:26                           ` Sasha Levin
2023-03-11 18:54                             ` Eric Biggers
2023-03-11 19:01                               ` Eric Biggers
2023-03-11 21:14                               ` Sasha Levin
2023-03-12  8:04                                 ` Amir Goldstein
2023-03-12 16:00                                   ` Sasha Levin
2023-03-13 17:41                               ` Greg KH
2023-03-13 18:54                                 ` Eric Biggers
2023-03-14 18:26                                   ` Greg KH
2023-03-11 20:17                             ` Eric Biggers
2023-03-11 21:02                               ` Sasha Levin
2023-03-12  4:23                                 ` Willy Tarreau
2023-03-11 18:33                           ` Willy Tarreau
2023-03-11 19:24                             ` Eric Biggers
2023-03-11 19:46                               ` Eric Biggers
2023-03-11 20:19                                 ` Willy Tarreau
2023-03-11 20:59                                   ` Sasha Levin
2023-03-11 20:11                               ` Willy Tarreau
2023-03-11 20:53                                 ` Eric Biggers
2023-03-12  4:32                                   ` Willy Tarreau
2023-03-12  5:21                                     ` Eric Biggers
2023-03-12  5:48                                       ` Willy Tarreau
2023-03-12  7:42                                       ` Amir Goldstein
2023-03-12 13:34                                         ` Mark Brown
2023-03-12 15:57                                         ` Sasha Levin
2023-03-12 13:55                                 ` Mark Brown
2023-03-11 22:38                       ` David Laight
2023-03-12  4:41                         ` Willy Tarreau
2023-03-12  5:09                           ` Theodore Ts'o
2023-03-14 14:12                             ` Jan Kara
2023-03-13  3:37             ` Bagas Sanjaya
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 13/21] sched/fair: sanitize vruntime of entity being placed Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 14/21] btrfs: scrub: improve tree block error reporting Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 15/21] arm64: zynqmp: Enable hs termination flag for USB dwc3 controller Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 16/21] cpuidle, intel_idle: Fix CPUIDLE_FLAG_INIT_XSTATE Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 17/21] entry, kasan, x86: Disallow overriding mem*() functions Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 18/21] x86/fpu: Don't set TIF_NEED_FPU_LOAD for PF_IO_WORKER threads Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 19/21] cpuidle: drivers: firmware: psci: Dont instrument suspend code Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 20/21] cpuidle: lib/bug: Disable rcu_is_watching() during WARN/BUG Sasha Levin
2023-02-26  3:42 ` [PATCH AUTOSEL 6.1 21/21] perf/x86/intel/uncore: Add Meteor Lake support Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y/1LlA5WogOAPBNv@gmail.com \
    --to=ebiggers@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).