public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Uros Bizjak' <ubizjak@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Mateusz Guzik <mjguzik@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] lib/genalloc: use try_cmpxchg in {set,clear}_bits_ll
Date: Mon, 23 Jan 2023 15:42:08 +0000	[thread overview]
Message-ID: <efcf1ad325064d47ba027db9a98222ac@AcuMS.aculab.com> (raw)
In-Reply-To: <CAFULd4aDORSrq7zf_LcAZRP8HOHcrq2-rGMaroKyG2zQDHNpOA@mail.gmail.com>

From: Uros Bizjak
> Sent: 23 January 2023 15:05
> 
> On Thu, Jan 19, 2023 at 1:47 PM David Laight <David.Laight@aculab.com> wrote:
> >
> > > BTW: Recently, it was determined [1] that the usage of cpu_relax()
> > > inside the cmpxchg loop can be harmful for performance. We actually
> > > have the same situation here, so perhaps cpu_relax() should be removed
> > > in the same way it was removed from the lockref.
> >
> > I'm not sure you can ever want a cpu_relax() in a loop that
> > is implementing an atomic operation.
> > Even the ia64 (die...) issue was with a loop that was waiting
> > for another cpu to change the location (eg a spinlock).
> >
> > For an atomic operation an immediate retry is likely to succeed.
> > Any kind of deferral to an another cpu can only make it worse.
> >
> > Clearly if you have 100s of cpu looping doing atomic operation
> > on the same cache line it is likely that some get starved.
> > But to fix that you need to increase the time between successful
> > operations, not delay on failure.
> 
> I would like to point out that the wikipedia article on
> compare-and-swap claims [1] that:
> 
> Instead of immediately retrying after a CAS operation fails,
> researchers have found that total system performance can be improved
> in multiprocessor systems—where many threads constantly update some
> particular shared variable—if threads that see their CAS fail use
> exponential backoff—in other words, wait a little before retrying the
> CAS [2].

Probably, but the real solution is 'don't do that'.
In any case I suspect the cpu_relax() explicitly lets the
other hyperthreading cpu run - which isn't useful at all.

What you actually want if for the cache logic to avoid losing
'exclusive' access to the cache line for enough clocks after a
failed compare+exchange to allow the cpu to re-issue the memory
cycle with an updated value.
You can't do anything about one cpu being starved, but a short
delay there is almost certainly beneficial.
(Some hardware cache engineer will probably say otherwise.)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

  reply	other threads:[~2023-01-23 15:42 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-18 15:07 [PATCH] lib/genalloc: use try_cmpxchg in {set,clear}_bits_ll Uros Bizjak
2023-01-18 21:18 ` Andrew Morton
2023-01-18 21:47   ` Uros Bizjak
2023-01-18 21:55     ` Uros Bizjak
2023-01-18 22:01       ` Uros Bizjak
2023-01-19 12:47       ` David Laight
2023-01-23 15:04         ` Uros Bizjak
2023-01-23 15:42           ` David Laight [this message]
2023-01-23 15:59           ` Mateusz Guzik
2023-01-23 19:36             ` Linus Torvalds
2023-01-24  0:11               ` Mateusz Guzik
2023-01-24  1:51                 ` Linus Torvalds
2023-01-24  6:48                   ` Mateusz Guzik
2023-01-24  8:54                 ` David Laight
2023-01-24 16:58                   ` Linus Torvalds
2023-01-24 17:32                     ` David Laight
2023-01-27  3:54               ` Al Viro
2023-01-27 19:25                 ` 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=efcf1ad325064d47ba027db9a98222ac@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mjguzik@gmail.com \
    --cc=torvalds@linux-foundation.org \
    --cc=ubizjak@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