linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Michal Hocko <mhocko@suse.com>
Cc: cgroups@vger.kernel.org, linux-mm@kvack.org,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Johannes Weiner" <hannes@cmpxchg.org>,
	"Michal Koutný" <mkoutny@suse.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Vladimir Davydov" <vdavydov.dev@gmail.com>,
	"Waiman Long" <longman@redhat.com>
Subject: Re: [PATCH 3/4] mm/memcg: Add a local_lock_t for IRQ and TASK object.
Date: Thu, 27 Jan 2022 12:53:40 +0100	[thread overview]
Message-ID: <YfKHxKda7bGJmrLJ@linutronix.de> (raw)
In-Reply-To: <YfFmxH1IXeegNOa9@dhcp22.suse.cz>

On 2022-01-26 16:20:36 [+0100], Michal Hocko wrote:
> I do not see any obvious problem with this patch. The code is ugly as
> hell, though, but a large part of that is because of the weird locking
> scheme we already have. I've had a look at 559271146efc ("mm/memcg:
> optimize user context object stock access") and while I agree that it
> makes sense to optimize for user context I do not really see any numbers
> justifying the awkward locking scheme. Is this complexity really worth
> it?

From https://https://lkml.kernel.org/r/.kernel.org/all/YdX+INO9gQje6d0S@linutronix.de/:

|        Sandy Bridge   Haswell        Skylake         AMD-A8 7100    Zen2           ARM64
|PREEMPT 5,123,896,822  5,215,055,226   5,077,611,590  6,012,287,874  6,234,674,489  20,000,000,100
|IRQ     7,494,119,638  6,810,367,629  10,620,130,377  4,178,546,086  4,898,076,012  13,538,461,925

basically if PREEMPT < IRQ then preempt_disable() + enable() was cheaper
than local_irq_save() + restore().

| Sandy Bridge
|                  SERVER OPT   SERVER NO-OPT    PREEMPT OPT     PREEMPT NO-OPT
| ALLOC/FREE    8,519,295,176   9,051,200,652    10,627,431,395  11,198,189,843
| SD                5,309,768      29,253,976       129,102,317      40,681,909
| ALLOC/FREE BH 9,996,704,330   8,927,026,031    11,680,149,900  11,139,356,465
| SD               38,237,534      72,913,120        23,626,932     116,413,331

OPT is code as-is while "NO-OPT" is with the following patch which
disables the optimisation (so it should be a revert of the optimisation
commit).

ALLOC/FREE is kfree(kmalloc()).
ALLOC/FREE BH is the same but in_interrupt() reported true.
The numbers are are time needed in ns for 100,000,000 iterations of the
free+alloc. SD is standard deviation.
I also let the test run on a Zen2 box:

|                  SERVER OPT   SERVER NO-OPT   PREEMPT OPT      PREEMPT NO-OPT
| ALLOC/FREE    8,126,735,313   8,751,307,383    9,822,927,142   10,045,105,425
| SD              100,806,471      87,234,047       55,170,179       25,832,386
| ALLOC/FREE BH 9,197,455,885   8,394,337,053   10,671,227,095    9,904,954,934
| SD              155,223,919      57,800,997       47,529,496      105,260,566

Is this what you asked for?

Sebastian


  reply	other threads:[~2022-01-27 11:53 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-25 16:43 [PATCH 0/4] mm/memcg: Address PREEMPT_RT problems instead of disabling it Sebastian Andrzej Siewior
2022-01-25 16:43 ` [PATCH 1/4] mm/memcg: Disable threshold event handlers on PREEMPT_RT Sebastian Andrzej Siewior
2022-01-26 14:40   ` Michal Hocko
2022-01-26 14:45     ` Sebastian Andrzej Siewior
2022-01-26 15:04       ` Michal Koutný
2022-01-27 13:36         ` Sebastian Andrzej Siewior
2022-01-26 15:21       ` Michal Hocko
2022-01-25 16:43 ` [PATCH 2/4] mm/memcg: Protect per-CPU counter by disabling preemption on PREEMPT_RT where needed Sebastian Andrzej Siewior
2022-01-26 10:06   ` Vlastimil Babka
2022-01-26 11:24     ` Sebastian Andrzej Siewior
2022-01-26 14:56   ` Michal Hocko
2022-01-25 16:43 ` [PATCH 3/4] mm/memcg: Add a local_lock_t for IRQ and TASK object Sebastian Andrzej Siewior
2022-01-26 15:20   ` Michal Hocko
2022-01-27 11:53     ` Sebastian Andrzej Siewior [this message]
2022-02-01 12:04       ` Michal Hocko
2022-02-01 12:11         ` Sebastian Andrzej Siewior
2022-02-01 15:29           ` Michal Hocko
2022-02-03  9:54             ` Sebastian Andrzej Siewior
2022-02-03 10:09               ` Michal Hocko
2022-02-03 11:09                 ` Sebastian Andrzej Siewior
2022-02-08 17:58                 ` Shakeel Butt
2022-02-09  9:17                   ` Michal Hocko
2022-01-26 16:57   ` Vlastimil Babka
2022-01-31 15:06     ` Sebastian Andrzej Siewior
2022-02-03 16:01       ` Vlastimil Babka
2022-02-08 17:17         ` Sebastian Andrzej Siewior
2022-02-08 17:28           ` Michal Hocko
2022-02-09  1:48   ` [mm/memcg] 86895e1e85: WARNING:possible_circular_locking_dependency_detected kernel test robot
2022-01-25 16:43 ` [PATCH 4/4] mm/memcg: Allow the task_obj optimization only on non-PREEMPTIBLE kernels Sebastian Andrzej Siewior
2022-01-25 23:21 ` [PATCH 0/4] mm/memcg: Address PREEMPT_RT problems instead of disabling it Andrew Morton
2022-01-26  7:30   ` 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=YfKHxKda7bGJmrLJ@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=linux-mm@kvack.org \
    --cc=longman@redhat.com \
    --cc=mhocko@suse.com \
    --cc=mkoutny@suse.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=vdavydov.dev@gmail.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).