public inbox for linux-parisc@vger.kernel.org
 help / color / mirror / Atom feed
* Regression with kernel 6.3 "kernel BUG at include/linux/swapops.h:472!"
@ 2023-05-10 17:56 Christoph Biedl
  2023-05-10 20:29 ` Helge Deller
  0 siblings, 1 reply; 11+ messages in thread
From: Christoph Biedl @ 2023-05-10 17:56 UTC (permalink / raw)
  To: linux-parisc

[-- Attachment #1: Type: text/plain, Size: 8307 bytes --]

Greetings,

after upgrading from 6.2 to 6.3, my old HPPA box becomes inresponsive
after 10-30 minutes, possibly with some memory pressure involved
(building some Debian packages), kernel messages below. This is 100%
reproducible, switching back to some 6.2 kernel is a workaround.

Userland is Debian unstable, the affected process (as far as I could
see) is either ntpd or ntpq. A reboot is needed to resolve the
situation.

As this still happens with 6.3.2-rc1 which includes "parisc: Ensure page
alignment in flush functions": Has this been spotted earlier, is there
some tentative fix somewhere?

So far, I haven't tried bisecting between 6.2 and 6.3 yet. While I'm
able to do this, it would take days since I don't have a quick
reproducer, and I don't want to do this work in vain. If you can
provide hints how to speed up this, let me know.

Regards,

    Christoph

[  828.356408] kernel BUG at include/linux/swapops.h:472!
[  828.361679] CPU: 0 PID: 7119 Comm: ntpq Not tainted 6.3.1 #1
[  828.367392] Hardware name: 9000/785/C3600
[  828.371440] 
[  828.372962]      YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI
[  828.377695] PSW: 00000000000001001111011100001111 Not tainted
[  828.383483] r00-03  0004f70f 11294f90 105c6408 31578540
[  828.388744] r04-07  00000c73 114157c0 00000000 f66326c7
[  828.394004] r08-11  4f855a58 31455c60 31455c60 00004000
[  828.399257] r12-15  00000873 00000006 4f855530 4f855530
[  828.404528] r16-19  00000002 00000100 31455c60 00000000
[  828.409789] r20-23  70000000 0000001b 0098b000 4f855a58
[  828.415060] r24-27  2970de30 315aa8c8 00000000 10e72f90
[  828.420325] r28-31  00000000 129ecf94 315785c0 129ecf00
[  828.425578] sr00-03  00000029 00000000 00000000 00000029
[  828.430925] sr04-07  00000000 00000000 00000000 00000000
[  828.436265] 
[  828.437780] IASQ: 00000000 00000000 IAOQ: 10559284 10559288
[  828.443389]  IIR: 03ffe01f    ISR: 10240085  IOR: 5e178584
[  828.448900]  CPU:        0   CR30: 4f855530 CR31: 11111111
[  828.454422]  ORIG_R28: 00000006
[  828.457594]  IAOQ[0]: migration_entry_wait_on_locked+0x254/0x274
[  828.463663]  IAOQ[1]: migration_entry_wait_on_locked+0x258/0x274
[  828.469701]  RP(r2): __migration_entry_wait+0x64/0x6c
[  828.474807] Backtrace:
[  828.477195]  [<105c6408>] __migration_entry_wait+0x64/0x6c
[  828.482732]  [<105c6444>] migration_entry_wait+0x34/0x44
[  828.488085]  [<1058c608>] do_swap_page+0x710/0x894
[  828.492926]  [<1058d07c>] handle_mm_fault+0x4e4/0xcb4
[  828.498009]  [<10406490>] do_page_fault+0xd0/0x42c
[  828.502852]  [<10408d60>] handle_interruption+0x4cc/0x6c8
[  828.508303]  [<10403070>] intr_check_sig+0x0/0x38
[  828.513051] 
[  828.514568] CPU: 0 PID: 7119 Comm: ntpq Not tainted 6.3.1 #1
[  828.520245] Hardware name: 9000/785/C3600
[  828.524269] Backtrace:
[  828.526651]  [<104084e0>] show_stack+0x34/0x48
[  828.531124]  [<10cdb894>] dump_stack_lvl+0x38/0x54
[  828.535953]  [<10cdb8cc>] dump_stack+0x1c/0x2c
[  828.540424]  [<104085fc>] die_if_kernel+0xec/0x1b0
[  828.545241]  [<10408eac>] handle_interruption+0x618/0x6c8
[  828.550668]  [<10403070>] intr_check_sig+0x0/0x38
[  828.555399] 
[  828.556936] ---[ end trace 0000000000000000 ]---
[  828.562457] ------------[ cut here ]------------
[  828.567151] kernel BUG at include/linux/swapops.h:472!
[  828.572394] CPU: 0 PID: 3059 Comm: in:imklog Tainted: G      D            6.3.1 #1
[  828.579997] Hardware name: 9000/785/C3600
[  828.584045] 
[  828.585567]      YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI
[  828.590306] PSW: 00000000000001001111011100001111 Tainted: G      D           
[  828.597546] r00-03  0004f70f 11294f90 105c6408 13030540
[  828.602837] r04-07  00000c73 114157c0 00000000 f5a32bef
[  828.608094] r08-11  13035a58 137df5e8 137df5e8 00004000
[  828.613363] r12-15  00000873 00000006 13035530 13035530
[  828.618623] r16-19  00000002 00000100 137df5e8 00000000
[  828.623885] r20-23  70000000 0000001b 0000000e 13035a58
[  828.629144] r24-27  1472c1f0 4fbdf8c8 00000000 10e72f90
[  828.634412] r28-31  00000000 1373b094 130305c0 1373b000
[  828.639667] sr00-03  00000029 00000000 00000000 0000000e
[  828.645014] sr04-07  00000000 00000000 00000000 00000000
[  828.650361] 
[  828.651874] IASQ: 00000000 00000000 IAOQ: 10559284 10559288
[  828.657476]  IIR: 03ffe01f    ISR: 00000000  IOR: 00000000
[  828.662997]  CPU:        0   CR30: 13035530 CR31: 11111111
[  828.668510]  ORIG_R28: 00000000
[  828.671688]  IAOQ[0]: migration_entry_wait_on_locked+0x254/0x274
[  828.677739]  IAOQ[1]: migration_entry_wait_on_locked+0x258/0x274
[  828.683788]  RP(r2): __migration_entry_wait+0x64/0x6c
[  828.688888] Backtrace:
[  828.691289]  [<105c6408>] __migration_entry_wait+0x64/0x6c
[  828.696809]  [<105c6444>] migration_entry_wait+0x34/0x44
[  828.702162]  [<1058c608>] do_swap_page+0x710/0x894
[  828.706993]  [<1058d07c>] handle_mm_fault+0x4e4/0xcb4
[  828.712087]  [<10406490>] do_page_fault+0xd0/0x42c
[  828.716924]  [<10408d60>] handle_interruption+0x4cc/0x6c8
[  828.722375]  [<10403070>] intr_check_sig+0x0/0x38
[  828.727116] 
[  828.728639] CPU: 0 PID: 3059 Comm: in:imklog Tainted: G      D            6.3.1 #1
[  828.736223] Hardware name: 9000/785/C3600
[  828.740252] Backtrace:
[  828.742627]  [<104084e0>] show_stack+0x34/0x48
[  828.747099]  [<10cdb894>] dump_stack_lvl+0x38/0x54
[  828.751928]  [<10cdb8cc>] dump_stack+0x1c/0x2c
[  828.756399]  [<104085fc>] die_if_kernel+0xec/0x1b0
[  828.761218]  [<10408eac>] handle_interruption+0x618/0x6c8
[  828.766643]  [<10403070>] intr_check_sig+0x0/0x38
[  828.771376] 
[  828.772915] ---[ end trace 0000000000000000 ]---
[  828.810379] ------------[ cut here ]------------
[  828.815050] kernel BUG at include/linux/swapops.h:472!
[  828.820241] CPU: 0 PID: 261 Comm: systemd-journal Tainted: G      D            6.3.1 #1
[  828.828273] Hardware name: 9000/785/C3600
[  828.832339] 
[  828.833861]      YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI
[  828.838585] PSW: 00000000000001001111111100001111 Tainted: G      D           
[  828.845841] r00-03  0004ff0f 11294f90 105c6408 4f830540
[  828.851108] r04-07  00000c73 11415fe8 00000002 f69b63af
[  828.856362] r08-11  4f857698 12951168 12951168 00004000
[  828.861630] r12-15  00000873 00000006 4f857170 4f857170
[  828.866884] r16-19  00000002 00000100 12951168 00000000
[  828.872151] r20-23  70000000 0000001b 0000000e 4f857698
[  828.877406] r24-27  129621f0 130726d8 00000000 10e72f90
[  828.882674] r28-31  00000000 4f8c7898 4f8305c0 4f8c7800
[  828.887925] sr00-03  00000029 00000000 00000000 00000004
[  828.893274] sr04-07  00000000 00000000 00000000 00000000
[  828.898612] 
[  828.900137] IASQ: 00000000 00000000 IAOQ: 10559284 10559288
[  828.905736]  IIR: 03ffe01f    ISR: 102400fe  IOR: 0c030584
[  828.911258]  CPU:        0   CR30: 4f857170 CR31: 11111111
[  828.916768]  ORIG_R28: 00000006
[  828.919940]  IAOQ[0]: migration_entry_wait_on_locked+0x254/0x274
[  828.925999]  IAOQ[1]: migration_entry_wait_on_locked+0x258/0x274
[  828.932047]  RP(r2): __migration_entry_wait+0x64/0x6c
[  828.937136] Backtrace:
[  828.939530]  [<105c6408>] __migration_entry_wait+0x64/0x6c
[  828.945059]  [<105c6444>] migration_entry_wait+0x34/0x44
[  828.950414]  [<1058c608>] do_swap_page+0x710/0x894
[  828.955242]  [<1058d07c>] handle_mm_fault+0x4e4/0xcb4
[  828.960336]  [<10406490>] do_page_fault+0xd0/0x42c
[  828.965167]  [<10408d60>] handle_interruption+0x4cc/0x6c8
[  828.970618]  [<10403070>] intr_check_sig+0x0/0x38
[  828.975358] 
[  828.976880] CPU: 0 PID: 261 Comm: systemd-journal Tainted: G      D            6.3.1 #1
[  828.984897] Hardware name: 9000/785/C3600
[  828.988929] Backtrace:
[  828.991311]  [<104084e0>] show_stack+0x34/0x48
[  828.995783]  [<10cdb894>] dump_stack_lvl+0x38/0x54
[  829.000613]  [<10cdb8cc>] dump_stack+0x1c/0x2c
[  829.005084]  [<104085fc>] die_if_kernel+0xec/0x1b0
[  829.009901]  [<10408eac>] handle_interruption+0x618/0x6c8
[  829.015328]  [<10403070>] intr_check_sig+0x0/0x38
[  829.020059] 
[  829.021590] ---[ end trace 0000000000000000 ]---
[  829.304173] ------------[ cut here ]------------
[  829.308873] kernel BUG at include/linux/swapops.h:472!
[  829.314076] die_if_kernel() recursion detected.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Regression with kernel 6.3 "kernel BUG at include/linux/swapops.h:472!"
  2023-05-10 17:56 Regression with kernel 6.3 "kernel BUG at include/linux/swapops.h:472!" Christoph Biedl
@ 2023-05-10 20:29 ` Helge Deller
  2023-05-11 17:22   ` Christoph Biedl
  0 siblings, 1 reply; 11+ messages in thread
From: Helge Deller @ 2023-05-10 20:29 UTC (permalink / raw)
  To: Christoph Biedl, linux-parisc

Hi Christoph,

On 5/10/23 19:56, Christoph Biedl wrote:
> after upgrading from 6.2 to 6.3, my old HPPA box becomes inresponsive
> after 10-30 minutes, possibly with some memory pressure involved
> (building some Debian packages), kernel messages below. This is 100%
> reproducible, switching back to some 6.2 kernel is a workaround.
>
> Userland is Debian unstable, the affected process (as far as I could
> see) is either ntpd or ntpq. A reboot is needed to resolve the
> situation.
>
> As this still happens with 6.3.2-rc1 which includes "parisc: Ensure page
> alignment in flush functions": Has this been spotted earlier, is there
> some tentative fix somewhere?
>
> So far, I haven't tried bisecting between 6.2 and 6.3 yet. While I'm
> able to do this, it would take days since I don't have a quick
> reproducer, and I don't want to do this work in vain. If you can
> provide hints how to speed up this, let me know.

I haven't used kernel 6.3 much yet, but the kernel BUG below seems
to be triggered by CONFIG_MIGRATION.
You could try to disable that config option first to verify if
it fixes your crash.
This might help to narrow down the problem....

Helge

> [  828.356408] kernel BUG at include/linux/swapops.h:472!
> [  828.361679] CPU: 0 PID: 7119 Comm: ntpq Not tainted 6.3.1 #1
> [  828.367392] Hardware name: 9000/785/C3600
> [  828.371440]
> [  828.372962]      YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI
> [  828.377695] PSW: 00000000000001001111011100001111 Not tainted
> [  828.383483] r00-03  0004f70f 11294f90 105c6408 31578540
> [  828.388744] r04-07  00000c73 114157c0 00000000 f66326c7
> [  828.394004] r08-11  4f855a58 31455c60 31455c60 00004000
> [  828.399257] r12-15  00000873 00000006 4f855530 4f855530
> [  828.404528] r16-19  00000002 00000100 31455c60 00000000
> [  828.409789] r20-23  70000000 0000001b 0098b000 4f855a58
> [  828.415060] r24-27  2970de30 315aa8c8 00000000 10e72f90
> [  828.420325] r28-31  00000000 129ecf94 315785c0 129ecf00
> [  828.425578] sr00-03  00000029 00000000 00000000 00000029
> [  828.430925] sr04-07  00000000 00000000 00000000 00000000
> [  828.436265]
> [  828.437780] IASQ: 00000000 00000000 IAOQ: 10559284 10559288
> [  828.443389]  IIR: 03ffe01f    ISR: 10240085  IOR: 5e178584
> [  828.448900]  CPU:        0   CR30: 4f855530 CR31: 11111111
> [  828.454422]  ORIG_R28: 00000006
> [  828.457594]  IAOQ[0]: migration_entry_wait_on_locked+0x254/0x274
> [  828.463663]  IAOQ[1]: migration_entry_wait_on_locked+0x258/0x274
> [  828.469701]  RP(r2): __migration_entry_wait+0x64/0x6c
> [  828.474807] Backtrace:
> [  828.477195]  [<105c6408>] __migration_entry_wait+0x64/0x6c
> [  828.482732]  [<105c6444>] migration_entry_wait+0x34/0x44
> [  828.488085]  [<1058c608>] do_swap_page+0x710/0x894
> [  828.492926]  [<1058d07c>] handle_mm_fault+0x4e4/0xcb4
> [  828.498009]  [<10406490>] do_page_fault+0xd0/0x42c
> [  828.502852]  [<10408d60>] handle_interruption+0x4cc/0x6c8
> [  828.508303]  [<10403070>] intr_check_sig+0x0/0x38
> [  828.513051]
> [  828.514568] CPU: 0 PID: 7119 Comm: ntpq Not tainted 6.3.1 #1
> [  828.520245] Hardware name: 9000/785/C3600
> [  828.524269] Backtrace:
> [  828.526651]  [<104084e0>] show_stack+0x34/0x48
> [  828.531124]  [<10cdb894>] dump_stack_lvl+0x38/0x54
> [  828.535953]  [<10cdb8cc>] dump_stack+0x1c/0x2c
> [  828.540424]  [<104085fc>] die_if_kernel+0xec/0x1b0
> [  828.545241]  [<10408eac>] handle_interruption+0x618/0x6c8
> [  828.550668]  [<10403070>] intr_check_sig+0x0/0x38
> [  828.555399]
> [  828.556936] ---[ end trace 0000000000000000 ]---
> [  828.562457] ------------[ cut here ]------------
> [  828.567151] kernel BUG at include/linux/swapops.h:472!
> [  828.572394] CPU: 0 PID: 3059 Comm: in:imklog Tainted: G      D            6.3.1 #1
> [  828.579997] Hardware name: 9000/785/C3600
> [  828.584045]
> [  828.585567]      YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI
> [  828.590306] PSW: 00000000000001001111011100001111 Tainted: G      D
> [  828.597546] r00-03  0004f70f 11294f90 105c6408 13030540
> [  828.602837] r04-07  00000c73 114157c0 00000000 f5a32bef
> [  828.608094] r08-11  13035a58 137df5e8 137df5e8 00004000
> [  828.613363] r12-15  00000873 00000006 13035530 13035530
> [  828.618623] r16-19  00000002 00000100 137df5e8 00000000
> [  828.623885] r20-23  70000000 0000001b 0000000e 13035a58
> [  828.629144] r24-27  1472c1f0 4fbdf8c8 00000000 10e72f90
> [  828.634412] r28-31  00000000 1373b094 130305c0 1373b000
> [  828.639667] sr00-03  00000029 00000000 00000000 0000000e
> [  828.645014] sr04-07  00000000 00000000 00000000 00000000
> [  828.650361]
> [  828.651874] IASQ: 00000000 00000000 IAOQ: 10559284 10559288
> [  828.657476]  IIR: 03ffe01f    ISR: 00000000  IOR: 00000000
> [  828.662997]  CPU:        0   CR30: 13035530 CR31: 11111111
> [  828.668510]  ORIG_R28: 00000000
> [  828.671688]  IAOQ[0]: migration_entry_wait_on_locked+0x254/0x274
> [  828.677739]  IAOQ[1]: migration_entry_wait_on_locked+0x258/0x274
> [  828.683788]  RP(r2): __migration_entry_wait+0x64/0x6c
> [  828.688888] Backtrace:
> [  828.691289]  [<105c6408>] __migration_entry_wait+0x64/0x6c
> [  828.696809]  [<105c6444>] migration_entry_wait+0x34/0x44
> [  828.702162]  [<1058c608>] do_swap_page+0x710/0x894
> [  828.706993]  [<1058d07c>] handle_mm_fault+0x4e4/0xcb4
> [  828.712087]  [<10406490>] do_page_fault+0xd0/0x42c
> [  828.716924]  [<10408d60>] handle_interruption+0x4cc/0x6c8
> [  828.722375]  [<10403070>] intr_check_sig+0x0/0x38
> [  828.727116]
> [  828.728639] CPU: 0 PID: 3059 Comm: in:imklog Tainted: G      D            6.3.1 #1
> [  828.736223] Hardware name: 9000/785/C3600
> [  828.740252] Backtrace:
> [  828.742627]  [<104084e0>] show_stack+0x34/0x48
> [  828.747099]  [<10cdb894>] dump_stack_lvl+0x38/0x54
> [  828.751928]  [<10cdb8cc>] dump_stack+0x1c/0x2c
> [  828.756399]  [<104085fc>] die_if_kernel+0xec/0x1b0
> [  828.761218]  [<10408eac>] handle_interruption+0x618/0x6c8
> [  828.766643]  [<10403070>] intr_check_sig+0x0/0x38
> [  828.771376]
> [  828.772915] ---[ end trace 0000000000000000 ]---
> [  828.810379] ------------[ cut here ]------------
> [  828.815050] kernel BUG at include/linux/swapops.h:472!
> [  828.820241] CPU: 0 PID: 261 Comm: systemd-journal Tainted: G      D            6.3.1 #1
> [  828.828273] Hardware name: 9000/785/C3600
> [  828.832339]
> [  828.833861]      YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI
> [  828.838585] PSW: 00000000000001001111111100001111 Tainted: G      D
> [  828.845841] r00-03  0004ff0f 11294f90 105c6408 4f830540
> [  828.851108] r04-07  00000c73 11415fe8 00000002 f69b63af
> [  828.856362] r08-11  4f857698 12951168 12951168 00004000
> [  828.861630] r12-15  00000873 00000006 4f857170 4f857170
> [  828.866884] r16-19  00000002 00000100 12951168 00000000
> [  828.872151] r20-23  70000000 0000001b 0000000e 4f857698
> [  828.877406] r24-27  129621f0 130726d8 00000000 10e72f90
> [  828.882674] r28-31  00000000 4f8c7898 4f8305c0 4f8c7800
> [  828.887925] sr00-03  00000029 00000000 00000000 00000004
> [  828.893274] sr04-07  00000000 00000000 00000000 00000000
> [  828.898612]
> [  828.900137] IASQ: 00000000 00000000 IAOQ: 10559284 10559288
> [  828.905736]  IIR: 03ffe01f    ISR: 102400fe  IOR: 0c030584
> [  828.911258]  CPU:        0   CR30: 4f857170 CR31: 11111111
> [  828.916768]  ORIG_R28: 00000006
> [  828.919940]  IAOQ[0]: migration_entry_wait_on_locked+0x254/0x274
> [  828.925999]  IAOQ[1]: migration_entry_wait_on_locked+0x258/0x274
> [  828.932047]  RP(r2): __migration_entry_wait+0x64/0x6c
> [  828.937136] Backtrace:
> [  828.939530]  [<105c6408>] __migration_entry_wait+0x64/0x6c
> [  828.945059]  [<105c6444>] migration_entry_wait+0x34/0x44
> [  828.950414]  [<1058c608>] do_swap_page+0x710/0x894
> [  828.955242]  [<1058d07c>] handle_mm_fault+0x4e4/0xcb4
> [  828.960336]  [<10406490>] do_page_fault+0xd0/0x42c
> [  828.965167]  [<10408d60>] handle_interruption+0x4cc/0x6c8
> [  828.970618]  [<10403070>] intr_check_sig+0x0/0x38
> [  828.975358]
> [  828.976880] CPU: 0 PID: 261 Comm: systemd-journal Tainted: G      D            6.3.1 #1
> [  828.984897] Hardware name: 9000/785/C3600
> [  828.988929] Backtrace:
> [  828.991311]  [<104084e0>] show_stack+0x34/0x48
> [  828.995783]  [<10cdb894>] dump_stack_lvl+0x38/0x54
> [  829.000613]  [<10cdb8cc>] dump_stack+0x1c/0x2c
> [  829.005084]  [<104085fc>] die_if_kernel+0xec/0x1b0
> [  829.009901]  [<10408eac>] handle_interruption+0x618/0x6c8
> [  829.015328]  [<10403070>] intr_check_sig+0x0/0x38
> [  829.020059]
> [  829.021590] ---[ end trace 0000000000000000 ]---
> [  829.304173] ------------[ cut here ]------------
> [  829.308873] kernel BUG at include/linux/swapops.h:472!
> [  829.314076] die_if_kernel() recursion detected.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Regression with kernel 6.3 "kernel BUG at include/linux/swapops.h:472!"
  2023-05-10 20:29 ` Helge Deller
