linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [patch 07/54] mm/slub: use stackdepot to save stack trace in objects
       [not found] ` <20210708010747.zIP9yxsci%akpm@linux-foundation.org>
@ 2021-07-16  7:39   ` Christoph Hellwig
  2021-07-16  8:57     ` Vlastimil Babka
  2021-07-16 20:12     ` Linus Torvalds
  0 siblings, 2 replies; 8+ messages in thread
From: Christoph Hellwig @ 2021-07-16  7:39 UTC (permalink / raw)
  To: Andrew Morton
  Cc: cl, glittao, iamjoonsoo.kim, linux-mm, mm-commits, penberg,
	rdunlap, rientjes, torvalds, vbabka, linux-xfs

This somewhat unexpectedly causes a crash when running the xfs/433 test
in xfstests for me.  Reverting the commit fixes the problem:

xfs/433 files ... [  138.422742] run fstests xfs/433 at 2021-07-16 07:30:42
[  140.128145] XFS (vdb): Mounting V5 Filesystem
[  140.160450] XFS (vdb): Ending clean mount
[  140.171782] xfs filesystem being mounted at /mnt/test supports timestamps un)
[  140.966560] XFS (vdc): Mounting V5 Filesystem
[  140.987911] XFS (vdc): Ending clean mount
[  141.000104] xfs filesystem being mounted at /mnt/scratch supports timestamps)
[  145.130156] XFS (vdc): Unmounting Filesystem
[  145.365230] XFS (vdc): Mounting V5 Filesystem
[  145.394542] XFS (vdc): Ending clean mount
[  145.409232] xfs filesystem being mounted at /mnt/scratch supports timestamps)
[  145.471384] XFS (vdc): Injecting error (false) at file fs/xfs/xfs_buf.c, lin"
[  145.478561] XFS (vdc): Injecting error (false) at file fs/xfs/xfs_buf.c, lin"
[  145.486070] XFS (vdc): Injecting error (false) at file fs/xfs/xfs_buf.c, lin"
[  145.492248] XFS (vdc): Injecting error (false) at file fs/xfs/xfs_buf.c, lin"
[  145.599964] XFS (vdb): Unmounting Filesystem
[  145.958340] BUG: kernel NULL pointer dereference, address: 0000000000000020
[  145.961760] #PF: supervisor read access in kernel mode
[  145.964278] #PF: error_code(0x0000) - not-present page
[  145.966758] PGD 0 P4D 0 
[  145.968041] Oops: 0000 [#1] PREEMPT SMP PTI
[  145.970077] CPU: 3 PID: 14172 Comm: xfs_scrub Not tainted 5.13.0+ #601
[  145.973243] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.144
[  145.977312] RIP: 0010:xfs_inode_hasattr+0x19/0x30
[  145.979626] Code: 83 c6 05 b2 55 75 02 01 e8 39 40 e4 00 eb b6 66 90 31 c0 80
[  145.989446] RSP: 0018:ffffc900070eba08 EFLAGS: 00010206
[  145.992280] RAX: ffffffff00ff0000 RBX: 0000000000000000 RCX: 0000000000000001
[  145.995970] RDX: 0000000000000000 RSI: ffffffff82fdd33f RDI: ffff88810dbe16c0
[  145.999945] RBP: ffff88810dbe16c0 R08: ffff888110e14348 R09: ffff888110e14348
[  146.003932] R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000000
[  146.007854] R13: ffff888110d99000 R14: ffff888110d99000 R15: ffffffff834acd60
[  146.011765] FS:  00007f2bf29d7700(0000) GS:ffff88813bd80000(0000) knlGS:00000
[  146.016127] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  146.019297] CR2: 0000000000000020 CR3: 0000000110c96000 CR4: 00000000000006e0
[  146.023315] Call Trace:
[  146.024726]  xfs_attr_inactive+0x152/0x350
[  146.027059]  xfs_inactive+0x18a/0x240
[  146.029141]  xfs_fs_destroy_inode+0xcc/0x2d0
[  146.031311]  destroy_inode+0x36/0x70
[  146.033130]  xfs_bulkstat_one_int+0x243/0x340
[  146.035342]  xfs_bulkstat_iwalk+0x19/0x30
[  146.037562]  xfs_iwalk_ag_recs+0xef/0x1e0
[  146.039845]  xfs_iwalk_run_callbacks+0x9f/0x140
[  146.042550]  xfs_iwalk_ag+0x230/0x2f0
[  146.044601]  xfs_iwalk+0x139/0x200
[  146.046505]  ? xfs_bulkstat_one_int+0x340/0x340
[  146.049151]  xfs_bulkstat+0xc4/0x130
[  146.050771]  ? xfs_flags2diflags+0xe0/0xe0
[  146.052309]  xfs_ioc_bulkstat.constprop.0.isra.0+0xbf/0x120
[  146.054200]  xfs_file_ioctl+0xb6/0xef0
[  146.055474]  ? lock_is_held_type+0xd5/0x130
[  146.056867]  ? find_held_lock+0x2b/0x80
[  146.058241]  ? lock_release+0x13c/0x2e0
[  146.059385]  ? lock_is_held_type+0xd5/0x130
[  146.060435]  ? __fget_files+0xce/0x1d0
[  146.061385]  __x64_sys_ioctl+0x7e/0xb0
[  146.062333]  do_syscall_64+0x3b/0x90
[  146.063284]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[  146.064572] RIP: 0033:0x7f2bf2df5427
[  146.065600] Code: 00 00 90 48 8b 05 69 aa 0c 00 64 c7 00 26 00 00 00 48 c7 c8
[  146.070244] RSP: 002b:00007f2bf29d6bd8 EFLAGS: 00000246 ORIG_RAX: 00000000000
[  146.072015] RAX: ffffffffffffffda RBX: 00007fffe44b8010 RCX: 00007f2bf2df5427
[  146.073692] RDX: 00007f2be4000b20 RSI: 000000008040587f RDI: 0000000000000003
[  146.075322] RBP: 00007f2be4000b20 R08: 00007f2be4003b70 R09: 0000000000000077
[  146.076962] R10: 0000000000000001 R11: 0000000000000246 R12: 00007f2be4003b70
[  146.078480] R13: 00007fffe44b8010 R14: 00007f2be4000b60 R15: 0000000000000018
[  146.079803] Modules linked in:
[  146.080379] CR2: 0000000000000020
[  146.081196] ---[ end trace 80a6ea90b0ea2a03 ]---
[  146.082130] RIP: 0010:xfs_inode_hasattr+0x19/0x30
[  146.083144] Code: 83 c6 05 b2 55 75 02 01 e8 39 40 e4 00 eb b6 66 90 31 c0 80
[  146.086831] RSP: 0018:ffffc900070eba08 EFLAGS: 00010206
[  146.087816] RAX: ffffffff00ff0000 RBX: 0000000000000000 RCX: 0000000000000001
[  146.089122] RDX: 0000000000000000 RSI: ffffffff82fdd33f RDI: ffff88810dbe16c0
[  146.090477] RBP: ffff88810dbe16c0 R08: ffff888110e14348 R09: ffff888110e14348
[  146.091794] R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000000
[  146.093096] R13: ffff888110d99000 R14: ffff888110d99000 R15: ffffffff834acd60
[  146.094429] FS:  00007f2bf29d7700(0000) GS:ffff88813bd80000(0000) knlGS:00000
[  146.096002] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  146.097079] CR2: 0000000000000020 CR3: 0000000110c96000 CR4: 00000000000006e0
[  146.098479] Kernel panic - not syncing: Fatal exception
[  146.099677] Kernel Offset: disabled
[  146.100397] ---[ end Kernel panic - not syncing: Fatal exception ]---



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

