All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Kogan <alex.kogan@oracle.com>
To: Waiman Long <longman@redhat.com>
Cc: linux@armlinux.org.uk, peterz@infradead.org, mingo@redhat.com,
	will.deacon@arm.com, arnd@arndb.de, linux-arch@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Steven Sistare <steven.sistare@oracle.com>,
	Daniel Jordan <daniel.m.jordan@oracle.com>,
	dave.dice@oracle.com, Rahul Yadav <rahul.x.yadav@oracle.com>
Subject: Re: [PATCH 2/3] locking/qspinlock: Introduce CNA into the slow path of qspinlock
Date: Fri, 1 Feb 2019 16:26:06 -0500	[thread overview]
Message-ID: <62BE66CF-1CFA-4921-B2FF-70D5460845D6@oracle.com> (raw)
In-Reply-To: <2c8aabab-1bb0-0708-94c4-305fd860609e@redhat.com>


> On Jan 31, 2019, at 12:38 PM, Waiman Long <longman@redhat.com> wrote:
> 
> On 01/30/2019 10:01 PM, Alex Kogan wrote:
>> In CNA, spinning threads are organized in two queues, a main queue for
>> threads running on the same socket as the current lock holder, and a
>> secondary queue for threads running on other sockets. For details,
>> see https://urldefense.proofpoint.com/v2/url?u=https-3A__arxiv.org_abs_1810.05600&d=DwIFaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=Hvhk3F4omdCk-GE1PTOm3Kn0A7ApWOZ2aZLTuVxFK4k&m=RdsRU4oU2j-_hNG5kDC5ZEE_XHikl3QLNttCaBde3QU&s=NQ840S1lz53Cq7AnOlPWdnjZI7_Ic3rfYsf-w2aYus4&e=.
>> 
>> Note that this variant of CNA may introduce starvation by continuously
>> passing the lock to threads running on the same socket. This issue
>> will be addressed later in the series.
>> 
>> Signed-off-by: Alex Kogan <alex.kogan@oracle.com>
>> Reviewed-by: Steve Sistare <steven.sistare@oracle.com>
> 
> Just wondering if you have tried include PARVIRT_SPINLOCKS option to see
> if that patch may screw up the PV qspinlock code.
No, I haven’t yet.
The idea was to make it work for non-PV systems first, and then extend to PV.

> 
> Anyway, I do believe your claim that NUMA-aware qspinlock is good for
> large systems with many nodes. However, all these extra code are
> overhead for small systems that have a single node/socket, for instance.
> 
> I will support doing something similar to what had been done to support
> PV qspinlock. IOW, a separate slowpath function that can be patched to
> become the default depending on the system being run on or a kernel boot
> option setting.
> 
> I would like to keep the core slowpath function simple and easy to
> understand. So most of the CNA code should be encapsulated into some
> helper functions and put into a separated file.
Sounds good. 
I think it should be pretty straightforward to encapsulate the CNA code and do what you suggest.
We will look into that.

Thanks,
— Alex

> 
> Thanks,
> Longman
> 


WARNING: multiple messages have this Message-ID (diff)
From: Alex Kogan <alex.kogan@oracle.com>
To: Waiman Long <longman@redhat.com>
Cc: linux-arch@vger.kernel.org, arnd@arndb.de, peterz@infradead.org,
	dave.dice@oracle.com, will.deacon@arm.com, linux@armlinux.org.uk,
	linux-kernel@vger.kernel.org,
	Rahul Yadav <rahul.x.yadav@oracle.com>,
	mingo@redhat.com, Steven Sistare <steven.sistare@oracle.com>,
	Daniel Jordan <daniel.m.jordan@oracle.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 2/3] locking/qspinlock: Introduce CNA into the slow path of qspinlock
Date: Fri, 1 Feb 2019 16:26:06 -0500	[thread overview]
Message-ID: <62BE66CF-1CFA-4921-B2FF-70D5460845D6@oracle.com> (raw)
In-Reply-To: <2c8aabab-1bb0-0708-94c4-305fd860609e@redhat.com>


