Ira,
On 3/4/21 00:23, kernel test robot wrote:
> Greeting,
>
> FYI, we noticed the following commit (built with gcc-9):
>
> commit: 61b205f579911a11f0b576f73275eca2aed0d108 ("mm/highmem: Convert memcpy_[to|from]_page() to kmap_local_page()")
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
>
>
> in testcase: trinity
> version: trinity-static-i386-x86_64-f93256fb_2019-08-28
> with following parameters:
>
> runtime: 300s
>
> test-description: Trinity is a linux system call fuzz tester.
> test-url: http://codemonkey.org.uk/projects/trinity/
>
>
> on test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 8G
>
> caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
Is the fix for this been posted yet ?
(asking since I didn't see the fix and my mailer is dropping emails from
lkml).
On Tue, Mar 09, 2021 at 08:53:04PM +0000, Chaitanya Kulkarni wrote:
> Ira,
>
> On 3/4/21 00:23, kernel test robot wrote:
> > Greeting,
> >
> > FYI, we noticed the following commit (built with gcc-9):
> >
> > commit: 61b205f579911a11f0b576f73275eca2aed0d108 ("mm/highmem: Convert memcpy_[to|from]_page() to kmap_local_page()")
> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
> >
> >
> > in testcase: trinity
> > version: trinity-static-i386-x86_64-f93256fb_2019-08-28
> > with following parameters:
> >
> > runtime: 300s
> >
> > test-description: Trinity is a linux system call fuzz tester.
> > test-url: http://codemonkey.org.uk/projects/trinity/
> >
> >
> > on test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 8G
> >
> > caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
>
> Is the fix for this been posted yet ?
No. I've been unable to reproduce it yet.
Ira
>
> (asking since I didn't see the fix and my mailer is dropping emails from
> lkml).
Hi Ira,
On Thu, Mar 11, 2021 at 08:02:20AM -0800, Ira Weiny wrote:
> On Tue, Mar 09, 2021 at 08:53:04PM +0000, Chaitanya Kulkarni wrote:
> > Ira,
> >
> > On 3/4/21 00:23, kernel test robot wrote:
> > > Greeting,
> > >
> > > FYI, we noticed the following commit (built with gcc-9):
> > >
> > > commit: 61b205f579911a11f0b576f73275eca2aed0d108 ("mm/highmem: Convert memcpy_[to|from]_page() to kmap_local_page()")
> > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
> > >
> > >
> > > in testcase: trinity
> > > version: trinity-static-i386-x86_64-f93256fb_2019-08-28
> > > with following parameters:
> > >
> > > runtime: 300s
> > >
> > > test-description: Trinity is a linux system call fuzz tester.
> > > test-url: http://codemonkey.org.uk/projects/trinity/
> > >
> > >
> > > on test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 8G
> > >
> > > caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
> >
> > Is the fix for this been posted yet ?
>
> No. I've been unable to reproduce it yet.
just FYI
the issue does not always happen but the rate on 61b205f579 is not low,
while we didn't observe it happen on parent commit.
bb90d4bc7b6a536b 61b205f579911a11f0b576f7327
---------------- ---------------------------
fail:runs %reproduction fail:runs
| | |
:38 16% 6:38 dmesg.EIP:__kmap_local_sched_in
:38 16% 6:38 dmesg.EIP:__kmap_local_sched_out
:38 16% 6:38 dmesg.WARNING:at_mm/highmem.c:#__kmap_local_sched_in
:38 16% 6:38 dmesg.WARNING:at_mm/highmem.c:#__kmap_local_sched_out
also please permit me to quote our internal analysis by Zhengjun (cced)
(Thanks a lot, Zhengjun)
"the commit has the potential to cause the issue.
It replaces " kmap_atomic" to " kmap_local_page".
Most of the two API is the same, except for " kmap_atomic" disable preemption and cannot sleep.
I check the issue happened when there is a preemption, in FBC " kmap_local_page",
the preemption is enabled, the issue may happen."
"
>
> Ira
>
> >
> > (asking since I didn't see the fix and my mailer is dropping emails from
> > lkml).
Adding Thomas. Thomas I've been looking into the following test failure:
https://lore.kernel.org/lkml/20210304083825.GB17830@xsang-OptiPlex-9020/
See below.
On Fri, Mar 12, 2021 at 02:27:55PM +0800, Oliver Sang wrote:
> Hi Ira,
[snip]
> > > >
> > > > caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
> > >
> > > Is the fix for this been posted yet ?
> >
> > No. I've been unable to reproduce it yet.
>
> just FYI
> the issue does not always happen but the rate on 61b205f579 is not low,
> while we didn't observe it happen on parent commit.
>
> bb90d4bc7b6a536b 61b205f579911a11f0b576f7327
> ---------------- ---------------------------
> fail:runs %reproduction fail:runs
> | | |
> :38 16% 6:38 dmesg.EIP:__kmap_local_sched_in
> :38 16% 6:38 dmesg.EIP:__kmap_local_sched_out
> :38 16% 6:38 dmesg.WARNING:at_mm/highmem.c:#__kmap_local_sched_in
> :38 16% 6:38 dmesg.WARNING:at_mm/highmem.c:#__kmap_local_sched_out
>
> also please permit me to quote our internal analysis by Zhengjun (cced)
> (Thanks a lot, Zhengjun)
>
> "the commit has the potential to cause the issue.
> It replaces " kmap_atomic" to " kmap_local_page".
>
> Most of the two API is the same, except for " kmap_atomic" disable preemption and cannot sleep.
> I check the issue happened when there is a preemption, in FBC " kmap_local_page",
> the preemption is enabled, the issue may happen."
> "
I think I see the issue. I think this is an invalid configuration.
00:26:43 > grep DEBUG_KMAP config-5.11.0-rc7-00002-g61b205f57991
CONFIG_DEBUG_KMAP_LOCAL=y
CONFIG_DEBUG_KMAP_LOCAL_FORCE_MAP=y
00:26:48 > grep DEBUG_HIGHMEM config-5.11.0-rc7-00002-g61b205f57991
# CONFIG_DEBUG_HIGHMEM is not set
DEBUG_KMAP_LOCAL causes guard pages to be added to the kmap_ctrl array. But
DEBUG_HIGHMEM is used in __kmap_local_sched_out() to check the guard pages.
DEBUG_HIGHMEM is supposed to select DEBUG_KMAP_LOCAL... but apparently that
did not happen when this configuration was made.
I still have not hit this condition in my testing. Could you ensure that
DEBUG_HIGMEM is set and rerun the test to see if I am correct?
Thomas wouldn't the following enable checks make more sense? Or perhaps be
more consistent with the processing of kmap_ctrl?
Ira
diff --git a/mm/highmem.c b/mm/highmem.c
index 86f2b9495f9c..6ef8f5e05e7e 100644
--- a/mm/highmem.c
+++ b/mm/highmem.c
@@ -618,7 +618,7 @@ void __kmap_local_sched_out(void)
int idx;
/* With debug all even slots are unmapped and act as guard */
- if (IS_ENABLED(CONFIG_DEBUG_HIGHMEM) && !(i & 0x01)) {
+ if (IS_ENABLED(CONFIG_DEBUG_KMAP_LOCAL) && !(i & 0x01)) {
WARN_ON_ONCE(!pte_none(pteval));
continue;
}
@@ -654,7 +654,7 @@ void __kmap_local_sched_in(void)
int idx;
/* With debug all even slots are unmapped and act as guard */
- if (IS_ENABLED(CONFIG_DEBUG_HIGHMEM) && !(i & 0x01)) {
+ if (IS_ENABLED(CONFIG_DEBUG_KMAP_LOCAL) && !(i & 0x01)) {
WARN_ON_ONCE(!pte_none(pteval));
continue;
}
On Tue, Mar 16 2021 at 00:37, Ira Weiny wrote:
>
> I think I see the issue. I think this is an invalid configuration.
>
> 00:26:43 > grep DEBUG_KMAP config-5.11.0-rc7-00002-g61b205f57991
> CONFIG_DEBUG_KMAP_LOCAL=y
> CONFIG_DEBUG_KMAP_LOCAL_FORCE_MAP=y
>
> 00:26:48 > grep DEBUG_HIGHMEM config-5.11.0-rc7-00002-g61b205f57991
> # CONFIG_DEBUG_HIGHMEM is not set
No. It's valid so you can test that crap on 64bit.
> DEBUG_KMAP_LOCAL causes guard pages to be added to the kmap_ctrl array. But
> DEBUG_HIGHMEM is used in __kmap_local_sched_out() to check the guard pages.
>
> DEBUG_HIGHMEM is supposed to select DEBUG_KMAP_LOCAL... but apparently that
> did not happen when this configuration was made.
>
> I still have not hit this condition in my testing. Could you ensure that
> DEBUG_HIGMEM is set and rerun the test to see if I am correct?
Why? The whole point of CONFIG_DEBUG_KMAP_LOCAL_FORCE_MAP is to allow
testing of this kmap_local/atomic stuff w/o HIGHMEM, e.g. on 64 bit.
> Thomas wouldn't the following enable checks make more sense? Or perhaps be
> more consistent with the processing of kmap_ctrl?
Neither nor. It's simply required to make DEBUG_KMAP_LOCAL=y &&
HIGHMEM=n case work.
Please add:
Fixes: 0e91a0c6984c ("mm/highmem: Provide CONFIG_DEBUG_KMAP_LOCAL_FORCE_MAP")
which should have contained exactly that change:
> - if (IS_ENABLED(CONFIG_DEBUG_HIGHMEM) && !(i & 0x01)) {
> + if (IS_ENABLED(CONFIG_DEBUG_KMAP_LOCAL) && !(i & 0x01)) {
> WARN_ON_ONCE(!pte_none(pteval));
> continue;
> }
> @@ -654,7 +654,7 @@ void __kmap_local_sched_in(void)
> int idx;
>
> /* With debug all even slots are unmapped and act as guard */
> - if (IS_ENABLED(CONFIG_DEBUG_HIGHMEM) && !(i & 0x01)) {
> + if (IS_ENABLED(CONFIG_DEBUG_KMAP_LOCAL) && !(i & 0x01)) {
> WARN_ON_ONCE(!pte_none(pteval));
> continue;
> }
Thanks,
tglx
On Wed, Mar 17, 2021 at 02:44:41PM +0100, Thomas Gleixner wrote:
> On Tue, Mar 16 2021 at 00:37, Ira Weiny wrote:
> >
> > I think I see the issue. I think this is an invalid configuration.
> >
> > 00:26:43 > grep DEBUG_KMAP config-5.11.0-rc7-00002-g61b205f57991
> > CONFIG_DEBUG_KMAP_LOCAL=y
> > CONFIG_DEBUG_KMAP_LOCAL_FORCE_MAP=y
> >
> > 00:26:48 > grep DEBUG_HIGHMEM config-5.11.0-rc7-00002-g61b205f57991
> > # CONFIG_DEBUG_HIGHMEM is not set
>
> No. It's valid so you can test that crap on 64bit.
>
> > DEBUG_KMAP_LOCAL causes guard pages to be added to the kmap_ctrl array. But
> > DEBUG_HIGHMEM is used in __kmap_local_sched_out() to check the guard pages.
> >
> > DEBUG_HIGHMEM is supposed to select DEBUG_KMAP_LOCAL... but apparently that
> > did not happen when this configuration was made.
> >
> > I still have not hit this condition in my testing. Could you ensure that
> > DEBUG_HIGMEM is set and rerun the test to see if I am correct?
>
> Why? The whole point of CONFIG_DEBUG_KMAP_LOCAL_FORCE_MAP is to allow
> testing of this kmap_local/atomic stuff w/o HIGHMEM, e.g. on 64 bit.
Ok yea I was looking at this backwards.
>
> > Thomas wouldn't the following enable checks make more sense? Or perhaps be
> > more consistent with the processing of kmap_ctrl?
>
> Neither nor. It's simply required to make DEBUG_KMAP_LOCAL=y &&
> HIGHMEM=n case work.
>
> Please add:
> Fixes: 0e91a0c6984c ("mm/highmem: Provide CONFIG_DEBUG_KMAP_LOCAL_FORCE_MAP")
>
Patch sent.
Thanks for setting me straight,
Ira