All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
To: Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org,
	Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>,
	Michal Hocko <mhocko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Vladimir Davydov
	<vdavydov.dev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Andrew Morton
	<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>,
	Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Subject: Re: [RFC PATCH 1/3] mm/memcg: Protect per-CPU counter by disabling preemption on PREEMPT_RT
Date: Thu, 23 Dec 2021 08:34:19 +0100	[thread overview]
Message-ID: <YcQme8BPFl7P9T02@linutronix.de> (raw)
In-Reply-To: <bdfc9791-4af2-f4fb-9ef5-dab1e2e3ff89-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On 2021-12-22 21:31:36 [-0500], Waiman Long wrote:
> On 12/22/21 06:41, Sebastian Andrzej Siewior wrote:
> > The per-CPU counter are modified with the non-atomic modifier. The
> > consistency is ensure by disabling interrupts for the update.
> > This breaks on PREEMPT_RT because some sections additionally
> > acquire a spinlock_t lock (which becomes sleeping and must not be
> > acquired with disabled interrupts). Another problem is that
> > mem_cgroup_swapout() expects to be invoked with disabled interrupts
> > because the caller has to acquire a spinlock_t which is acquired with
> > disabled interrupts. Since spinlock_t never disables interrupts on
> > PREEMPT_RT the interrupts are never disabled at this point.
> > 
> > The code is never called from in_irq() context on PREEMPT_RT therefore
> 
> How do you guarantee that these percpu update functions won't be called in
> in_irq() context for PREEMPT_RT? Do you think we should add a
> WARN_ON_ONCE(in_irq()) just to be sure?

There are no invocations to the memory allocator (neither malloc() nor
free()) on RT and the memory allocator itself (SLUB and the
page-allocator so both) has sleeping locks. That means invocations
in_atomic() are bad. All interrupt handler are force-threaded. Those
which are not (like timer, per-CPU interrupts or those which explicitly
asked not to be force threaded) are limited in their doing as they can't
invoke anything that has a sleeping lock. Lockdep or
CONFIG_DEBUG_ATOMIC_SLEEP will yell here.
The other counter are protected the same way, see
  c68ed7945701a ("mm/vmstat: protect per cpu variables with preempt disable on RT")

> Cheers,
> Longman

Sebastian

WARNING: multiple messages have this Message-ID (diff)
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Waiman Long <longman@redhat.com>
Cc: cgroups@vger.kernel.org, linux-mm@kvack.org,
	Johannes Weiner <hannes@cmpxchg.org>,
	Michal Hocko <mhocko@kernel.org>,
	Vladimir Davydov <vdavydov.dev@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: [RFC PATCH 1/3] mm/memcg: Protect per-CPU counter by disabling preemption on PREEMPT_RT
Date: Thu, 23 Dec 2021 08:34:19 +0100	[thread overview]
Message-ID: <YcQme8BPFl7P9T02@linutronix.de> (raw)
In-Reply-To: <bdfc9791-4af2-f4fb-9ef5-dab1e2e3ff89@redhat.com>

On 2021-12-22 21:31:36 [-0500], Waiman Long wrote:
> On 12/22/21 06:41, Sebastian Andrzej Siewior wrote:
> > The per-CPU counter are modified with the non-atomic modifier. The
> > consistency is ensure by disabling interrupts for the update.
> > This breaks on PREEMPT_RT because some sections additionally
> > acquire a spinlock_t lock (which becomes sleeping and must not be
> > acquired with disabled interrupts). Another problem is that
> > mem_cgroup_swapout() expects to be invoked with disabled interrupts
> > because the caller has to acquire a spinlock_t which is acquired with
> > disabled interrupts. Since spinlock_t never disables interrupts on
> > PREEMPT_RT the interrupts are never disabled at this point.
> > 
> > The code is never called from in_irq() context on PREEMPT_RT therefore
> 
> How do you guarantee that these percpu update functions won't be called in
> in_irq() context for PREEMPT_RT? Do you think we should add a
> WARN_ON_ONCE(in_irq()) just to be sure?

There are no invocations to the memory allocator (neither malloc() nor
free()) on RT and the memory allocator itself (SLUB and the
page-allocator so both) has sleeping locks. That means invocations
in_atomic() are bad. All interrupt handler are force-threaded. Those
which are not (like timer, per-CPU interrupts or those which explicitly
asked not to be force threaded) are limited in their doing as they can't
invoke anything that has a sleeping lock. Lockdep or
CONFIG_DEBUG_ATOMIC_SLEEP will yell here.
The other counter are protected the same way, see
  c68ed7945701a ("mm/vmstat: protect per cpu variables with preempt disable on RT")

> Cheers,
> Longman