* Re: [patch 07/54] mm/slub: use stackdepot to save stack trace in objects
  2021-07-16  7:39   ` [patch 07/54] mm/slub: use stackdepot to save stack trace in objects Christoph Hellwig
@ 2021-07-16  8:57     ` Vlastimil Babka
  2021-07-16  9:12       ` Christoph Hellwig
  2021-07-16 20:12     ` Linus Torvalds
  1 sibling, 1 reply; 8+ messages in thread
From: Vlastimil Babka @ 2021-07-16  8:57 UTC (permalink / raw)
  To: Christoph Hellwig, Andrew Morton
  Cc: cl, glittao, iamjoonsoo.kim, linux-mm, mm-commits, penberg,
	rdunlap, rientjes, torvalds, linux-xfs

On 7/16/21 9:39 AM, Christoph Hellwig wrote:
> This somewhat unexpectedly causes a crash when running the xfs/433 test
> in xfstests for me.  Reverting the commit fixes the problem:

That's weird, the backtrace doesn't even include SLUB/stackdepot code.
Is that kernel actually booted with slub_debug option/built with
CONFIG_SLUB_DEBUG_ON or some cache created with SLAB_STORE_USER ?

> 
> xfs/433 files ... [  138.422742] run fstests xfs/433 at 2021-07-16 07:30:42
> [  140.128145] XFS (vdb): Mounting V5 Filesystem
> [  140.160450] XFS (vdb): Ending clean mount
> [  140.171782] xfs filesystem being mounted at /mnt/test supports timestamps un)
> [  140.966560] XFS (vdc): Mounting V5 Filesystem
> [  140.987911] XFS (vdc): Ending clean mount
> [  141.000104] xfs filesystem being mounted at /mnt/scratch supports timestamps)
> [  145.130156] XFS (vdc): Unmounting Filesystem
> [  145.365230] XFS (vdc): Mounting V5 Filesystem
> [  145.394542] XFS (vdc): Ending clean mount
> [  145.409232] xfs filesystem being mounted at /mnt/scratch supports timestamps)
> [  145.471384] XFS (vdc): Injecting error (false) at file fs/xfs/xfs_buf.c, lin"
> [  145.478561] XFS (vdc): Injecting error (false) at file fs/xfs/xfs_buf.c, lin"
> [  145.486070] XFS (vdc): Injecting error (false) at file fs/xfs/xfs_buf.c, lin"
> [  145.492248] XFS (vdc): Injecting error (false) at file fs/xfs/xfs_buf.c, lin"
> [  145.599964] XFS (vdb): Unmounting Filesystem
> [  145.958340] BUG: kernel NULL pointer dereference, address: 0000000000000020
> [  145.961760] #PF: supervisor read access in kernel mode
> [  145.964278] #PF: error_code(0x0000) - not-present page
> [  145.966758] PGD 0 P4D 0 
> [  145.968041] Oops: 0000 [#1] PREEMPT SMP PTI
> [  145.970077] CPU: 3 PID: 14172 Comm: xfs_scrub Not tainted 5.13.0+ #601
> [  145.973243] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.144
> [  145.977312] RIP: 0010:xfs_inode_hasattr+0x19/0x30
> [  145.979626] Code: 83 c6 05 b2 55 75 02 01 e8 39 40 e4 00 eb b6 66 90 31 c0 80
> [  145.989446] RSP: 0018:ffffc900070eba08 EFLAGS: 00010206
> [  145.992280] RAX: ffffffff00ff0000 RBX: 0000000000000000 RCX: 0000000000000001
> [  145.995970] RDX: 0000000000000000 RSI: ffffffff82fdd33f RDI: ffff88810dbe16c0
> [  145.999945] RBP: ffff88810dbe16c0 R08: ffff888110e14348 R09: ffff888110e14348
> [  146.003932] R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000000
> [  146.007854] R13: ffff888110d99000 R14: ffff888110d99000 R15: ffffffff834acd60
> [  146.011765] FS:  00007f2bf29d7700(0000) GS:ffff88813bd80000(0000) knlGS:00000
> [  146.016127] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  146.019297] CR2: 0000000000000020 CR3: 0000000110c96000 CR4: 00000000000006e0
> [  146.023315] Call Trace:
> [  146.024726]  xfs_attr_inactive+0x152/0x350
> [  146.027059]  xfs_inactive+0x18a/0x240
> [  146.029141]  xfs_fs_destroy_inode+0xcc/0x2d0
> [  146.031311]  destroy_inode+0x36/0x70
> [  146.033130]  xfs_bulkstat_one_int+0x243/0x340
> [  146.035342]  xfs_bulkstat_iwalk+0x19/0x30
> [  146.037562]  xfs_iwalk_ag_recs+0xef/0x1e0
> [  146.039845]  xfs_iwalk_run_callbacks+0x9f/0x140
> [  146.042550]  xfs_iwalk_ag+0x230/0x2f0
> [  146.044601]  xfs_iwalk+0x139/0x200
> [  146.046505]  ? xfs_bulkstat_one_int+0x340/0x340
> [  146.049151]  xfs_bulkstat+0xc4/0x130
> [  146.050771]  ? xfs_flags2diflags+0xe0/0xe0
> [  146.052309]  xfs_ioc_bulkstat.constprop.0.isra.0+0xbf/0x120
> [  146.054200]  xfs_file_ioctl+0xb6/0xef0
> [  146.055474]  ? lock_is_held_type+0xd5/0x130
> [  146.056867]  ? find_held_lock+0x2b/0x80
> [  146.058241]  ? lock_release+0x13c/0x2e0
> [  146.059385]  ? lock_is_held_type+0xd5/0x130
> [  146.060435]  ? __fget_files+0xce/0x1d0
> [  146.061385]  __x64_sys_ioctl+0x7e/0xb0
> [  146.062333]  do_syscall_64+0x3b/0x90
> [  146.063284]  entry_SYSCALL_64_after_hwframe+0x44/0xae
> [  146.064572] RIP: 0033:0x7f2bf2df5427
> [  146.065600] Code: 00 00 90 48 8b 05 69 aa 0c 00 64 c7 00 26 00 00 00 48 c7 c8
> [  146.070244] RSP: 002b:00007f2bf29d6bd8 EFLAGS: 00000246 ORIG_RAX: 00000000000
> [  146.072015] RAX: ffffffffffffffda RBX: 00007fffe44b8010 RCX: 00007f2bf2df5427
> [  146.073692] RDX: 00007f2be4000b20 RSI: 000000008040587f RDI: 0000000000000003
> [  146.075322] RBP: 00007f2be4000b20 R08: 00007f2be4003b70 R09: 0000000000000077
> [  146.076962] R10: 0000000000000001 R11: 0000000000000246 R12: 00007f2be4003b70
> [  146.078480] R13: 00007fffe44b8010 R14: 00007f2be4000b60 R15: 0000000000000018
> [  146.079803] Modules linked in:
> [  146.080379] CR2: 0000000000000020
> [  146.081196] ---[ end trace 80a6ea90b0ea2a03 ]---
> [  146.082130] RIP: 0010:xfs_inode_hasattr+0x19/0x30
> [  146.083144] Code: 83 c6 05 b2 55 75 02 01 e8 39 40 e4 00 eb b6 66 90 31 c0 80
> [  146.086831] RSP: 0018:ffffc900070eba08 EFLAGS: 00010206
> [  146.087816] RAX: ffffffff00ff0000 RBX: 0000000000000000 RCX: 0000000000000001
> [  146.089122] RDX: 0000000000000000 RSI: ffffffff82fdd33f RDI: ffff88810dbe16c0
> [  146.090477] RBP: ffff88810dbe16c0 R08: ffff888110e14348 R09: ffff888110e14348
> [  146.091794] R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000000
> [  146.093096] R13: ffff888110d99000 R14: ffff888110d99000 R15: ffffffff834acd60
> [  146.094429] FS:  00007f2bf29d7700(0000) GS:ffff88813bd80000(0000) knlGS:00000
> [  146.096002] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  146.097079] CR2: 0000000000000020 CR3: 0000000110c96000 CR4: 00000000000006e0
> [  146.098479] Kernel panic - not syncing: Fatal exception
> [  146.099677] Kernel Offset: disabled
> [  146.100397] ---[ end Kernel panic - not syncing: Fatal exception ]---
> 
> 


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

