archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <>
To: Alexey Gladkov <>
Cc: "Eric W. Biederman" <>,
	kernel test robot <>,
	 0day robot <>, LKML <>,,  "Huang, Ying" <>,
	Feng Tang <>,,  io-uring <>,
	 Kernel Hardening <>,
	 Linux Containers <>,
	Linux-MM <>,
	 Andrew Morton <>,
	 Christian Brauner <>,
	Jann Horn <>,  Jens Axboe <>,
	Kees Cook <>,
	Oleg Nesterov <>
Subject: Re: d28296d248: stress-ng.sigsegv.ops_per_sec -82.7% regression
Date: Wed, 24 Feb 2021 11:10:43 -0800	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Wed, Feb 24, 2021 at 10:38 AM Alexey Gladkov
<> wrote:
> One of the reasons for this is that I rolled back the patch that changed
> the ucounts.count type to atomic_t. Now get_ucounts() is forced to use a
> spin_lock to increase the reference count.

Yeah, that definitely should be an atomic type, since the extended use
of ucounts clearly puts way too much pressure on that ucount lock.

I remember complaining about one version of that patch, but my
complaint wasabout it changing semantics of the saturation logic (and
I think it was also wrong because it still kept the spinlock for
get_ucounts(), so it didn't even take advantage of the atomic

Side note: I don't think a refcount_t" is necessarily the right thing
to do, since the ucount reference counter does its own saturation
logic, and the refcount_t version is imho not great.

So it probably just needs to use an atomic_t, and do the saturation
thing manually.

Side note: look at try_get_page(). That one actually does refcounting
with overflow protection better than refcount_t, in my opinion. But I
am obviously biased, since I wrote it ;)

See commits

    88b1a17dfc3e mm: add 'try_get_page()' helper function
    f958d7b528b1 mm: make page ref count overflow check tighter and
more explicit

with that "page->_recount" being just a regular atomic_t.


  parent reply	other threads:[~2021-02-24 19:11 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-22  9:56 [PATCH v7 0/7] Count rlimits in each user namespace Alexey Gladkov
2021-02-22  9:56 ` [PATCH v7 1/7] Increase size of ucounts to atomic_long_t Alexey Gladkov
2021-02-22  9:56 ` [PATCH v7 2/7] Add a reference to ucounts for each cred Alexey Gladkov
2021-03-01  7:32   ` e1e57d56fe: stress-ng.access.ops_per_sec -41.6% regression kernel test robot
2021-02-22  9:56 ` [PATCH v7 3/7] Reimplement RLIMIT_NPROC on top of ucounts Alexey Gladkov
2021-02-22  9:56 ` [PATCH v7 4/7] Reimplement RLIMIT_MSGQUEUE " Alexey Gladkov
2021-02-22  9:56 ` [PATCH v7 5/7] Reimplement RLIMIT_SIGPENDING " Alexey Gladkov
2021-02-24  5:18   ` d28296d248: stress-ng.sigsegv.ops_per_sec -82.7% regression kernel test robot
2021-02-24 16:54     ` Eric W. Biederman
2021-02-24 18:38       ` Alexey Gladkov
2021-02-24 18:50         ` Eric W. Biederman
2021-02-25 20:36           ` Alexey Gladkov
2021-03-05 17:56             ` Eric W. Biederman
2021-02-24 19:10         ` Linus Torvalds [this message]
2021-02-22  9:56 ` [PATCH v7 6/7] Reimplement RLIMIT_MEMLOCK on top of ucounts Alexey Gladkov
2021-03-01  6:51   ` 5b5c35b757: BUG:KASAN:use-after-free_in_dec_rlimit_ucounts kernel test robot
2021-02-22  9:56 ` [PATCH v7 7/7] kselftests: Add test to check for rlimit changes in different user namespaces Alexey Gladkov

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='' \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* 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).