@ 2023-05-11 17:22   ` Christoph Biedl
  2023-05-11 17:35     ` Helge Deller
  0 siblings, 1 reply; 11+ messages in thread
From: Christoph Biedl @ 2023-05-11 17:22 UTC (permalink / raw)
  To: linux-parisc

[-- Attachment #1: Type: text/plain, Size: 642 bytes --]

Helge Deller wrote...

> I haven't used kernel 6.3 much yet, but the kernel BUG below seems
> to be triggered by CONFIG_MIGRATION.
> You could try to disable that config option first to verify if
> it fixes your crash.
> This might help to narrow down the problem....

Looks good, still have a running system after 45 minutes, never got that
far with the initial kernel configuration.

In the meantime I realized only some 16 commits between v6.2 and v6.3
affect arch/parisc. Do you think it's worth check right around those?
Also, if you can think of a way to trigger the crashes, that would ease
the testing a lot.

Regards,

    Christoph

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Regression with kernel 6.3 "kernel BUG at include/linux/swapops.h:472!"
  2023-05-11 17:22   ` Christoph Biedl
@ 2023-05-11 17:35     ` Helge Deller
  2023-05-12 21:56       ` Christoph Biedl
  0 siblings, 1 reply; 11+ messages in thread
From: Helge Deller @ 2023-05-11 17:35 UTC (permalink / raw)
  To: Christoph Biedl, linux-parisc

On 5/11/23 19:22, Christoph Biedl wrote:
> Helge Deller wrote...
>
>> I haven't used kernel 6.3 much yet, but the kernel BUG below seems
>> to be triggered by CONFIG_MIGRATION.
>> You could try to disable that config option first to verify if
>> it fixes your crash.
>> This might help to narrow down the problem....
>
> Looks good, still have a running system after 45 minutes, never got that
> far with the initial kernel configuration.

Good!

> In the meantime I realized only some 16 commits between v6.2 and v6.3
> affect arch/parisc. Do you think it's worth check right around those?

Don't think so.
Very unlikely is this one:
commit	88d7b12068b95731c280af8ce88e8ee9561f96de
highmem: round down the address passed to kunmap_flush_on_unmap()

> Also, if you can think of a way to trigger the crashes, that would ease
> the testing a lot.

Since you run the 32-bit kernel, huge-pages are not involved as they
aren't available in the 32-bit kernels.
So I think swapping is triggering it.
You could try to find a test program which triggers swapping, e.g. LTS testcases?
Another test could be to enable CONFIG_MIGRATION again and disable
all swap spaces and see if it survives.

Helge

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Regression with kernel 6.3 "kernel BUG at include/linux/swapops.h:472!"
  2023-05-11 17:35     ` Helge Deller
@ 2023-05-12 21:56       ` Christoph Biedl
  2023-05-13 12:10         ` Helge Deller
  0 siblings, 1 reply; 11+ messages in thread
From: Christoph Biedl @ 2023-05-12 21:56 UTC (permalink / raw)
  To: linux-parisc

[-- Attachment #1: Type: text/plain, Size: 1744 bytes --]

Helge Deller wrote...

> Since you run the 32-bit kernel, huge-pages are not involved as they
> aren't available in the 32-bit kernels.
> So I think swapping is triggering it.
> You could try to find a test program which triggers swapping, e.g. LTS testcases?
> Another test could be to enable CONFIG_MIGRATION again and disable
> all swap spaces and see if it survives.

Well, turns out I'm not using swap at all. But the "memory under
pressure" seemed right, and I could easily trigger the crash by
allowcating almost the entire available memory[1].

Then bisecting led to

commit 6d239fc78c0b0c687e5408573350714e6e789d71
Author: David Hildenbrand <david@redhat.com>
Date:   Fri Jan 13 18:10:16 2023 +0100

    parisc/mm: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE

    Let's support __HAVE_ARCH_PTE_SWP_EXCLUSIVE by using the yet-unused
    _PAGE_ACCESSED location in the swap PTE.  Looking at pte_present() and
    pte_none() checks, there seems to be no actual reason why we cannot use
    it: we only have to make sure we're not using _PAGE_PRESENT.

    Reusing this bit avoids having to steal one bit from the swap offset.

    Link: https://lkml.kernel.org/r/20230113171026.582290-17-david@redhat.com
    Signed-off-by: David Hildenbrand <david@redhat.com>
    Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
    Cc: Helge Deller <deller@gmx.de>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

Does this make sense?

    Christoph

[1] Total is 1 Gbyte, and running
    | dd if=/dev/zero bs=896M count=1 | pv --rate-limit=1k >/dev/null
    might not be the best style but does the trick: Wait for pv to
    count up to a minute, then ^C it. If the host is still okay after
    that, it's considered "good".

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Regression with kernel 6.3 "kernel BUG at include/linux/swapops.h:472!"
  2023-05-12 21:56       ` Christoph Biedl
@ 2023-05-13 12:10         ` Helge Deller
  2023-05-13 13:21           ` David Hildenbrand
  2023-05-13 16:24           ` Christoph Biedl
  0 siblings, 2 replies; 11+ messages in thread
From: Helge Deller @ 2023-05-13 12:10 UTC (permalink / raw)
  To: Christoph Biedl; +Cc: linux-parisc, David Hildenbrand

* Christoph Biedl <linux-kernel.bfrz@manchmal.in-ulm.de>:
> Helge Deller wrote...
>
> > Since you run the 32-bit kernel, huge-pages are not involved as they
> > aren't available in the 32-bit kernels.
> > So I think swapping is triggering it.
> > You could try to find a test program which triggers swapping, e.g. LTS testcases?
> > Another test could be to enable CONFIG_MIGRATION again and disable
> > all swap spaces and see if it survives.
>
> Well, turns out I'm not using swap at all. But the "memory under
> pressure" seemed right, and I could easily trigger the crash by
> allowcating almost the entire available memory[1].
>
> Then bisecting led to
>
> commit 6d239fc78c0b0c687e5408573350714e6e789d71
> Author: David Hildenbrand <david@redhat.com>
> Date:   Fri Jan 13 18:10:16 2023 +0100
>
>     parisc/mm: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE
>
>     Let's support __HAVE_ARCH_PTE_SWP_EXCLUSIVE by using the yet-unused
>     _PAGE_ACCESSED location in the swap PTE.  Looking at pte_present() and
>     pte_none() checks, there seems to be no actual reason why we cannot use
>     it: we only have to make sure we're not using _PAGE_PRESENT.
>
>     Reusing this bit avoids having to steal one bit from the swap offset.
>
>     Link: https://lkml.kernel.org/r/20230113171026.582290-17-david@redhat.com
>     Signed-off-by: David Hildenbrand <david@redhat.com>
>     Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
>     Cc: Helge Deller <deller@gmx.de>
>     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>
> Does this make sense?

Yes, makes sense.

> [1] Total is 1 Gbyte, and running
>     | dd if=/dev/zero bs=896M count=1 | pv --rate-limit=1k >/dev/null
>     might not be the best style but does the trick: Wait for pv to
>     count up to a minute, then ^C it. If the host is still okay after
>     that, it's considered "good".

Thanks for bisecting and coming up with a testcase!
The attached patch survives for me on my C3000 with 2GB RAM with this test:
	dd if=/dev/zero bs=1896M count=1 | pv
(well, the OOM-killer might jump in, but even that is survived).

Could you try the patch below?

Helge

-

[PATCH] parisc: Fix encoding of swp_entry due to added SWP_EXCLUSIVE flag

Fix the __swp_offset() and __swp_entry() macros due to commit 6d239fc78c0b
("parisc/mm: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE") which introduced the
SWP_EXCLUSIVE flag by reusing the _PAGE_ACCESSED flag.

Reported-by: Christoph Biedl <linux-kernel.bfrz@manchmal.in-ulm.de>
Fixes: 6d239fc78c0b ("parisc/mm: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE")

diff --git a/arch/parisc/include/asm/pgtable.h b/arch/parisc/include/asm/pgtable.h
index e2950f5db7c9..522846be54b7 100644
--- a/arch/parisc/include/asm/pgtable.h
+++ b/arch/parisc/include/asm/pgtable.h
@@ -413,12 +413,12 @@ extern void paging_init (void);
  *   For the 64bit version, the offset is extended by 32bit.
  */
 #define __swp_type(x)                     ((x).val & 0x1f)
-#define __swp_offset(x)                   ( (((x).val >> 6) &  0x7) | \
-					  (((x).val >> 8) & ~0x7) )
+#define __swp_offset(x)                   ( (((x).val >> 5) &  0x7) | \
+					  (((x).val >> 10) << 3) )
 #define __swp_entry(type, offset)         ((swp_entry_t) { \
 					    ((type) & 0x1f) | \
