From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4E751C433ED for ; Fri, 30 Apr 2021 18:33:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 24774613F7 for ; Fri, 30 Apr 2021 18:33:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230329AbhD3Sd5 (ORCPT ); Fri, 30 Apr 2021 14:33:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44926 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229750AbhD3Sdw (ORCPT ); Fri, 30 Apr 2021 14:33:52 -0400 Received: from mail-lf1-x129.google.com (mail-lf1-x129.google.com [IPv6:2a00:1450:4864:20::129]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C8F51C06174A for ; Fri, 30 Apr 2021 11:33:02 -0700 (PDT) Received: by mail-lf1-x129.google.com with SMTP id j10so18369620lfb.12 for ; Fri, 30 Apr 2021 11:33:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=P1Q7BrgOtfscywkg5hVce2+3guXdura6NsxZgt2BXPQ=; b=ghUSTWnV/xszGi51+GjhZMwQjv3qdRdMqSYN9LIkqgLV5KPrG+VjnGtbADfK1LmrIp YRwqJBjUp/Uy5HhEoka+hBk5fKZ+mSgxT3bexDrNzdicKrv/nhGO/Nj+ZNYu52WV0KQM TvoSIw5LXjBTwfmuto3eXFfjthwjQMW9AxyII= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=P1Q7BrgOtfscywkg5hVce2+3guXdura6NsxZgt2BXPQ=; b=nivAV0OU+Nqt7DaTOnEUhPLobkEaXULfCk7kog/cqWjxA54QCf0k9xy3TUvLGyOGWJ JVPqUqrYsPskUP03mKY+GCbZLD7maHr3evtIMb8P5KPKJOwNoKbBg+FmLbjIJdBCnVMl /2gV7tUBHB11MTgtieHzOb6ato5qVnqp89BnIOfTyKi/uQyTNo0/DvYm3sx1Z9/5i7c9 idJkU09563sh0ejUmRMfUYaOXTEFKw8WffjAJwd8+GKaqL5K89/cGlLHaW7d3tI+539r 1WNSrvlKh0CJcZN0JOzMG06GEW+Onmx34sDvBfQNcI+iX0Px8C16Es7zvFnkPqQrDYUK v8jA== X-Gm-Message-State: AOAM531Vk3FnaY34D0PXW15u6rTdolSvyigc6dJuXXmHjoRUTelWDD3C BHJeXUp55V7LI7ojtqF9gjit6O5WRS7WJQPp X-Google-Smtp-Source: ABdhPJwKFKG79OK/+BhBlKQL1nUI9Hpo1G+c5/bQeae9/+dHmqN51BwnmZJqaAhqMXVKC2Osr2PTbA== X-Received: by 2002:a05:6512:108a:: with SMTP id j10mr4198470lfg.559.1619807581071; Fri, 30 Apr 2021 11:33:01 -0700 (PDT) Received: from mail-lf1-f53.google.com (mail-lf1-f53.google.com. [209.85.167.53]) by smtp.gmail.com with ESMTPSA id q2sm327148ljc.130.2021.04.30.11.33.00 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 30 Apr 2021 11:33:00 -0700 (PDT) Received: by mail-lf1-f53.google.com with SMTP id j10so18369536lfb.12 for ; Fri, 30 Apr 2021 11:33:00 -0700 (PDT) X-Received: by 2002:a19:7504:: with SMTP id y4mr4054152lfe.41.1619807579878; Fri, 30 Apr 2021 11:32:59 -0700 (PDT) MIME-Version: 1.0 References: <20210429225251.02b6386d21b69255b4f6c163@linux-foundation.org> <20210430060049.FrKVS3ZER%akpm@linux-foundation.org> In-Reply-To: <20210430060049.FrKVS3ZER%akpm@linux-foundation.org> From: Linus Torvalds Date: Fri, 30 Apr 2021 11:32:43 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [patch 148/178] kasan: detect false-positives in tests To: Andrew Morton , Stephen Rothwell Cc: Andrey Konovalov , Andrey Ryabinin , Dmitry Vyukov , Marco Elver , Alexander Potapenko , Linux-MM , mm-commits@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Precedence: bulk Reply-To: linux-kernel@vger.kernel.org List-ID: X-Mailing-List: mm-commits@vger.kernel.org On Thu, Apr 29, 2021 at 11:00 PM Andrew Morton wrote: > > From: Andrey Konovalov > 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 89109C433B4 for ; Fri, 30 Apr 2021 18:33:06 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 1E2876141E for ; Fri, 30 Apr 2021 18:33:06 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1E2876141E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=linux-foundation.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 9FD5D6B006C; Fri, 30 Apr 2021 14:33:05 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9D3F86B0070; Fri, 30 Apr 2021 14:33:05 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 874436B0071; Fri, 30 Apr 2021 14:33:05 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0240.hostedemail.com [216.40.44.240]) by kanga.kvack.org (Postfix) with ESMTP id 655CB6B006C for ; Fri, 30 Apr 2021 14:33:05 -0400 (EDT) Received: from smtpin34.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 27889180ACEE8 for ; Fri, 30 Apr 2021 18:33:05 +0000 (UTC) X-FDA: 78089880330.34.25997BA Received: from mail-lj1-f182.google.com (mail-lj1-f182.google.com [209.85.208.182]) by imf27.hostedemail.com (Postfix) with ESMTP id 433FA801A80F for ; Fri, 30 Apr 2021 18:32:38 +0000 (UTC) Received: by mail-lj1-f182.google.com with SMTP id u25so43633992ljg.7 for ; Fri, 30 Apr 2021 11:33:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=P1Q7BrgOtfscywkg5hVce2+3guXdura6NsxZgt2BXPQ=; b=ghUSTWnV/xszGi51+GjhZMwQjv3qdRdMqSYN9LIkqgLV5KPrG+VjnGtbADfK1LmrIp YRwqJBjUp/Uy5HhEoka+hBk5fKZ+mSgxT3bexDrNzdicKrv/nhGO/Nj+ZNYu52WV0KQM TvoSIw5LXjBTwfmuto3eXFfjthwjQMW9AxyII= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=P1Q7BrgOtfscywkg5hVce2+3guXdura6NsxZgt2BXPQ=; b=NON7GXrdb58Wm1KXZIBpGO3JPPP5wI+9hHKl2JCOFpuyCVdVgyBCXRiOsBgLRqWmzH vggiLSGaN/I/FlwTm2J7SXlV1iuqQaXO1ZAiJ9ONc1HyJCMLIvLar5my1SWhJuv4M27l AiO6XxMg/BBWkwCuDJgvHJro3hqdVKjWpd3+c4oyAcZqJIoJDxrM/z2C9rBnvk4+ZgJa 0oRzk9BUCgK/ydnM/E73ITukFdUemYONKOf+LkRXA1wnz88CkPCAOYR5qJ1Ey1dkMDm9 7TlaZ81zAP68r5707A76A1k0JpGlP9/umGxCOV+TflSDUmyZhLOp1wABBv0vnkvXEAOJ xJtA== X-Gm-Message-State: AOAM531jgCYplpnCiaDwwe2PZw/2/F92Dz07CnXA4fx8kzKBQF0DvUbc fb1l9liZoxBrvr8O4rWs3Z2/gIkrRLxvUke3 X-Google-Smtp-Source: ABdhPJyraDSHr5R4Ewekn8kNrIkZ1fSr0C6d5jZrcrZe32iscN7crMDd+C03FbHWMWBpwrrpoN+5cQ== X-Received: by 2002:a2e:1444:: with SMTP id 4mr1253216lju.196.1619807581184; Fri, 30 Apr 2021 11:33:01 -0700 (PDT) Received: from mail-lf1-f51.google.com (mail-lf1-f51.google.com. [209.85.167.51]) by smtp.gmail.com with ESMTPSA id n144sm65566lfa.293.2021.04.30.11.33.00 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 30 Apr 2021 11:33:00 -0700 (PDT) Received: by mail-lf1-f51.google.com with SMTP id x20so81232828lfu.6 for ; Fri, 30 Apr 2021 11:33:00 -0700 (PDT) X-Received: by 2002:a19:7504:: with SMTP id y4mr4054152lfe.41.1619807579878; Fri, 30 Apr 2021 11:32:59 -0700 (PDT) MIME-Version: 1.0 References: <20210429225251.02b6386d21b69255b4f6c163@linux-foundation.org> <20210430060049.FrKVS3ZER%akpm@linux-foundation.org> In-Reply-To: <20210430060049.FrKVS3ZER%akpm@linux-foundation.org> From: Linus Torvalds Date: Fri, 30 Apr 2021 11:32:43 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [patch 148/178] kasan: detect false-positives in tests To: Andrew Morton , Stephen Rothwell Cc: Andrey Konovalov , Andrey Ryabinin , Dmitry Vyukov , Marco Elver , Alexander Potapenko , Linux-MM , mm-commits@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-Stat-Signature: wyfhca8f4r9kg73d7krq9ieioxhuoqud X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 433FA801A80F Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=google header.b=ghUSTWnV; spf=pass (imf27.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.208.182 as permitted sender) smtp.mailfrom=torvalds@linuxfoundation.org; dmarc=none Received-SPF: none (linuxfoundation.org>: No applicable sender policy available) receiver=imf27; identity=mailfrom; envelope-from=""; helo=mail-lj1-f182.google.com; client-ip=209.85.208.182 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1619807558-634681 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Apr 29, 2021 at 11:00 PM Andrew Morton wrote: > > From: Andrey Konovalov > 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