All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shakeel Butt <shakeel.butt@linux.dev>
To: Dev Jain <dev.jain@arm.com>
Cc: Harry Yoo <harry.yoo@oracle.com>,
	 Andrew Morton <akpm@linux-foundation.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	 Michal Hocko <mhocko@kernel.org>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	 Muchun Song <muchun.song@linux.dev>,
	Qi Zheng <qi.zheng@linux.dev>, Vlastimil Babka <vbabka@suse.cz>,
	 linux-mm@kvack.org, cgroups@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	 Meta kernel team <kernel-team@meta.com>
Subject: Re: [PATCH 1/4] memcg: use mod_node_page_state to update stats
Date: Tue, 10 Feb 2026 08:29:58 -0800	[thread overview]
Message-ID: <aYtbevHEwx_3fn0Q@linux.dev> (raw)
In-Reply-To: <b77dc11e-fe09-4f0c-a912-d05faa01ff1c@arm.com>

On Tue, Feb 10, 2026 at 01:08:49PM +0530, Dev Jain wrote:
[...]
> 
> >>
> > Oh so it is arm64 specific issue. I tested on x86-64 machine and it solves
> > the little regression it had before. So, on arm64 all this_cpu_ops i.e. without
> > double underscore, uses LL/SC instructions. 
> >
> > Need more thought on this. 
> >
> >>> Also can you confirm whether my analysis of the regression was correct?
> >>>  Because if it was, then this diff looks wrong - AFAIU preempt_disable()
> >>>  won't stop an irq handler from interrupting the execution, so this
> >>>  will introduce a bug for code paths running in irq context.
> >>>
> >> I was worried about the correctness too, but this_cpu_add() is safe
> >> against IRQs and so the stat will be _eventually_ consistent?
> >>
> >> Ofc it's so confusing! Maybe I'm the one confused.
> > Yeah there is no issue with proposed patch as it is making the function
> > re-entrant safe.
> 
> Ah yes, this_cpu_add() does the addition in one shot without read-modify-write.
> 
> I am still puzzled whether the original patch was a bug fix or an optimization.

The original patch was a cleanup patch. The memcg stats update functions
were already irq/nmi safe without disabling irqs and that patch did the
same for the numa stats. Though it seems like that is causing regression
for arm64 as this_cpu* ops are expensive on arm64. 

> The patch description says that node stat updation uses irq unsafe interface.
> Therefore, we had foo() calling __foo() nested with local_irq_save/restore. But
> there were code paths which directly called __foo() - so, your patch fixes a bug right

No, those places were already disabling irqs and should be fine.

I am working on adding batched stats update functionality in the hope
that will fix the regression.
> 

  reply	other threads:[~2026-02-10 16:30 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-10 23:20 [PATCH 0/4] memcg: cleanup the memcg stats interfaces Shakeel Butt
2025-11-10 23:20 ` [PATCH 1/4] memcg: use mod_node_page_state to update stats Shakeel Butt
2025-11-11  1:39   ` Harry Yoo
2025-11-11 18:58   ` Roman Gushchin
2026-01-29 13:05   ` Dev Jain
2026-02-02  4:26     ` Shakeel Butt
2026-02-02  4:48       ` Dev Jain
2026-02-02  4:54         ` Shakeel Butt
2026-02-02  8:53           ` Dev Jain
2026-02-04 20:38             ` Shakeel Butt
2026-02-05  5:20               ` Dev Jain
2026-02-05  5:45                 ` Harry Yoo
2026-02-05  5:58                   ` Shakeel Butt
2026-02-10  7:38                     ` Dev Jain
2026-02-10 16:29                       ` Shakeel Butt [this message]
2026-02-11  7:37                         ` Dev Jain
2026-02-11  8:53                           ` Harry Yoo
2026-02-11  9:24                             ` Shakeel Butt
2026-02-11 10:14                               ` Harry Yoo
2026-02-12  5:16                               ` Dev Jain
2026-02-12  5:14                             ` Dev Jain
2026-02-12  1:31                     ` Shakeel Butt
2025-11-10 23:20 ` [PATCH 2/4] memcg: remove __mod_lruvec_kmem_state Shakeel Butt
2025-11-11  1:46   ` Harry Yoo
2025-11-11  8:23   ` Qi Zheng
2025-11-11 18:58   ` Roman Gushchin
2025-11-10 23:20 ` [PATCH 3/4] memcg: remove __mod_lruvec_state Shakeel Butt
2025-11-11  5:21   ` Harry Yoo
2025-11-11 18:58   ` Roman Gushchin
2025-11-10 23:20 ` [PATCH 4/4] memcg: remove __lruvec_stat_mod_folio Shakeel Butt
2025-11-11  5:41   ` Harry Yoo
2025-11-11 18:59   ` Roman Gushchin
2025-11-11  0:59 ` [PATCH 0/4] memcg: cleanup the memcg stats interfaces Harry Yoo
2025-11-11  2:23   ` Qi Zheng
2025-11-11  2:39     ` Shakeel Butt
2025-11-11  2:48       ` Qi Zheng
2025-11-11  3:00         ` Shakeel Butt
2025-11-11  3:07           ` Qi Zheng
2025-11-11  3:18             ` Harry Yoo
2025-11-11  3:29               ` Qi Zheng
2025-11-11  3:05         ` Harry Yoo
2025-11-11  8:01           ` Sebastian Andrzej Siewior
2025-11-11  8:36 ` Qi Zheng
2025-11-11 16:45   ` Shakeel Butt
2025-11-12  2:11     ` Qi Zheng
2025-11-11  9:54 ` Vlastimil Babka
2025-11-11 19:01 ` Roman Gushchin
2025-11-11 19:34   ` Shakeel Butt
2025-11-15 19:27 ` Shakeel Butt

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=aYtbevHEwx_3fn0Q@linux.dev \
    --to=shakeel.butt@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=dev.jain@arm.com \
    --cc=hannes@cmpxchg.org \
    --cc=harry.yoo@oracle.com \
    --cc=kernel-team@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=muchun.song@linux.dev \
    --cc=qi.zheng@linux.dev \
    --cc=roman.gushchin@linux.dev \
    --cc=vbabka@suse.cz \
    /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.