* Re: [patch 07/54] mm/slub: use stackdepot to save stack trace in objects
  2021-07-16  8:57     ` Vlastimil Babka
@ 2021-07-16  9:12       ` Christoph Hellwig
  0 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2021-07-16  9:12 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: Christoph Hellwig, Andrew Morton, cl, glittao, iamjoonsoo.kim,
	linux-mm, mm-commits, penberg, rdunlap, rientjes, torvalds,
	linux-xfs

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

On Fri, Jul 16, 2021 at 10:57:51AM +0200, Vlastimil Babka wrote:
> On 7/16/21 9:39 AM, Christoph Hellwig wrote:
> > This somewhat unexpectedly causes a crash when running the xfs/433 test
> > in xfstests for me.  Reverting the commit fixes the problem:
> 
> That's weird, the backtrace doesn't even include SLUB/stackdepot code.
> Is that kernel actually booted with slub_debug option/built with
> CONFIG_SLUB_DEBUG_ON or some cache created with SLAB_STORE_USER ?

CONFIG_SLUB_DEBUG_ON is enabled, yes.  Full .config attached.

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 35959 bytes --]

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

* Re: [patch 07/54] mm/slub: use stackdepot to save stack trace in objects
  2021-07-16  7:39   ` [patch 07/54] mm/slub: use stackdepot to save stack trace in objects Christoph Hellwig
  2021-07-16  8:57     ` Vlastimil Babka
