linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Hillf Danton <hdanton@sina.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Alexey Gladkov <legion@kernel.org>
Subject: Re: [GIT PULL] ucount fix for v5.14-rc
Date: Sat, 7 Aug 2021 01:23:07 -0700	[thread overview]
Message-ID: <CAHk-=whyqY=1cAOXTE1o=w2jm8CKcM47=iOR2o2aNundzUpa_g@mail.gmail.com> (raw)
In-Reply-To: <20210807050314.1807-1-hdanton@sina.com>

On Fri, Aug 6, 2021 at 10:03 PM Hillf Danton <hdanton@sina.com> wrote:
>
> Then the current atomic_add_negative() in consideration over the "risk"
> of count overflow in real workloads can be replaced with the not_zero
> version.

What? No.

The atomic_add_negative() has absolutely nothing to do with not_zero.

The "negative" comes not at all from the count ever being zero, and as
I explained, that isn't even an issue here.

The "negative" is from a large _positive_ count growing so much that
the sign bit gets set. It's basically a "31-bit overflow" thing.

So:

 - not_zero makes no sense for get_ucounts(), because it can't be
zero, because we hold a reference to it

 - atomic_add_negative() is about not letting the counts become too
large, and when they do, we undo the reference (ie the pattern is
"increment ref - but if it then overflows into bit #31, decrement it
again"

and the two have *NOTHING* to do with each other. So your statement
about replacing one with the other makes no sense.

I was trying to explain that in _other_ situations, the
"atomic_inc_not_zero()" kind of pattern is used as a way to allow the
find-vs-last-drop race to be done without locking, but that's not what
the ucounts code does.

ucounts uses the ucounts_lock, and that one is entirely immaterial for
the atomic_add_negative() case, because the "negative" test is
literally about the value being as far away from zero as is _possible_
(and at that point, the lock is most definitely not needed - it's
needed only for the cases where the refcount goes to zero, and to make
sure that a "find" cannot race with that).

                Linus

  parent reply	other threads:[~2021-08-07  8:23 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-05 17:15 Eric W. Biederman
2021-08-05 19:26 ` pr-tracker-bot
     [not found] ` <20210806021052.3013-1-hdanton@sina.com>
2021-08-06  3:38   ` Eric W. Biederman
     [not found]     ` <20210806061458.3075-1-hdanton@sina.com>
2021-08-06 17:37       ` Linus Torvalds
     [not found]         ` <20210807050314.1807-1-hdanton@sina.com>
2021-08-07  8:23           ` Linus Torvalds [this message]
     [not found]             ` <20210807091128.1862-1-hdanton@sina.com>
2021-08-07 15:10               ` Linus Torvalds
     [not found]                 ` <20210808004243.2007-1-hdanton@sina.com>
2021-08-08  1:00                   ` Linus Torvalds
     [not found]                     ` <20210808012056.2067-1-hdanton@sina.com>
2021-08-08  1:45                       ` Linus Torvalds
2021-08-08  2:05                         ` Linus Torvalds

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='CAHk-=whyqY=1cAOXTE1o=w2jm8CKcM47=iOR2o2aNundzUpa_g@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=ebiederm@xmission.com \
    --cc=hdanton@sina.com \
    --cc=legion@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [GIT PULL] ucount fix for v5.14-rc' \
    /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

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