-					    ((offset &  0x7) << 6) | \
-					    ((offset & ~0x7) << 8) })
+					    ((offset &  0x7) << 5) | \
+					    ((offset & ~0x7) << 7) })
 #define __pte_to_swp_entry(pte)		((swp_entry_t) { pte_val(pte) })
 #define __swp_entry_to_pte(x)		((pte_t) { (x).val })




^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: Regression with kernel 6.3 "kernel BUG at include/linux/swapops.h:472!"
  2023-05-13 12:10         ` Helge Deller
@ 2023-05-13 13:21           ` David Hildenbrand
  2023-05-13 13:58             ` Helge Deller
  2023-05-13 16:24           ` Christoph Biedl
  1 sibling, 1 reply; 11+ messages in thread
From: David Hildenbrand @ 2023-05-13 13:21 UTC (permalink / raw)
  To: Helge Deller, Christoph Biedl; +Cc: linux-parisc

> Yes, makes sense.
> 
>> [1] Total is 1 Gbyte, and running
>>      | dd if=/dev/zero bs=896M count=1 | pv --rate-limit=1k >/dev/null
>>      might not be the best style but does the trick: Wait for pv to
>>      count up to a minute, then ^C it. If the host is still okay after
>>      that, it's considered "good".
> 
> Thanks for bisecting and coming up with a testcase!
> The attached patch survives for me on my C3000 with 2GB RAM with this test:
> 	dd if=/dev/zero bs=1896M count=1 | pv
> (well, the OOM-killer might jump in, but even that is survived).
> 
> Could you try the patch below?