@ 2021-07-16 20:12     ` Linus Torvalds
  2021-07-16 22:37       ` Vlastimil Babka
  1 sibling, 1 reply; 8+ messages in thread
From: Linus Torvalds @ 2021-07-16 20:12 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Andrew Morton, Christoph Lameter, glittao, Joonsoo Kim, Linux-MM,
	mm-commits, Pekka Enberg, Randy Dunlap, David Rientjes,
	Vlastimil Babka, linux-xfs

On Fri, Jul 16, 2021 at 12:39 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> This somewhat unexpectedly causes a crash when running the xfs/433 test
> in xfstests for me.  Reverting the commit fixes the problem:

I don't see why that would be the case, but I'm inclined to revert
that commit for another reason: the code doesn't seem to match the
description of the commit.

It used to be that CONFIG_SLUB_DEBUG was a config option that was
harmless and that defaulted to 'y' because there was little downside.
In fact, it's not just "default y", it doesn't even *ask* the user
unless CONFIG_EXPERT is on. Because it was fairly harmless. And then
SLOB_DEBUG_ON was that "do you actually want this code _enabled_".

But now it basically force-enables that STACKDEPOT support too, and
then instead of having an _optional_ CONFIG_STACKTRACE, you basically
have that as being forced on you whether you want active debugging or
not.

