Skip to content

Commit

Permalink
cgroup: Use kernel command line to disable memory cgroup
Browse files Browse the repository at this point in the history
Commit 94a23e9 ("cgroup: Disable cgroup "memory" by default")
disabled the memory cgroup by default when initing the cgroups. However,
it's possible to disable the memory cgroup by a kernel command line.
Hard-coding such a feature can be problematic as some memory management
features depend on the order that things are set.

For example, it is possible to see a NULL pointer dereference caused by
commit 94a23e9. The NULL pointer
dereference is triggered by the memory shrinker and ends up in a kernel
crash.

[   50.028629] ==================================================================
[   50.028645] BUG: KASAN: null-ptr-deref in do_shrink_slab+0x1fc/0x978
[   50.028663] Write of size 8 at addr 0000000000000000 by task gfxrecon-replay/1965

[   50.028676] CPU: 3 UID: 1000 PID: 1965 Comm: gfxrecon-replay Tainted: G         C         6.12.0-rc4-v8-thp-kasan+ #85
[   50.028685] Tainted: [C]=CRAP
[   50.028689] Hardware name: Raspberry Pi 5 Model B Rev 1.0 (DT)
[   50.028694] Call trace:
[   50.028697]  dump_backtrace+0xfc/0x120
[   50.028706]  show_stack+0x24/0x38
[   50.028711]  dump_stack_lvl+0x40/0x88
[   50.028720]  print_report+0xe4/0x708
[   50.028728]  kasan_report+0xcc/0x130
[   50.028733]  kasan_check_range+0x254/0x298
[   50.028738]  __kasan_check_write+0x20/0x30
[   50.028745]  do_shrink_slab+0x1fc/0x978
[   50.028751]  shrink_slab+0x318/0xc38
[   50.028756]  shrink_one+0x254/0x6d8
[   50.028762]  shrink_node+0x26b4/0x2848
[   50.028767]  do_try_to_free_pages+0x3e4/0x1190
[   50.028773]  try_to_free_pages+0x5a4/0xb40
[   50.028778]  __alloc_pages_direct_reclaim+0x144/0x298
[   50.028787]  __alloc_pages_slowpath+0x5c4/0xc70
[   50.028793]  __alloc_pages_noprof+0x4a8/0x6a8
[   50.028800]  __folio_alloc_noprof+0x24/0xa8
[   50.028806]  shmem_alloc_and_add_folio+0x2ec/0xce0
[   50.028812]  shmem_get_folio_gfp+0x380/0xc20
[   50.028818]  shmem_read_folio_gfp+0xe0/0x160
[   50.028824]  drm_gem_get_pages+0x238/0x620 [drm]
[   50.029039]  drm_gem_shmem_get_pages_sgt+0xd8/0x4b8 [drm_shmem_helper]
[   50.029053]  v3d_bo_create_finish+0x58/0x1e0 [v3d]
[   50.029083]  v3d_create_bo_ioctl+0xac/0x210 [v3d]
[   50.029105]  drm_ioctl_kernel+0x1d8/0x2b8 [drm]
[   50.029220]  drm_ioctl+0x4b4/0x920 [drm]
[   50.029330]  __arm64_sys_ioctl+0x11c/0x160
[   50.029337]  invoke_syscall+0x88/0x268
[   50.029345]  el0_svc_common+0x160/0x1d8
[   50.029351]  do_el0_svc+0x50/0x68
[   50.029358]  el0_svc+0x34/0x80
[   50.029364]  el0t_64_sync_handler+0x84/0x100
[   50.029371]  el0t_64_sync+0x190/0x198
[   50.029376] ==================================================================

This happens because the memory shrinker is unaware that we are
artificially disabling the memory cgroups and therefore it doesn't
allocate `nr_deferred` (as it would if we used the kernel command line).

To avoid such an issue, revert the artificial disablement and disable it
through the command line. If a user wants to enable the feature, it can
use the `cgroup_enable=` command line.

Signed-off-by: Maíra Canal <[email protected]>
  • Loading branch information
mairacanal authored and pelwell committed Oct 25, 2024
1 parent 8334494 commit 86099de
Show file tree
Hide file tree
Showing 7 changed files with 7 additions and 20 deletions.
2 changes: 1 addition & 1 deletion arch/arm/boot/dts/broadcom/bcm2708-rpi-bt.dtsi
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