> On Jan 31, 2019, at 12:38 PM, Waiman Long <longman@redhat.com> wrote:
> 
> On 01/30/2019 10:01 PM, Alex Kogan wrote:
>> In CNA, spinning threads are organized in two queues, a main queue for
>> threads running on the same socket as the current lock holder, and a
>> secondary queue for threads running on other sockets. For details,
>> see https://urldefense.proofpoint.com/v2/url?u=https-3A__arxiv.org_abs_1810.05600&d=DwIFaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=Hvhk3F4omdCk-GE1PTOm3Kn0A7ApWOZ2aZLTuVxFK4k&m=RdsRU4oU2j-_hNG5kDC5ZEE_XHikl3QLNttCaBde3QU&s=NQ840S1lz53Cq7AnOlPWdnjZI7_Ic3rfYsf-w2aYus4&e=.
>> 
>> Note that this variant of CNA may introduce starvation by continuously
>> passing the lock to threads running on the same socket. This issue
>> will be addressed later in the series.
>> 
>> Signed-off-by: Alex Kogan <alex.kogan@oracle.com>
>> Reviewed-by: Steve Sistare <steven.sistare@oracle.com>
> 
> Just wondering if you have tried include PARVIRT_SPINLOCKS option to see
> if that patch may screw up the PV qspinlock code.
No, I haven’t yet.
The idea was to make it work for non-PV systems first, and then extend to PV.

> 
> Anyway, I do believe your claim that NUMA-aware qspinlock is good for
> large systems with many nodes. However, all these extra code are
> overhead for small systems that have a single node/socket, for instance.
> 
> I will support doing something similar to what had been done to support
> PV qspinlock. IOW, a separate slowpath function that can be patched to
> become the default depending on the system being run on or a kernel boot
> option setting.
> 
> I would like to keep the core slowpath function simple and easy to
> understand. So most of the CNA code should be encapsulated into some
> helper functions and put into a separated file.
Sounds good. 
I think it should be pretty straightforward to encapsulate the CNA code and do what you suggest.
We will look into that.

Thanks,
— Alex

> 
> Thanks,
> Longman
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2019-02-01 21:26 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-31  3:01 [PATCH 0/3] Add NUMA-awareness to qspinlock Alex Kogan
2019-01-31  3:01 ` Alex Kogan
2019-01-31  3:01 ` [PATCH 1/3] locking/qspinlock: Make arch_mcs_spin_unlock_contended more generic Alex Kogan
2019-01-31  3:01   ` Alex Kogan
2019-01-31  3:01 ` [PATCH 2/3] locking/qspinlock: Introduce CNA into the slow path of qspinlock Alex Kogan
2019-01-31  3:01   ` Alex Kogan
2019-01-31 17:38   ` Waiman Long
2019-01-31 17:38     ` Waiman Long
2019-02-01 21:26     ` Alex Kogan [this message]
2019-02-01 21:26       ` Alex Kogan
2019-01-31  3:01 ` [PATCH 3/3] locking/qspinlock: Introduce starvation avoidance into CNA Alex Kogan
2019-01-31  3:01   ` Alex Kogan
2019-01-31 10:00   ` Peter Zijlstra
2019-01-31 10:00     ` Peter Zijlstra
2019-02-05  3:35     ` Alex Kogan
2019-02-05  3:35       ` Alex Kogan
2019-02-05  9:22       ` Peter Zijlstra
2019-02-05  9:22         ` Peter Zijlstra
2019-02-05  9:22         ` Peter Zijlstra
2019-02-05 13:48         ` Waiman Long
2019-02-05 13:48           ` Waiman Long
2019-02-05 21:07         ` Alex Kogan
2019-02-05 21:07           ` Alex Kogan
2019-02-05 21:12           ` Waiman Long
2019-02-05 21:12             ` Waiman Long
2019-01-31  9:56 ` [PATCH 0/3] Add NUMA-awareness to qspinlock Peter Zijlstra
2019-01-31  9:56   ` Peter Zijlstra
2019-01-31  9:56   ` Peter Zijlstra
2019-02-01 21:20   ` Alex Kogan
2019-02-01 21:20     ` Alex Kogan

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=62BE66CF-1CFA-4921-B2FF-70D5460845D6@oracle.com \
    --to=alex.kogan@oracle.com \
    --cc=arnd@arndb.de \
    --cc=daniel.m.jordan@oracle.com \
    --cc=dave.dice@oracle.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=longman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rahul.x.yadav@oracle.com \
    --cc=steven.sistare@oracle.com \
    --cc=will.deacon@arm.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 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.