All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marco Elver <elver@google.com>
To: Kefeng Wang <wangkefeng.wang@huawei.com>
Cc: Russell King <linux@armlinux.org.uk>,
	Alexander Potapenko <glider@google.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 0/4] ARM: Support KFENCE feature
Date: Wed, 25 Aug 2021 12:14:40 +0200	[thread overview]
Message-ID: <CANpmjNMnU5P9xsDhgeBKQR7Tg-3cHPkMNx7906yYwEAj85sNWg@mail.gmail.com> (raw)
In-Reply-To: <20210825092116.149975-1-wangkefeng.wang@huawei.com>

On Wed, 25 Aug 2021 at 11:17, Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
> The patch 1~3 is to support KFENCE feature on ARM.
>
> NOTE:
> The context of patch2/3 changes in arch/arm/mm/fault.c is based on link[1],
> which make some refactor and cleanup about page fault.
>
> kfence_test is not useful when kfence is not enabled, skip kfence test
> when kfence not enabled in patch4.
>
> I tested the kfence_test on ARM QEMU with or without ARM_LPAE and all passed.

Thank you for enabling KFENCE on ARM -- I'll leave arch-code review to
an ARM maintainer.

However, as said on the patch, please drop the change to the
kfence_test and associated changes. This is working as intended; while
you claim that it takes a long time to run when disabled, when running
manually you just should not run it when disabled. There are CI
systems that rely on the KUnit test output and the fact that the
various test cases say "not ok" etc. Changing that would mean such CI
systems would no longer fail if KFENCE was accidentally disabled (once
KFENCE is enabled on various CI, which we'd like to do at some point).
There are ways to fail the test faster, but they all complicate the
test for no good reason. (And the addition of a new exported function
that is essentially useless.)

Thanks,
-- Marco

WARNING: multiple messages have this Message-ID (diff)
From: Marco Elver <elver@google.com>
To: Kefeng Wang <wangkefeng.wang@huawei.com>
Cc: Russell King <linux@armlinux.org.uk>,
	Alexander Potapenko <glider@google.com>,
	 Dmitry Vyukov <dvyukov@google.com>,
	linux-arm-kernel@lists.infradead.org,
	 linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com,
	 Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 0/4] ARM: Support KFENCE feature
Date: Wed, 25 Aug 2021 12:14:40 +0200	[thread overview]
Message-ID: <CANpmjNMnU5P9xsDhgeBKQR7Tg-3cHPkMNx7906yYwEAj85sNWg@mail.gmail.com> (raw)
In-Reply-To: <20210825092116.149975-1-wangkefeng.wang@huawei.com>

On Wed, 25 Aug 2021 at 11:17, Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
> The patch 1~3 is to support KFENCE feature on ARM.
>
> NOTE:
> The context of patch2/3 changes in arch/arm/mm/fault.c is based on link[1],
> which make some refactor and cleanup about page fault.
>
> kfence_test is not useful when kfence is not enabled, skip kfence test
> when kfence not enabled in patch4.
>
> I tested the kfence_test on ARM QEMU with or without ARM_LPAE and all passed.

Thank you for enabling KFENCE on ARM -- I'll leave arch-code review to
an ARM maintainer.

However, as said on the patch, please drop the change to the
kfence_test and associated changes. This is working as intended; while
you claim that it takes a long time to run when disabled, when running
manually you just should not run it when disabled. There are CI
systems that rely on the KUnit test output and the fact that the
various test cases say "not ok" etc. Changing that would mean such CI
systems would no longer fail if KFENCE was accidentally disabled (once
KFENCE is enabled on various CI, which we'd like to do at some point).
There are ways to fail the test faster, but they all complicate the
test for no good reason. (And the addition of a new exported function
that is essentially useless.)

Thanks,
-- Marco

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2021-08-25 10:14 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-25  9:21 [PATCH 0/4] ARM: Support KFENCE feature Kefeng Wang
2021-08-25  9:21 ` Kefeng Wang
2021-08-25  9:21 ` [PATCH 1/4] ARM: mm: Provide set_memory_valid() Kefeng Wang
2021-08-25  9:21   ` Kefeng Wang
2021-08-25  9:21 ` [PATCH 2/4] ARM: mm: Provide is_write_fault() Kefeng Wang
2021-08-25  9:21   ` Kefeng Wang
2021-08-25  9:21 ` [PATCH 3/4] ARM: Support KFENCE for ARM Kefeng Wang
2021-08-25  9:21   ` Kefeng Wang
2021-08-25 13:18   ` ownia
2021-08-25 13:18     ` ownia
2021-08-25 14:31     ` Kefeng Wang
2021-08-25 14:31       ` Kefeng Wang
2021-08-25  9:21 ` [PATCH 4/4] mm: kfence: Only load kfence_test when kfence is enabled Kefeng Wang
2021-08-25  9:21   ` Kefeng Wang
2021-08-25  9:31   ` Alexander Potapenko
2021-08-25  9:31     ` Alexander Potapenko
2021-08-25  9:54     ` Marco Elver
2021-08-25  9:54       ` Marco Elver
2021-08-25  9:55     ` Kefeng Wang
2021-08-25  9:55       ` Kefeng Wang
2021-08-25  9:59       ` Marco Elver
2021-08-25  9:59         ` Marco Elver
2021-08-25 10:14 ` Marco Elver [this message]
2021-08-25 10:14   ` [PATCH 0/4] ARM: Support KFENCE feature Marco Elver
2021-08-25 10:57   ` Marco Elver
2021-08-25 10:57     ` Marco Elver
2021-08-25 14:15     ` Kefeng Wang
2021-08-25 14:15       ` Kefeng Wang
2021-08-25 14:28     ` Kefeng Wang
2021-08-25 14:28       ` Kefeng Wang

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=CANpmjNMnU5P9xsDhgeBKQR7Tg-3cHPkMNx7906yYwEAj85sNWg@mail.gmail.com \
    --to=elver@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=dvyukov@google.com \
    --cc=glider@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=wangkefeng.wang@huawei.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.