Maybe that

        select STACKDEPOT if STACKTRACE_SUPPORT

should have been

        select STACKDEPOT if STACKTRACE

because i\t used to be that CONFIG_STACKTRACE was somewhat unusual,
and only enabled for special debug cases (admittedly "CONFIG_TRACING"
likely meant that it was fairly widely enabled).

In contrast, STACKTRACE_SUPPORT is basically "this architecture supports it".

So now it seems STACKDEPOT is enabled basically unconditionally.

So I really don't see why it would cause that xfs issue, but I think
there are multiple reasons to just go "Hmm" on that commit.

Comments?

                Linus

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

* Re: [patch 07/54] mm/slub: use stackdepot to save stack trace in objects
  2021-07-16 20:12     ` Linus Torvalds
@ 2021-07-16 22:37       ` Vlastimil Babka
  2021-07-17 17:34         ` Randy Dunlap
  0 siblings, 1 reply; 8+ messages in thread
From: Vlastimil Babka @ 2021-07-16 22:37 UTC (permalink / raw)
  To: Linus Torvalds, Christoph Hellwig
  Cc: Andrew Morton, Christoph Lameter, glittao, Joonsoo Kim, Linux-MM,
	mm-commits, Pekka Enberg, Randy Dunlap, David Rientjes,
	linux-xfs, Geert Uytterhoeven

On 7/16/21 10:12 PM, Linus Torvalds wrote:
> On Fri, Jul 16, 2021 at 12:39 AM Christoph Hellwig <hch@infradead.org> wrote:
>>
>> This somewhat unexpectedly causes a crash when running the xfs/433 test
>> in xfstests for me.  Reverting the commit fixes the problem:
> 
> I don't see why that would be the case, but I'm inclined to revert
> that commit for another reason: the code doesn't seem to match the
> description of the commit.
> 
> It used to be that CONFIG_SLUB_DEBUG was a config option that was
> harmless and that defaulted to 'y' because there was little downside.
> In fact, it's not just "default y", it doesn't even *ask* the user
> unless CONFIG_EXPERT is on. Because it was fairly harmless. And then
> SLOB_DEBUG_ON was that "do you actually want this code _enabled_".
> 
> But now it basically force-enables that STACKDEPOT support too, and
> then instead of having an _optional_ CONFIG_STACKTRACE, you basically
> have that as being forced on you whether you want active debugging or
> not.
> 
> Maybe that
> 
>         select STACKDEPOT if STACKTRACE_SUPPORT
> 
> should have been
> 
>         select STACKDEPOT if STACKTRACE

I recall we tried that and run into KConfig recursive dependency hell as
"config STACKDEPOT" does "select STACKTRACE", and after some attempts
ended up with the above.

> because i\t used to be that CONFIG_STACKTRACE was somewhat unusual,
> and only enabled for special debug cases (admittedly "CONFIG_TRACING"
> likely meant that it was fairly widely enabled).
> 
> In contrast, STACKTRACE_SUPPORT is basically "this architecture supports it".
> 
> So now it seems STACKDEPOT is enabled basically unconditionally.

It seemed rather harmless as it was just a bit of extra code. But it's
true Geert reports [1] unexpected memory usage which I would have only
expected if actual stacks started to be collected. So I guess we'll have
to look into that.

[1]
https://lore.kernel.org/lkml/CAMuHMdW=eoVzM1Re5FVoEN87nKfiLmM2+Ah7eNu2KXEhCvbZyA@mail.gmail.com/

> So I really don't see why it would cause that xfs issue, but I think
> there are multiple reasons to just go "Hmm" on that commit.
> 
> Comments?
> 
>                 Linus
> 


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

* Re: [patch 07/54] mm/slub: use stackdepot to save stack trace in objects
  2021-07-16 22:37       ` Vlastimil Babka