Sebastian


  parent reply	other threads:[~2021-12-23  7:34 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-22 11:41 [RFC PATCH 0/3] mm/memcg: Address PREEMPT_RT problems instead of disabling it Sebastian Andrzej Siewior
2021-12-22 11:41 ` Sebastian Andrzej Siewior
     [not found] ` <20211222114111.2206248-1-bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
2021-12-22 11:41   ` [RFC PATCH 1/3] mm/memcg: Protect per-CPU counter by disabling preemption on PREEMPT_RT Sebastian Andrzej Siewior
2021-12-22 11:41     ` Sebastian Andrzej Siewior
     [not found]     ` <20211222114111.2206248-2-bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
2021-12-23  2:31       ` Waiman Long
2021-12-23  2:31         ` Waiman Long
     [not found]         ` <bdfc9791-4af2-f4fb-9ef5-dab1e2e3ff89-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2021-12-23  7:34           ` Sebastian Andrzej Siewior [this message]
2021-12-23  7:34             ` Sebastian Andrzej Siewior
     [not found]             ` <YcQme8BPFl7P9T02-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
2021-12-23 16:01               ` Waiman Long
2021-12-23 16:01                 ` Waiman Long
2022-01-05 14:16       ` Michal Koutný
2022-01-05 14:16         ` Michal Koutný
     [not found]         ` <20220105141653.GA6464-9OudH3eul5jcvrawFnH+a6VXKuFTiq87@public.gmane.org>
2022-01-13 13:08           ` Sebastian Andrzej Siewior
2022-01-13 13:08             ` Sebastian Andrzej Siewior
     [not found]             ` <YeAkOm0YsAe5jFRb-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
2022-01-13 14:48               ` Michal Koutný
2022-01-13 14:48                 ` Michal Koutný
     [not found]                 ` <20220113144803.GB28468-9OudH3eul5jcvrawFnH+a6VXKuFTiq87@public.gmane.org>
2022-01-14  9:09                   ` Sebastian Andrzej Siewior
2022-01-14  9:09                     ` Sebastian Andrzej Siewior
     [not found]                     ` <YeE9zyUokSY9L2ZI-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
2022-01-18 18:26                       ` [PATCH] mm/memcg: Do not check v1 event counter when not needed Michal Koutný
2022-01-18 18:26                         ` Michal Koutný
     [not found]                         ` <20220118182600.15007-1-mkoutny-IBi9RG/b67k@public.gmane.org>
2022-01-18 19:57                           ` Sebastian Andrzej Siewior
2022-01-18 19:57                             ` Sebastian Andrzej Siewior
2021-12-22 11:41   ` [RFC PATCH 2/3] mm/memcg: Add a local_lock_t for IRQ and TASK object Sebastian Andrzej Siewior
2021-12-22 11:41     ` Sebastian Andrzej Siewior
     [not found]     ` <20211222114111.2206248-3-bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
2021-12-23 21:38       ` Waiman Long
2021-12-23 21:38         ` Waiman Long
     [not found]         ` <4fe30c89-df34-bbdb-a9a1-5519e0363cc5-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2022-01-03 16:34           ` Sebastian Andrzej Siewior
2022-01-03 16:34             ` Sebastian Andrzej Siewior
     [not found]             ` <YdMlrFPvb94rzv8Z-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
2022-01-03 17:09               ` Waiman Long
2022-01-03 17:09                 ` Waiman Long
2021-12-22 11:41   ` [RFC PATCH 3/3] mm/memcg: Allow the task_obj optimization only on non-PREEMPTIBLE kernels Sebastian Andrzej Siewior
2021-12-22 11:41     ` Sebastian Andrzej Siewior
     [not found]     ` <20211222114111.2206248-4-bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
2021-12-23 21:48       ` Waiman Long
2021-12-23 21:48         ` Waiman Long
     [not found]         ` <f6bb93c8-3940-6141-d0e0-50144549a4f5-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2022-01-03 14:44           ` Sebastian Andrzej Siewior
2022-01-03 14:44             ` Sebastian Andrzej Siewior
     [not found]             ` <YdML2zaU17clEZgt-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
2022-01-03 15:04               ` Waiman Long
2022-01-03 15:04                 ` Waiman Long
     [not found]                 ` <df637005-6c72-a1c6-c6b9-70f81f74884d-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2022-01-05 20:22                   ` Sebastian Andrzej Siewior
2022-01-05 20:22                     ` Sebastian Andrzej Siewior
     [not found]                     ` <YdX+INO9gQje6d0S-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
2022-01-06  3:28                       ` Waiman Long
2022-01-06  3:28                         ` Waiman Long
     [not found]                         ` <29457251-cf4f-4c7d-b36d-c2a0af4da707-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2022-01-13 15:26                           ` Sebastian Andrzej Siewior
2022-01-13 15:26                             ` Sebastian Andrzej Siewior
2022-01-05 14:59   ` [RFC PATCH 0/3] mm/memcg: Address PREEMPT_RT problems instead of disabling it Michal Koutný
2022-01-05 14:59     ` Michal Koutný
     [not found]     ` <20220105145956.GB6464-9OudH3eul5jcvrawFnH+a6VXKuFTiq87@public.gmane.org>
2022-01-05 15:06       ` Sebastian Andrzej Siewior
2022-01-05 15:06         ` Sebastian Andrzej Siewior

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=YcQme8BPFl7P9T02@linutronix.de \
    --to=bigeasy-hfztesqfncyowbw4kg4ksq@public.gmane.org \
    --cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    --cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org \
    --cc=linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org \
    --cc=longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=mhocko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org \
    --cc=tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org \
    --cc=vdavydov.dev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.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.