Thanks for debugging! :)

> 
> Helge
> 
> -
> 
> [PATCH] parisc: Fix encoding of swp_entry due to added SWP_EXCLUSIVE flag
> 
> Fix the __swp_offset() and __swp_entry() macros due to commit 6d239fc78c0b
> ("parisc/mm: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE") which introduced the
> SWP_EXCLUSIVE flag by reusing the _PAGE_ACCESSED flag.
> 
> Reported-by: Christoph Biedl <linux-kernel.bfrz@manchmal.in-ulm.de>
> Fixes: 6d239fc78c0b ("parisc/mm: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE")
> 
> diff --git a/arch/parisc/include/asm/pgtable.h b/arch/parisc/include/asm/pgtable.h
> index e2950f5db7c9..522846be54b7 100644
> --- a/arch/parisc/include/asm/pgtable.h
> +++ b/arch/parisc/include/asm/pgtable.h
> @@ -413,12 +413,12 @@ extern void paging_init (void);
>    *   For the 64bit version, the offset is extended by 32bit.
>    */
>   #define __swp_type(x)                     ((x).val & 0x1f)
> -#define __swp_offset(x)                   ( (((x).val >> 6) &  0x7) | \
> -					  (((x).val >> 8) & ~0x7) )
> +#define __swp_offset(x)                   ( (((x).val >> 5) &  0x7) | \
> +					  (((x).val >> 10) << 3) )
>   #define __swp_entry(type, offset)         ((swp_entry_t) { \
>   					    ((type) & 0x1f) | \
> -					    ((offset &  0x7) << 6) | \
> -					    ((offset & ~0x7) << 8) })
> +					    ((offset &  0x7) << 5) | \
> +					    ((offset & ~0x7) << 7) })
>   #define __pte_to_swp_entry(pte)		((swp_entry_t) { pte_val(pte) })
>   #define __swp_entry_to_pte(x)		((pte_t) { (x).val })