@ 2021-07-17 17:34         ` Randy Dunlap
  2021-07-18  7:29           ` Vlastimil Babka
  0 siblings, 1 reply; 8+ messages in thread
From: Randy Dunlap @ 2021-07-17 17:34 UTC (permalink / raw)
  To: Vlastimil Babka, Linus Torvalds, Christoph Hellwig
  Cc: Andrew Morton, Christoph Lameter, glittao, Joonsoo Kim, Linux-MM,
	mm-commits, Pekka Enberg, David Rientjes, linux-xfs,
	Geert Uytterhoeven

On 7/16/21 3:37 PM, Vlastimil Babka wrote:
> On 7/16/21 10:12 PM, Linus Torvalds wrote:
>> On Fri, Jul 16, 2021 at 12:39 AM Christoph Hellwig <hch@infradead.org> wrote:
>>>
>>> This somewhat unexpectedly causes a crash when running the xfs/433 test
>>> in xfstests for me.  Reverting the commit fixes the problem:
>>
>> I don't see why that would be the case, but I'm inclined to revert
>> that commit for another reason: the code doesn't seem to match the
>> description of the commit.
>>
>> It used to be that CONFIG_SLUB_DEBUG was a config option that was
>> harmless and that defaulted to 'y' because there was little downside.
>> In fact, it's not just "default y", it doesn't even *ask* the user
>> unless CONFIG_EXPERT is on. Because it was fairly harmless. And then
>> SLOB_DEBUG_ON was that "do you actually want this code _enabled_".
>>
>> But now it basically force-enables that STACKDEPOT support too, and
>> then instead of having an _optional_ CONFIG_STACKTRACE, you basically
>> have that as being forced on you whether you want active debugging or
>> not.
>>
>> Maybe that
>>
>>         select STACKDEPOT if STACKTRACE_SUPPORT
>>
>> should have been
>>
>>         select STACKDEPOT if STACKTRACE
> 
> I recall we tried that and run into KConfig recursive dependency hell as
> "config STACKDEPOT" does "select STACKTRACE", and after some attempts
> ended up with the above.
> 
>> because i\t used to be that CONFIG_STACKTRACE was somewhat unusual,
>> and only enabled for special debug cases (admittedly "CONFIG_TRACING"
>> likely meant that it was fairly widely enabled).
>>
>> In contrast, STACKTRACE_SUPPORT is basically "this architecture supports it".
>>
>> So now it seems STACKDEPOT is enabled basically unconditionally.
> 
> It seemed rather harmless as it was just a bit of extra code. But it's
> true Geert reports [1] unexpected memory usage which I would have only
> expected if actual stacks started to be collected. So I guess we'll have
> to look into that.
> 
> [1]
> https://lore.kernel.org/lkml/CAMuHMdW=eoVzM1Re5FVoEN87nKfiLmM2+Ah7eNu2KXEhCvbZyA@mail.gmail.com/
> 
>> So I really don't see why it would cause that xfs issue, but I think
>> there are multiple reasons to just go "Hmm" on that commit.
>>
>> Comments?
>>
>>                 Linus
>>
> 

