All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org, tj@kernel.org,
	hannes@cmpxchg.org, mkoutny@suse.com
Subject: Re: [PATCH v6 3/6] cgroup/dmem: Add reclaim callback for lowering max below current usage
Date: Thu, 11 Jun 2026 18:01:07 +0000	[thread overview]
Message-ID: <20260611180107.C01451F00899@smtp.kernel.org> (raw)
In-Reply-To: <20260611173301.17473-4-thomas.hellstrom@linux.intel.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] dmemcg_parse_limit does not clamp parsed limits to PAGE_COUNTER_MAX, leading to silent truncation and broken limits on 32-bit systems.
--

commit 71a34045a2531fca277baab558f3a81add06614b
Author: Thomas Hellström <thomas.hellstrom@linux.intel.com>

cgroup/dmem: Add reclaim callback for lowering max below current usage

This patch adds an optional reclaim callback to dmem_cgroup_region to
allow memory eviction when dmem.max is lowered below current usage. It
handles reclaim in a bounded loop and respects O_NONBLOCK to avoid
charging reclaim costs to writers.

> diff --git a/kernel/cgroup/dmem.c b/kernel/cgroup/dmem.c
> index 373ad04d289d5..f7166b8255c7b 100644
> --- a/kernel/cgroup/dmem.c
> +++ b/kernel/cgroup/dmem.c
[ ... ]
> @@ -145,21 +170,71 @@ static void free_cg_pool(struct dmem_cgroup_pool_state *pool)
[ ... ]
>  static void
> -set_resource_max(struct dmem_cgroup_pool_state *pool, u64 val)
> +set_resource_max(struct dmem_cgroup_pool_state *pool, u64 val, bool nonblock)
>  {
> -	page_counter_set_max(&pool->cnt, val);
> +	struct dmem_cgroup_region *region = pool->region;
> +	unsigned long limit = (unsigned long)val;

[Severity: High]
This isn't a bug introduced by this patch, but does casting the u64 value
to unsigned long here cause issues on 32-bit systems?

It looks like dmemcg_parse_limit() parses user input into a u64 via
memparse() without clamping it to PAGE_COUNTER_MAX. The dmem controller uses
page_counter to track raw bytes rather than pages.

On 32-bit systems, page_counter tracks limits using unsigned long and usage
using atomic_long_t (signed), meaning it can safely track at most LONG_MAX
(2GB-1) bytes. If a user writes a limit larger than 4GB (e.g., 5GB), the
cast to unsigned long will silently truncate it to 1GB.

Additionally, if a value between 2GB and 4GB is written, does actual usage
exceeding 2GB cause atomic_long_sub_return() in page_counter_cancel() to
result in a negative value, triggering a WARN_ONCE and permanently
resetting the cgroup's usage to 0?

Would it be safer to clamp the parsed value to PAGE_COUNTER_MAX in
dmemcg_parse_limit() before it reaches this point?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611173301.17473-1-thomas.hellstrom@linux.intel.com?part=3

  reply	other threads:[~2026-06-11 18:01 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-11 17:32 [PATCH v6 0/6] [PATCH v6 0/6] Add reclaim to the dmem cgroup controller Thomas Hellström
2026-06-11 17:32 ` [PATCH v6 1/6] drm/amdgpu: Fix init ordering in amdgpu_vram_mgr_init() Thomas Hellström
2026-06-11 17:45   ` sashiko-bot
2026-06-11 17:32 ` [PATCH v6 2/6] cgroup/dmem: Introduce struct dmem_cgroup_init for region initialization Thomas Hellström
2026-06-11 17:32 ` [PATCH v6 3/6] cgroup/dmem: Add reclaim callback for lowering max below current usage Thomas Hellström
2026-06-11 18:01   ` sashiko-bot [this message]
2026-06-11 17:32 ` [PATCH v6 4/6] drm/ttm: Hook up a cgroup-aware reclaim callback for the dmem controller Thomas Hellström
2026-06-11 17:33 ` [PATCH v6 5/6] drm/xe: Wire up dmem cgroup reclaim for VRAM manager Thomas Hellström
2026-06-11 17:33 ` [PATCH v6 6/6] drm/amdgpu: " Thomas Hellström
2026-06-11 18:32   ` sashiko-bot
2026-06-11 19:41   ` Thomas Hellström
2026-06-11 20:00 ` ✗ CI.checkpatch: warning for Add reclaim to the dmem cgroup controller (rev6) Patchwork
2026-06-11 20:02 ` ✓ CI.KUnit: success " Patchwork
2026-06-11 20:49 ` ✓ Xe.CI.BAT: " Patchwork

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=20260611180107.C01451F00899@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hannes@cmpxchg.org \
    --cc=mkoutny@suse.com \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=thomas.hellstrom@linux.intel.com \
    --cc=tj@kernel.org \
    /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.