linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Michal Koutný" <mkoutny@suse.com>
To: "T.J. Mercier" <tjmercier@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
	Michal Hocko <mhocko@kernel.org>,
	 Roman Gushchin <roman.gushchin@linux.dev>,
	Shakeel Butt <shakeelb@google.com>,
	 Muchun Song <muchun.song@linux.dev>,
	Andrew Morton <akpm@linux-foundation.org>,
	android-mm@google.com,  Minchan Kim <minchan@google.com>,
	cgroups@vger.kernel.org, linux-mm@kvack.org,
	 linux-kernel@vger.kernel.org
Subject: Re: Re: [PATCH v2] mm: memcg: Don't periodically flush stats when memcg is disabled
Date: Fri, 2 Feb 2024 23:39:30 +0100	[thread overview]
Message-ID: <rlm36iypckvxol2edyr25jyo4imvlidtepbcjdaa2ouvwh3wjq@pqyuk3v2jesb> (raw)
In-Reply-To: <CABdmKX1b2GjrUmgTEq+tgwdYyqp_2qhs1G5AHBeKCNSfdbO8Eg@mail.gmail.com>

On Thu, Feb 01, 2024 at 01:02:04PM -0800, "T.J. Mercier" <tjmercier@google.com> wrote:
> It does blow up, but not how I was expecting. There's a null pointer
> dereference inside find_css_set when trying to get a css pointer for
> the memory controller, I think because the allocation in
> cgroup_init_subsys is skipped:

Thanks for trying! I suspected it won't be easy. At the same time I
suspected there must be a hook for your purpose -- after looking at
cpuset, I was reminded of cgroup_subsys.bind callback.
What about triggering periodic flush in that callback? (memcg doesn't
implement it yet but cgroup_init() takes it into account.)
It would take any dwork activation out of mem_cgroup_css_online() and it
seems cleaner. (Ideally, the flush could be disabled again when memcg
root is unmounted again. (That's impossible and practically unused but
that's why consider callback approach cleaner. Of course, your original
guard serves the purpose too.))

Regards,
Michal

      reply	other threads:[~2024-02-02 22:39 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-26 21:19 [PATCH v2] mm: memcg: Don't periodically flush stats when memcg is disabled T.J. Mercier
2024-01-26 21:28 ` Chris Li
2024-01-29 20:32 ` Yosry Ahmed
2024-02-01 14:26 ` Michal Koutný
2024-02-01 21:02   ` T.J. Mercier
2024-02-02 22:39     ` Michal Koutný [this message]

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=rlm36iypckvxol2edyr25jyo4imvlidtepbcjdaa2ouvwh3wjq@pqyuk3v2jesb \
    --to=mkoutny@suse.com \
    --cc=akpm@linux-foundation.org \
    --cc=android-mm@google.com \
    --cc=cgroups@vger.kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=minchan@google.com \
    --cc=muchun.song@linux.dev \
    --cc=roman.gushchin@linux.dev \
    --cc=shakeelb@google.com \
    --cc=tjmercier@google.com \
    /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 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).