There is also the matter of lib/stackdepot.c build errors on ARCH=arc:

https://lore.kernel.org/lkml/202107150600.LkGNb4Vb-lkp@intel.com/


-- 
~Randy


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

* Re: [patch 07/54] mm/slub: use stackdepot to save stack trace in objects
  2021-07-17 17:34         ` Randy Dunlap
@ 2021-07-18  7:29           ` Vlastimil Babka
  2021-07-18 14:17             ` Randy Dunlap
  0 siblings, 1 reply; 8+ messages in thread
From: Vlastimil Babka @ 2021-07-18  7:29 UTC (permalink / raw)
  To: Randy Dunlap, Linus Torvalds, Christoph Hellwig
  Cc: Andrew Morton, Christoph Lameter, glittao, Joonsoo Kim, Linux-MM,
	mm-commits, Pekka Enberg, David Rientjes, linux-xfs,
	Geert Uytterhoeven

On 7/17/21 7:34 PM, Randy Dunlap wrote:
>>> because i\t used to be that CONFIG_STACKTRACE was somewhat unusual,
>>> and only enabled for special debug cases (admittedly "CONFIG_TRACING"
>>> likely meant that it was fairly widely enabled).
>>>
>>> In contrast, STACKTRACE_SUPPORT is basically "this architecture supports it".
>>>
>>> So now it seems STACKDEPOT is enabled basically unconditionally.
>>
>> It seemed rather harmless as it was just a bit of extra code. But it's
>> true Geert reports [1] unexpected memory usage which I would have only
>> expected if actual stacks started to be collected. So I guess we'll have
>> to look into that.
>>
>> [1]
>> https://lore.kernel.org/lkml/CAMuHMdW=eoVzM1Re5FVoEN87nKfiLmM2+Ah7eNu2KXEhCvbZyA@mail.gmail.com/
>>
>>> So I really don't see why it would cause that xfs issue, but I think
>>> there are multiple reasons to just go "Hmm" on that commit.
>>>
>>> Comments?
>>>
>>>                 Linus
>>>
>>
> 
> There is also the matter of lib/stackdepot.c build errors on ARCH=arc:
> 
> https://lore.kernel.org/lkml/202107150600.LkGNb4Vb-lkp@intel.com/

That's being fixed AFAIK?

https://lore.kernel.org/lkml/20210710145033.2804047-1-linux@roeck-us.net/

I'll try to come up with some KConfig flag set that will make it depend
on STRACKTRACE again without recursion issues.

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

* Re: [patch 07/54] mm/slub: use stackdepot to save stack trace in objects
  2021-07-18  7:29           ` Vlastimil Babka
