On Thu, Apr 29, 2021 at 11:00 PM Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> From: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
> Subject: kasan: detect false-positives in tests
>
> Currently, KASAN-KUnit tests can check that a particular annotated part of
> code causes a KASAN report. However, they do not check that no unwanted
> reports happen between the annotated parts.
>
> This patch implements these checks.
No.
This patch does no such thing.
This patch is untested garbage that doesn't even compile.
> + if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) { \
> + if (READ_ONCE(fail_data.report_found)) \
> + kasan_enable_tagging(); \
> + migrate_enable(); \
kasan_enable_tagging() was renamed to kasan_enable_tagging_sync() in commit
2603f8a78dfb ("kasan: Add KASAN mode kernel parameter")
and this patch was clearly rebased onto current -git without any testing at all.
You can even see the effects of the rename in the patch itself, in
that the code it removes has the new name:
- if (IS_ENABLED(CONFIG_KASAN_HW_TAGS) && \
- !kasan_async_mode_enabled()) { \
- if (READ_ONCE(fail_data.report_found)) \
- kasan_enable_tagging_sync(); \
but then the patch re-introduces the old name in the new version of that code.
Andrew, you really need to stop using your garbage workflow of
randomly rebasing patches at the last minute.
In the cover letter, you clearly say:
"178 patches, based on 8ca5297e7e38f2dc8c753d33a5092e7be181fff0."
where that commit was basically my top commit as of yesterday
afternoon - and that base very much has that rename in place.
So you rebased your patch series - apparently without any testing at
all - since that afternoon commit.
This *HAS* to stop.
If you use your old legacy "patch series" model, then dammit, stop
sending those patches on top of random commits. Pick a base, and
*STICK* to it. Make sure your patch series is actually *tested*.
Because I'd much rather handle conflicts and blame myself if I screw
something up during the merge, than get something from you that is
untested and may be subtly - or as in this case, very much not-subtly
- broken.
Right now, the way you work, all the work that Stephen Rothwell has
done to merge your patch series and have it tested in linux-next is
basically made almost entirely worthless, because you end up changing
your patches and rebasing it all the last minute, and all the testing
it got earlier is now thrown right out the window.
What used to be a perfectly cromulent patch is now garbage that
doesn't even compile.
I'm going to fix this one patch up by hand, but I really need you to
stop doing this. Use the previous release as a base (ie in this case
5.12), and don't do any last-minute rebasing. If you have patches in
your patch-series that depend on stuff that comes in this merge
window, just drop them - or at least don't send them to me.
Or better yet, base it on something even older than the release that
opens the merge window, so that you don't have to rebase anything
*during* the merge window, and so that your patch series actually gets
tested as-is in the real format in linux-next even before the merge
window opens. If you had created this series two weeks ago, marked it
as "ready for the 5.13 merge window", and it had been tested as such
in linux-next, we would have known about the conflict from linux-next,
and I'd have seen it as something that was trivial to fix when I did
the merge.
And that would catch all the cases that I can't catch, because
linux-next actually does a whole lot more build-testing than I do.
Different architectures, lots of different configs etc.
Instead, I applied a series of 178 patches (ok, 175 after I had
dropped a couple), saw nothing wrong, and did my (very basic and
minimal) build testing, and saw that the patches you sent to me didn't
even compile.
I bet a fixed workflow will make it easier for Stephen too, to not
continually have to rebase your series, and have a separate queue for
patch-series that get randomly updated.
I'm perfectly happy with you not using git - I apply patches from
others too. But I don't want to see this broken "rebase with no
testing" model. A patch queue that has its base changed has all the
same issues that a blind "git rebase" has, and is wrong for all the
same reasons - all whether you use git or not.
Linus
