All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <jbeulich@suse.com>, Juergen Gross <jgross@suse.com>
Cc: javi.merino@cloud.com, "George Dunlap" <george.dunlap@citrix.com>,
	"Julien Grall" <julien@xen.org>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Wei Liu" <wl@xen.org>, "Roger Pau Monné" <roger.pau@citrix.com>,
	"Paul Durrant" <paul@xen.org>,
	"Bertrand Marquis" <bertrand.marquis@arm.com>,
	"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
	"Tamas K Lengyel" <tamas@tklengyel.com>,
	"Lukasz Hawrylko" <lukasz@hawrylko.pl>,
	"Daniel P. Smith" <dpsmith@apertussolutions.com>,
	"Mateusz Mówka" <mateusz.mowka@intel.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type
Date: Thu, 19 Oct 2023 10:35:07 +0100	[thread overview]
Message-ID: <5f0cecef-8600-40b7-8e59-77d8e33750a7@citrix.com> (raw)
In-Reply-To: <c86cb866-b795-b6af-0ad7-38e68c7d35c4@suse.com>

On 19/10/2023 8:48 am, Jan Beulich wrote:
> On 13.10.2023 11:42, Juergen Gross wrote:
>> Instead of being able to use normal spinlocks as recursive ones, too,
>> make recursive spinlocks a special lock type.
>>
>> This will make the spinlock structure smaller in production builds and
>> add type-safety.
>>
>> This allows to increase the maximum number of physical cpus from 8191
>> to 65535 without increasing the size of the lock structure in production
>> builds (the size of recursive spinlocks in debug builds will grow to
>> 12 bytes due to that change).
>>
>> Changes in V2:
>> - addressed comments by Jan Beulich
>> - lots of additional cleanups
>> - reorganized complete series
>>
>> Juergen Gross (13):
>>   xen/spinlock: fix coding style issues
>>   xen/spinlock: reduce lock profile ifdefs
>>   xen/spinlock: make spinlock initializers more readable
>>   xen/spinlock: introduce new type for recursive spinlocks
>>   xen/spinlock: rename recursive lock functions
>>   xen/spinlock: add rspin_[un]lock_irq[save|restore]()
>>   xen/spinlock: make struct lock_profile rspinlock_t aware
>>   xen/spinlock: add explicit non-recursive locking functions
>>   xen/spinlock: add another function level
>>   xen/spinlock: add missing rspin_is_locked() and rspin_barrier()
>>   xen/spinlock: split recursive spinlocks from normal ones
>>   xen/spinlock: remove indirection through macros for spin_*() functions
>>   xen/spinlock: support higher number of cpus
> Before looking at patches 4 and onwards, I'd like us to settle on the future
> of recursive locking in Xen, considering in particular Andrew's objections
> to their use in the code base. If the plan was to eliminate them, I'd see
> little point in reworking the infrastructure. I'd like to suggest that one
> of us tries to remember to put this up as an agenda item for the next
> Community Call. But of course the discussion can also happen right here; I
> merely expect there might not be much of a reaction.

Actually, I consider this series an improvement.  The CPU limit is the
most urgent problem to fix.

XenServer has just jumped to NR_CPUS=2k in order to support 2024's range
of hardware, and it's only going to be a couple of years more before
we're stuck given the current spinlocks.

I do genuinely think the code and logic would be better without
recursive locks, but making that happen is going to be very invasive and
complicated.

But in the meantime with spinlocks properly separated from recursive
locks, it becomes easier IMO to dissuade the introduction of new cases
while we unpick the existing ones.

And so what if we do end up deleting recursive locks in a few years
time?  That's not an argument against doing this untangling now.

~Andrew


  reply	other threads:[~2023-10-19  9:35 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-13  9:42 [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Juergen Gross
2023-10-13  9:42 ` [PATCH v2 01/13] xen/spinlock: fix coding style issues Juergen Gross
2023-10-18 15:47   ` Jan Beulich
2023-10-18 15:52     ` Juergen Gross
2023-10-13  9:42 ` [PATCH v2 02/13] xen/spinlock: reduce lock profile ifdefs Juergen Gross
2023-10-18 15:57   ` Jan Beulich
2023-10-19  9:05     ` Juergen Gross
2023-10-13  9:42 ` [PATCH v2 03/13] xen/spinlock: make spinlock initializers more readable Juergen Gross
2023-10-18 16:00   ` Jan Beulich
2023-10-13  9:42 ` [PATCH v2 04/13] xen/spinlock: introduce new type for recursive spinlocks Juergen Gross
2023-10-13  9:42 ` [PATCH v2 05/13] xen/spinlock: rename recursive lock functions Juergen Gross
2023-10-13  9:42 ` [PATCH v2 06/13] xen/spinlock: add rspin_[un]lock_irq[save|restore]() Juergen Gross
2023-10-13  9:42 ` [PATCH v2 07/13] xen/spinlock: make struct lock_profile rspinlock_t aware Juergen Gross
2023-10-13  9:42 ` [PATCH v2 08/13] xen/spinlock: add explicit non-recursive locking functions Juergen Gross
2023-10-13  9:42 ` [PATCH v2 09/13] xen/spinlock: add another function level Juergen Gross
2023-10-13  9:42 ` [PATCH v2 10/13] xen/spinlock: add missing rspin_is_locked() and rspin_barrier() Juergen Gross
2023-10-13  9:42 ` [PATCH v2 11/13] xen/spinlock: split recursive spinlocks from normal ones Juergen Gross
2023-10-13  9:42 ` [PATCH v2 12/13] xen/spinlock: remove indirection through macros for spin_*() functions Juergen Gross
2023-10-13  9:42 ` [PATCH v2 13/13] xen/spinlock: support higher number of cpus Juergen Gross
2023-10-19  7:48 ` [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Jan Beulich
2023-10-19  9:35   ` Andrew Cooper [this message]
2023-10-19  9:39     ` Jan Beulich
2023-10-19 11:05       ` Andrew Cooper

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=5f0cecef-8600-40b7-8e59-77d8e33750a7@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=Volodymyr_Babchuk@epam.com \
    --cc=bertrand.marquis@arm.com \
    --cc=dpsmith@apertussolutions.com \
    --cc=george.dunlap@citrix.com \
    --cc=javi.merino@cloud.com \
    --cc=jbeulich@suse.com \
    --cc=jgross@suse.com \
    --cc=julien@xen.org \
    --cc=lukasz@hawrylko.pl \
    --cc=mateusz.mowka@intel.com \
    --cc=paul@xen.org \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=tamas@tklengyel.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.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.