/ {
chosen {
bootargs = "coherent_pool=1M 8250.nr_uarts=1 snd_bcm2835.enable_headphones=0";
bootargs = "coherent_pool=1M 8250.nr_uarts=1 snd_bcm2835.enable_headphones=0 cgroup_disable=memory";
};

aliases {
Expand Down
2 changes: 1 addition & 1 deletion arch/arm/boot/dts/broadcom/bcm270x.dtsi
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
/ {
chosen: chosen {
// Disable audio by default
bootargs = "coherent_pool=1M snd_bcm2835.enable_headphones=0";
bootargs = "coherent_pool=1M snd_bcm2835.enable_headphones=0 cgroup_disable=memory";
stdout-path = "serial0:115200n8";
};

Expand Down
2 changes: 1 addition & 1 deletion arch/arm/boot/dts/broadcom/bcm2711-rpi-cm4s.dts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@

/ {
chosen {
bootargs = "coherent_pool=1M snd_bcm2835.enable_headphones=0 numa_policy=interleave";
bootargs = "coherent_pool=1M snd_bcm2835.enable_headphones=0 cgroup_disable=memory numa_policy=interleave";
};

aliases {
Expand Down
2 changes: 1 addition & 1 deletion arch/arm/boot/dts/broadcom/bcm2711-rpi-ds.dtsi
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

/ {
chosen {
bootargs = "coherent_pool=1M 8250.nr_uarts=1 snd_bcm2835.enable_headphones=0 numa_policy=interleave";
bootargs = "coherent_pool=1M 8250.nr_uarts=1 snd_bcm2835.enable_headphones=0 cgroup_disable=memory numa_policy=interleave";
};

__overrides__ {
Expand Down
2 changes: 1 addition & 1 deletion arch/arm/boot/dts/broadcom/bcm271x-rpi-bt.dtsi
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

/ {
chosen {
bootargs = "coherent_pool=1M 8250.nr_uarts=1 snd_bcm2835.enable_headphones=0";
bootargs = "coherent_pool=1M 8250.nr_uarts=1 snd_bcm2835.enable_headphones=0 cgroup_disable=memory";
};

aliases {
Expand Down
2 changes: 1 addition & 1 deletion arch/arm64/boot/dts/broadcom/bcm2712-rpi.dtsi
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@

/ {
chosen: chosen {
bootargs = "reboot=w coherent_pool=1M 8250.nr_uarts=1 pci=pcie_bus_safe numa_policy=interleave iommu_dma_numa_policy=interleave system_heap.max_order=0";
bootargs = "reboot=w coherent_pool=1M 8250.nr_uarts=1 pci=pcie_bus_safe cgroup_disable=memory numa_policy=interleave iommu_dma_numa_policy=interleave system_heap.max_order=0";
stdout-path = "serial10:115200n8";
};

Expand Down
15 changes: 1 addition & 14 deletions kernel/cgroup/cgroup.c
Original file line number Diff line number Diff line change
Expand Up @@ -6060,9 +6060,6 @@ int __init cgroup_init_early(void)
return 0;
}

static u16 cgroup_enable_mask __initdata;
static int __init cgroup_disable(char *str);

/**
* cgroup_init - cgroup initialization
*
Expand Down Expand Up @@ -6096,12 +6093,6 @@ int __init cgroup_init(void)

cgroup_unlock();

/*
* Apply an implicit disable, knowing that an explicit enable will
* prevent if from doing anything.
*/
cgroup_disable("memory");

for_each_subsys(ss, ssid) {
if (ss->early_init) {
struct cgroup_subsys_state *css =
Expand Down Expand Up @@ -6742,10 +6733,6 @@ static int __init cgroup_disable(char *str)
strcmp(token, ss->legacy_name))
continue;

/* An explicit cgroup_enable overrides a disable */
if (cgroup_enable_mask & (1 << i))
continue;

static_branch_disable(cgroup_subsys_enabled_key[i]);
pr_info("Disabling %s control group subsystem\n",
ss->name);
Expand Down Expand Up @@ -6779,7 +6766,7 @@ static int __init cgroup_enable(char *str)
strcmp(token, ss->legacy_name))
continue;

cgroup_enable_mask |= 1 << i;
cgroup_feature_disable_mask &= ~(1 << i);
static_branch_enable(cgroup_subsys_enabled_key[i]);
pr_info("Enabling %s control group subsystem\n",
ss->name);
Expand Down

0 comments on commit 86099de

Please sign in to comment.