This fix makes it work like the layout I documented.

What I originally tried doing was reusing one of the spare bits instead of reworking
the layout. Apparently, I got the old layout wrong. :(

So if I understood the layout right this time, maybe we can just use one of the two
spare bits: _PAGE_HUGE (or alternatively, _PAGE_DIRTY_BIT)?



diff --git a/arch/parisc/include/asm/pgtable.h b/arch/parisc/include/asm/pgtable.h
index e2950f5db7c9..ee9f08cd5938 100644
--- a/arch/parisc/include/asm/pgtable.h
+++ b/arch/parisc/include/asm/pgtable.h
@@ -218,8 +218,8 @@ extern void __update_cache(pte_t pte);
  #define _PAGE_KERNEL_RWX       (_PAGE_KERNEL_EXEC | _PAGE_WRITE)
  #define _PAGE_KERNEL           (_PAGE_KERNEL_RO | _PAGE_WRITE)
  
-/* We borrow bit 23 to store the exclusive marker in swap PTEs. */
-#define _PAGE_SWP_EXCLUSIVE    _PAGE_ACCESSED
+/* We borrow bit 21 to store the exclusive marker in swap PTEs. */
+#define _PAGE_SWP_EXCLUSIVE    _PAGE_HUGE
  
  /* The pgd/pmd contains a ptr (in phys addr space); since all pgds/pmds
   * are page-aligned, we don't care about the PAGE_OFFSET bits, except
@@ -405,7 +405,7 @@ extern void paging_init (void);
   *
   *                         1 1 1 1 1 1 1 1 1 2 2 2 2 2 2 2 2 2 2 3 3
   *   0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
- *   <---------------- offset -----------------> P E <ofs> < type ->
+ *   <---------------- offset ---------------> E P <ofs> 0 < type ->
   *
   *   E is the exclusive marker that is not stored in swap entries.
   *   _PAGE_PRESENT (P) must be 0.
diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 110a36479dce..7510db355f48 100644


-- 
Thanks,

David / dhildenb


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: Regression with kernel 6.3 "kernel BUG at include/linux/swapops.h:472!"
  2023-05-13 13:21           ` David Hildenbrand
@ 2023-05-13 13:58             ` Helge Deller
  2023-05-13 23:32               ` David Hildenbrand
  0 siblings, 1 reply; 11+ messages in thread
From: Helge Deller @ 2023-05-13 13:58 UTC (permalink / raw)
  To: David Hildenbrand, Christoph Biedl; +Cc: linux-parisc

On 5/13/23 15:21, David Hildenbrand wrote:
>> Yes, makes sense.
>>
>>> [1] Total is 1 Gbyte, and running
>>>      | dd if=/dev/zero bs=896M count=1 | pv --rate-limit=1k >/dev/null
>>>      might not be the best style but does the trick: Wait for pv to
>>>      count up to a minute, then ^C it. If the host is still okay after
>>>      that, it's considered "good".
>>
>> Thanks for bisecting and coming up with a testcase!
>> The attached patch survives for me on my C3000 with 2GB RAM with this test:
>>     dd if=/dev/zero bs=1896M count=1 | pv
>> (well, the OOM-killer might jump in, but even that is survived).
>>
>> Could you try the patch below?
>
> Thanks for debugging! :)
>>
>> [PATCH] parisc: Fix encoding of swp_entry due to added SWP_EXCLUSIVE flag
>>
>> Fix the __swp_offset() and __swp_entry() macros due to commit 6d239fc78c0b
>> ("parisc/mm: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE") which introduced the
>> SWP_EXCLUSIVE flag by reusing the _PAGE_ACCESSED flag.
>>
>> Reported-by: Christoph Biedl <linux-kernel.bfrz@manchmal.in-ulm.de>
>> Fixes: 6d239fc78c0b ("parisc/mm: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE")
>>
>> diff --git a/arch/parisc/include/asm/pgtable.h b/arch/parisc/include/asm/pgtable.h
>> index e2950f5db7c9..522846be54b7 100644
>> --- a/arch/parisc/include/asm/pgtable.h
>> +++ b/arch/parisc/include/asm/pgtable.h
>> @@ -413,12 +413,12 @@ extern void paging_init (void);
>>    *   For the 64bit version, the offset is extended by 32bit.
>>    */
>>   #define __swp_type(x)                     ((x).val & 0x1f)
>> -#define __swp_offset(x)                   ( (((x).val >> 6) &  0x7) | \
>> -                      (((x).val >> 8) & ~0x7) )
>> +#define __swp_offset(x)                   ( (((x).val >> 5) &  0x7) | \
>> +                      (((x).val >> 10) << 3) )
>>   #define __swp_entry(type, offset)         ((swp_entry_t) { \
>>                           ((type) & 0x1f) | \
>> -                        ((offset &  0x7) << 6) | \
>> -                        ((offset & ~0x7) << 8) })
>> +                        ((offset &  0x7) << 5) | \
>> +                        ((offset & ~0x7) << 7) })
>>   #define __pte_to_swp_entry(pte)        ((swp_entry_t) { pte_val(pte) })
>>   #define __swp_entry_to_pte(x)        ((pte_t) { (x).val })
>
> This fix makes it work like the layout I documented.