@ 2021-07-18 14:17             ` Randy Dunlap
  0 siblings, 0 replies; 8+ messages in thread
From: Randy Dunlap @ 2021-07-18 14:17 UTC (permalink / raw)
  To: Vlastimil Babka, Linus Torvalds, Christoph Hellwig
  Cc: Andrew Morton, Christoph Lameter, glittao, Joonsoo Kim, Linux-MM,
	mm-commits, Pekka Enberg, David Rientjes, linux-xfs,
	Geert Uytterhoeven

On 7/18/21 12:29 AM, Vlastimil Babka wrote:
> On 7/17/21 7:34 PM, Randy Dunlap wrote:
>>>> because i\t used to be that CONFIG_STACKTRACE was somewhat unusual,
>>>> and only enabled for special debug cases (admittedly "CONFIG_TRACING"
>>>> likely meant that it was fairly widely enabled).
>>>>
>>>> In contrast, STACKTRACE_SUPPORT is basically "this architecture supports it".
>>>>
>>>> So now it seems STACKDEPOT is enabled basically unconditionally.
>>>
>>> It seemed rather harmless as it was just a bit of extra code. But it's
>>> true Geert reports [1] unexpected memory usage which I would have only
>>> expected if actual stacks started to be collected. So I guess we'll have
>>> to look into that.
>>>
>>> [1]
>>> https://lore.kernel.org/lkml/CAMuHMdW=eoVzM1Re5FVoEN87nKfiLmM2+Ah7eNu2KXEhCvbZyA@mail.gmail.com/
>>>
>>>> So I really don't see why it would cause that xfs issue, but I think
>>>> there are multiple reasons to just go "Hmm" on that commit.
>>>>
>>>> Comments?
>>>>
>>>>                 Linus
>>>>
>>>
>>
>> There is also the matter of lib/stackdepot.c build errors on ARCH=arc:
>>
>> https://lore.kernel.org/lkml/202107150600.LkGNb4Vb-lkp@intel.com/
> 
> That's being fixed AFAIK?
> 
> https://lore.kernel.org/lkml/20210710145033.2804047-1-linux@roeck-us.net/

Ah, thanks.

> I'll try to come up with some KConfig flag set that will make it depend
> on STRACKTRACE again without recursion issues.
> 


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

end of thread, other threads:[~2021-07-18 14:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210707175950.eceddb86c6c555555d4730e2@linux-foundation.org>
     [not found] ` <20210708010747.zIP9yxsci%akpm@linux-foundation.org>
2021-07-16  7:39   ` [patch 07/54] mm/slub: use stackdepot to save stack trace in objects Christoph Hellwig
2021-07-16  8:57     ` Vlastimil Babka
2021-07-16  9:12       ` Christoph Hellwig
2021-07-16 20:12     ` Linus Torvalds
2021-07-16 22:37       ` Vlastimil Babka
2021-07-17 17:34         ` Randy Dunlap
2021-07-18  7:29           ` Vlastimil Babka
2021-07-18 14:17             ` Randy Dunlap

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).