Yes, and your layout looks good for me.

> What I originally tried doing was reusing one of the spare bits instead of reworking
> the layout. Apparently, I got the old layout wrong. :(

Don't worry! Your patch harmonizes parisc to the other platforms, which is good.

> So if I understood the layout right this time, maybe we can just use one of the two
> spare bits: _PAGE_HUGE (or alternatively, _PAGE_DIRTY_BIT)?

Yes, or keep what you suggested.

What I don't understand yet is the original code:
#define __swp_type(x)                     ((x).val & 0x1f)
#define __swp_offset(x)                   ( (((x).val >> 6) &  0x7) | \
                                           (((x).val >> 8) & ~0x7) )
#define __swp_entry(type, offset)         ((swp_entry_t) { (type) | \
                                             ((offset &  0x7) << 6) | \
                                             ((offset & ~0x7) << 8) })

Don't we loose one of the offset bits?
Mask 0x7 is 3 bits, but we shift by 6 and 8 (=2 bits difference), so I believe the second shift should be 9.
If it would be 9, then no &0x07 is needed and only one shift would be sufficient.

I don't know much in the swap pte area, but isn't the previous original code wrong?
Which bits of the swp_entry are used where?

Helge

> diff --git a/arch/parisc/include/asm/pgtable.h b/arch/parisc/include/asm/pgtable.h
> index e2950f5db7c9..ee9f08cd5938 100644
> --- a/arch/parisc/include/asm/pgtable.h
> +++ b/arch/parisc/include/asm/pgtable.h
> @@ -218,8 +218,8 @@ extern void __update_cache(pte_t pte);
>   #define _PAGE_KERNEL_RWX       (_PAGE_KERNEL_EXEC | _PAGE_WRITE)
>   #define _PAGE_KERNEL           (_PAGE_KERNEL_RO | _PAGE_WRITE)
>
> -/* We borrow bit 23 to store the exclusive marker in swap PTEs. */
> -#define _PAGE_SWP_EXCLUSIVE    _PAGE_ACCESSED
> +/* We borrow bit 21 to store the exclusive marker in swap PTEs. */
> +#define _PAGE_SWP_EXCLUSIVE    _PAGE_HUGE
>
>   /* The pgd/pmd contains a ptr (in phys addr space); since all pgds/pmds
>    * are page-aligned, we don't care about the PAGE_OFFSET bits, except
> @@ -405,7 +405,7 @@ extern void paging_init (void);
>    *
>    *                         1 1 1 1 1 1 1 1 1 2 2 2 2 2 2 2 2 2 2 3 3
>    *   0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> - *   <---------------- offset -----------------> P E <ofs> < type ->
> + *   <---------------- offset ---------------> E P <ofs> 0 < type ->
>    *
>    *   E is the exclusive marker that is not stored in swap entries.
>    *   _PAGE_PRESENT (P) must be 0.
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index 110a36479dce..7510db355f48 100644
>
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Regression with kernel 6.3 "kernel BUG at include/linux/swapops.h:472!"
  2023-05-13 12:10         ` Helge Deller
  2023-05-13 13:21           ` David Hildenbrand
@ 2023-05-13 16:24           ` Christoph Biedl
  1 sibling, 0 replies; 11+ messages in thread
From: Christoph Biedl @ 2023-05-13 16:24 UTC (permalink / raw)
  To: Helge Deller; +Cc: linux-parisc, David Hildenbrand

[-- Attachment #1: Type: text/plain, Size: 230 bytes --]

Helge Deller wrote...

> Thanks for bisecting and coming up with a testcase!

Well, thanks for all the advice, and the switft responses as well.

> Could you try the patch below?

Can confirm: Works very well here.

    Christoph

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Regression with kernel 6.3 "kernel BUG at include/linux/swapops.h:472!"
  2023-05-13 13:58             ` Helge Deller
@ 2023-05-13 23:32               ` David Hildenbrand
  2023-05-14  0:09                 ` Helge Deller
  0 siblings, 1 reply; 11+ messages in thread
From: David Hildenbrand @ 2023-05-13 23:32 UTC (permalink / raw)
  To: Helge Deller, Christoph Biedl; +Cc: linux-parisc

>>
>> This fix makes it work like the layout I documented.
> 
> Yes, and your layout looks good for me.

Good :)

> 
>> What I originally tried doing was reusing one of the spare bits instead of reworking
>> the layout. Apparently, I got the old layout wrong. :(
> 
> Don't worry! Your patch harmonizes parisc to the other platforms, which is good.
> 
>> So if I understood the layout right this time, maybe we can just use one of the two
>> spare bits: _PAGE_HUGE (or alternatively, _PAGE_DIRTY_BIT)?
> 
> Yes, or keep what you suggested.
> 
> What I don't understand yet is the original code:
> #define __swp_type(x)                     ((x).val & 0x1f)
> #define __swp_offset(x)                   ( (((x).val >> 6) &  0x7) | \
>                                             (((x).val >> 8) & ~0x7) )
> #define __swp_entry(type, offset)         ((swp_entry_t) { (type) | \
>                                               ((offset &  0x7) << 6) | \
>                                               ((offset & ~0x7) << 8) })
> 
> Don't we loose one of the offset bits?

Let's assume we have the offset 0xff. Encoding it with type 0 would be

((0xff & 0x7) << 6) | ((0xff & ~0x7) << 8)
-> (0x7 << 6) | (0xf8 << 8)
-> 0x1c0 | 0xf800
-> 0xf9c0

Extracting the offset:

((0xf9c0 >> 6) & 0x7) | ((0xf9c0 >> 8) & ~0x7)
-> (0x3e7 & 0x7) | (0xf9 & ~0x7)
-> 0x7 | 0xf8
-> 0xff

I think it's correct. The confusing part (that resulted in the BUG here) 
is that we end up wasting bit #26, because there is a spare bit between 
the type and the offset.

Maybe a relic from the past -- or copy-and-paste, because some archs 
supported types with > 5 bits, but core-MM only ever uses 5 bits.

> Mask 0x7 is 3 bits, but we shift by 6 and 8 (=2 bits difference), so I believe the second shift should be 9.
> If it would be 9, then no &0x07 is needed and only one shift would be sufficient.
> 
> I don't know much in the swap pte area, but isn't the previous original code wrong?
> Which bits of the swp_entry are used where?
I think the old code was correct. There are apparently two spare bits 
that we can use. I just messed up the old layout, thinking there is only 
one.

So we can either use the new layout I documented (with the fix you 
propose), or use another layout.

In any case, we *gain* one more bit for the offset compared to the old 
layout.


I'm more than happy to keep the new layout. Regarding your fix, maybe 
avoid the other ~0x7 as well by using similar shifting in __swp_entry()


  #define __swp_entry(type, offset)         ((swp_entry_t) { \
  					    ((type) & 0x1f) | \
					    ((offset &  0x7) << 5) | \
					    ((offset >> 3) << 10) })

So it's easier to match to the logic/values in __swp_offset().


In any case,

Reviewed-by: David Hildenbrand <david@redhat.com>

and thanks!

-- 
Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Regression with kernel 6.3 "kernel BUG at include/linux/swapops.h:472!"
  2023-05-13 23:32               ` David Hildenbrand
@ 2023-05-14  0:09                 ` Helge Deller
  0 siblings, 0 replies; 11+ messages in thread
From: Helge Deller @ 2023-05-14  0:09 UTC (permalink / raw)
  To: David Hildenbrand, Christoph Biedl; +Cc: linux-parisc

On 5/14/23 01:32, David Hildenbrand wrote:
>>>
>>> This fix makes it work like the layout I documented.
>>
>> Yes, and your layout looks good for me.
>
> Good :)
>
>>
>>> What I originally tried doing was reusing one of the spare bits instead of reworking
>>> the layout. Apparently, I got the old layout wrong. :(
>>
>> Don't worry! Your patch harmonizes parisc to the other platforms, which is good.
>>
>>> So if I understood the layout right this time, maybe we can just use one of the two
>>> spare bits: _PAGE_HUGE (or alternatively, _PAGE_DIRTY_BIT)?
>>
>> Yes, or keep what you suggested.
>>
>> What I don't understand yet is the original code:
>> #define __swp_type(x)                     ((x).val & 0x1f)
>> #define __swp_offset(x)                   ( (((x).val >> 6) &  0x7) | \
>>                                             (((x).val >> 8) & ~0x7) )
>> #define __swp_entry(type, offset)         ((swp_entry_t) { (type) | \
>>                                               ((offset &  0x7) << 6) | \
>>                                               ((offset & ~0x7) << 8) })
>>
>> Don't we loose one of the offset bits?
>
> Let's assume we have the offset 0xff. Encoding it with type 0 would be
>
> ((0xff & 0x7) << 6) | ((0xff & ~0x7) << 8)
> -> (0x7 << 6) | (0xf8 << 8)
> -> 0x1c0 | 0xf800
> -> 0xf9c0
>
> Extracting the offset:
>
> ((0xf9c0 >> 6) & 0x7) | ((0xf9c0 >> 8) & ~0x7)
> -> (0x3e7 & 0x7) | (0xf9 & ~0x7)
> -> 0x7 | 0xf8
> -> 0xff
>
> I think it's correct.

Yes. Seems good.

> The confusing part (that resulted in the BUG here) is that we end up wasting bit #26, because there is a spare bit between the type and the offset.
>
> Maybe a relic from the past -- or copy-and-paste, because some archs supported types with > 5 bits, but core-MM only ever uses 5 bits.

Hard to say. It has been as such since a long time....

>> Mask 0x7 is 3 bits, but we shift by 6 and 8 (=2 bits difference), so I believe the second shift should be 9.
>> If it would be 9, then no &0x07 is needed and only one shift would be sufficient.
>>
>> I don't know much in the swap pte area, but isn't the previous original code wrong?
>> Which bits of the swp_entry are used where?
> I think the old code was correct. There are apparently two spare bits that we can use. I just messed up the old layout, thinking there is only one.
>
> So we can either use the new layout I documented (with the fix you propose), or use another layout.

I think I prefer the layout which you documented.

> In any case, we *gain* one more bit for the offset compared to the old layout.
>
>
> I'm more than happy to keep the new layout. Regarding your fix, maybe avoid the other ~0x7 as well by using similar shifting in __swp_entry()
>
>
>   #define __swp_entry(type, offset)         ((swp_entry_t) { \
>                           ((type) & 0x1f) | \
>                          ((offset &  0x7) << 5) | \
>                          ((offset >> 3) << 10) })
>
> So it's easier to match to the logic/values in __swp_offset().

Yes, it's much better.
I fixed it up like this in my current git tree.

> In any case,
> Reviewed-by: David Hildenbrand <david@redhat.com>

Great. Thanks for your help & suggestions!

Helge

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-05-14  0:09 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-10 17:56 Regression with kernel 6.3 "kernel BUG at include/linux/swapops.h:472!" Christoph Biedl
2023-05-10 20:29 ` Helge Deller
2023-05-11 17:22   ` Christoph Biedl
2023-05-11 17:35     ` Helge Deller
2023-05-12 21:56       ` Christoph Biedl
2023-05-13 12:10         ` Helge Deller
2023-05-13 13:21           ` David Hildenbrand
2023-05-13 13:58             ` Helge Deller
2023-05-13 23:32               ` David Hildenbrand
2023-05-14  0:09                 ` Helge Deller
2023-05-13 16:24           ` Christoph Biedl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox