From: Alex Kogan <alex.kogan@oracle.com> To: Peter Zijlstra <peterz@infradead.org> Cc: linux@armlinux.org.uk, mingo@redhat.com, will.deacon@arm.com, arnd@arndb.de, longman@redhat.com, linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, tglx@linutronix.de, bp@alien8.de, hpa@zytor.com, x86@kernel.org, steven.sistare@oracle.com, daniel.m.jordan@oracle.com, dave.dice@oracle.com, rahul.x.yadav@oracle.com Subject: Re: [PATCH v2 3/5] locking/qspinlock: Introduce CNA into the slow path of qspinlock Date: Wed, 3 Apr 2019 11:53:53 -0400 [thread overview] Message-ID: <7A8A6827-BF79-47FC-99A1-C9EE00D9C3B1@oracle.com> (raw) In-Reply-To: <20190401093345.GA14281@hirez.programming.kicks-ass.net> > On Apr 1, 2019, at 5:33 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > On Mon, Apr 01, 2019 at 11:06:53AM +0200, Peter Zijlstra wrote: >> On Fri, Mar 29, 2019 at 11:20:04AM -0400, Alex Kogan wrote: >>> diff --git a/kernel/locking/mcs_spinlock.h b/kernel/locking/mcs_spinlock.h >>> index bc6d3244e1af..71ee4b64c5d4 100644 >>> --- a/kernel/locking/mcs_spinlock.h >>> +++ b/kernel/locking/mcs_spinlock.h >>> @@ -17,8 +17,18 @@ >>> >>> struct mcs_spinlock { >>> struct mcs_spinlock *next; >>> +#ifndef CONFIG_NUMA_AWARE_SPINLOCKS >>> int locked; /* 1 if lock acquired */ >>> int count; /* nesting count, see qspinlock.c */ >>> +#else /* CONFIG_NUMA_AWARE_SPINLOCKS */ >>> + uintptr_t locked; /* 1 if lock acquired, 0 if not, other values */ >>> + /* represent a pointer to the secondary queue head */ >>> + u32 node_and_count; /* node id on which this thread is running */ >>> + /* with two lower bits reserved for nesting */ >>> + /* count, see qspinlock.c */ >>> + u32 encoded_tail; /* encoding of this node as the main queue tail */ >>> + struct mcs_spinlock *tail; /* points to the secondary queue tail */ >>> +#endif /* CONFIG_NUMA_AWARE_SPINLOCKS */ >>> }; >> >> Please, have another look at the paravirt code, in particular at struct >> pv_node and its usage. This is horrible. > > Thing is, this turns into a right mess when you also have PV spinlocks > on. > > One thing we could maybe do is change locked and count to u8, then your > overlay structure could be something like: > > struct mcs_spinlock { > struct mcs_spinlock *next; > u8 locked; > u8 count; > }; I was trying to keep the size of the mcs_spinlock structure for the non-NUMA variant unchanged. If this is not a huge concern, changing the fields as above would indeed simplify a few things. — Alex > > struct cna_node { > /* struct mcs_spinlock overlay */ > struct mcs_spinlock *next; > u8 locked; > u8 count; > > /* our CNA bits, consuming the slack and PV space */ > u16 node; > u32 encoded_tail; > struct mcs_spinlock *head; > struct mcs_spinlock *tail; > }; > > Then you also don't need the first two patches.
WARNING: multiple messages have this Message-ID (diff)
From: Alex Kogan <alex.kogan@oracle.com> To: Peter Zijlstra <peterz@infradead.org> Cc: linux-arch@vger.kernel.org, arnd@arndb.de, dave.dice@oracle.com, x86@kernel.org, will.deacon@arm.com, linux@armlinux.org.uk, steven.sistare@oracle.com, linux-kernel@vger.kernel.org, rahul.x.yadav@oracle.com, mingo@redhat.com, bp@alien8.de, hpa@zytor.com, longman@redhat.com, tglx@linutronix.de, daniel.m.jordan@oracle.com, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2 3/5] locking/qspinlock: Introduce CNA into the slow path of qspinlock Date: Wed, 3 Apr 2019 11:53:53 -0400 [thread overview] Message-ID: <7A8A6827-BF79-47FC-99A1-C9EE00D9C3B1@oracle.com> (raw) In-Reply-To: <20190401093345.GA14281@hirez.programming.kicks-ass.net> > On Apr 1, 2019, at 5:33 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > On Mon, Apr 01, 2019 at 11:06:53AM +0200, Peter Zijlstra wrote: >> On Fri, Mar 29, 2019 at 11:20:04AM -0400, Alex Kogan wrote: >>> diff --git a/kernel/locking/mcs_spinlock.h b/kernel/locking/mcs_spinlock.h >>> index bc6d3244e1af..71ee4b64c5d4 100644 >>> --- a/kernel/locking/mcs_spinlock.h >>> +++ b/kernel/locking/mcs_spinlock.h >>> @@ -17,8 +17,18 @@ >>> >>> struct mcs_spinlock { >>> struct mcs_spinlock *next; >>> +#ifndef CONFIG_NUMA_AWARE_SPINLOCKS >>> int locked; /* 1 if lock acquired */ >>> int count; /* nesting count, see qspinlock.c */ >>> +#else /* CONFIG_NUMA_AWARE_SPINLOCKS */ >>> + uintptr_t locked; /* 1 if lock acquired, 0 if not, other values */ >>> + /* represent a pointer to the secondary queue head */ >>> + u32 node_and_count; /* node id on which this thread is running */ >>> + /* with two lower bits reserved for nesting */ >>> + /* count, see qspinlock.c */ >>> + u32 encoded_tail; /* encoding of this node as the main queue tail */ >>> + struct mcs_spinlock *tail; /* points to the secondary queue tail */ >>> +#endif /* CONFIG_NUMA_AWARE_SPINLOCKS */ >>> }; >> >> Please, have another look at the paravirt code, in particular at struct >> pv_node and its usage. This is horrible. > > Thing is, this turns into a right mess when you also have PV spinlocks > on. > > One thing we could maybe do is change locked and count to u8, then your > overlay structure could be something like: > > struct mcs_spinlock { > struct mcs_spinlock *next; > u8 locked; > u8 count; > }; I was trying to keep the size of the mcs_spinlock structure for the non-NUMA variant unchanged. If this is not a huge concern, changing the fields as above would indeed simplify a few things. — Alex > > struct cna_node { > /* struct mcs_spinlock overlay */ > struct mcs_spinlock *next; > u8 locked; > u8 count; > > /* our CNA bits, consuming the slack and PV space */ > u16 node; > u32 encoded_tail; > struct mcs_spinlock *head; > struct mcs_spinlock *tail; > }; > > Then you also don't need the first two patches. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-04-03 15:54 UTC|newest] Thread overview: 86+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-03-29 15:20 [PATCH v2 0/5] Add NUMA-awareness to qspinlock Alex Kogan 2019-03-29 15:20 ` Alex Kogan 2019-03-29 15:20 ` [PATCH v2 1/5] locking/qspinlock: Make arch_mcs_spin_unlock_contended more generic Alex Kogan 2019-03-29 15:20 ` Alex Kogan 2019-03-29 15:20 ` [PATCH v2 2/5] locking/qspinlock: Refactor the qspinlock slow path Alex Kogan 2019-03-29 15:20 ` Alex Kogan 2019-03-29 15:20 ` [PATCH v2 3/5] locking/qspinlock: Introduce CNA into the slow path of qspinlock Alex Kogan 2019-03-29 15:20 ` Alex Kogan 2019-04-01 9:06 ` Peter Zijlstra 2019-04-01 9:06 ` Peter Zijlstra 2019-04-01 9:06 ` Peter Zijlstra 2019-04-01 9:33 ` Peter Zijlstra 2019-04-01 9:33 ` Peter Zijlstra 2019-04-01 9:33 ` Peter Zijlstra 2019-04-03 15:53 ` Alex Kogan [this message] 2019-04-03 15:53 ` Alex Kogan 2019-04-03 16:10 ` Peter Zijlstra 2019-04-03 16:10 ` Peter Zijlstra 2019-04-03 16:10 ` Peter Zijlstra 2019-04-01 9:21 ` Peter Zijlstra 2019-04-01 9:21 ` Peter Zijlstra 2019-04-01 14:36 ` Waiman Long 2019-04-01 14:36 ` Waiman Long 2019-04-02 9:43 ` Peter Zijlstra 2019-04-02 9:43 ` Peter Zijlstra 2019-04-02 9:43 ` Peter Zijlstra 2019-04-03 15:39 ` Alex Kogan 2019-04-03 15:39 ` Alex Kogan 2019-04-03 15:48 ` Waiman Long 2019-04-03 15:48 ` Waiman Long 2019-04-03 16:01 ` Peter Zijlstra 2019-04-03 16:01 ` Peter Zijlstra 2019-04-04 5:05 ` Juergen Gross 2019-04-04 5:05 ` Juergen Gross 2019-04-04 9:38 ` Peter Zijlstra 2019-04-04 9:38 ` Peter Zijlstra 2019-04-04 9:38 ` Peter Zijlstra 2019-04-04 18:03 ` Waiman Long 2019-04-04 18:03 ` Waiman Long 2019-06-04 23:21 ` Alex Kogan 2019-06-04 23:21 ` Alex Kogan 2019-06-05 20:40 ` Peter Zijlstra 2019-06-05 20:40 ` Peter Zijlstra 2019-06-06 15:21 ` Alex Kogan 2019-06-06 15:21 ` Alex Kogan 2019-06-06 15:32 ` Waiman Long 2019-06-06 15:32 ` Waiman Long 2019-06-06 15:42 ` Waiman Long 2019-06-06 15:42 ` Waiman Long 2019-04-03 16:33 ` Waiman Long 2019-04-03 16:33 ` Waiman Long 2019-04-03 17:16 ` Peter Zijlstra 2019-04-03 17:16 ` Peter Zijlstra 2019-04-03 17:16 ` Peter Zijlstra 2019-04-03 17:40 ` Waiman Long 2019-04-03 17:40 ` Waiman Long 2019-04-04 2:02 ` Hanjun Guo 2019-04-04 2:02 ` Hanjun Guo 2019-04-04 2:02 ` Hanjun Guo 2019-04-04 3:14 ` Alex Kogan 2019-04-04 3:14 ` Alex Kogan 2019-06-11 4:22 ` liwei (GF) 2019-06-11 4:22 ` liwei (GF) 2019-06-11 4:22 ` liwei (GF) 2019-06-12 4:38 ` Alex Kogan 2019-06-12 4:38 ` Alex Kogan 2019-06-12 15:05 ` Waiman Long 2019-06-12 15:05 ` Waiman Long 2019-03-29 15:20 ` [PATCH v2 4/5] locking/qspinlock: Introduce starvation avoidance into CNA Alex Kogan 2019-03-29 15:20 ` Alex Kogan 2019-04-02 10:37 ` Peter Zijlstra 2019-04-02 10:37 ` Peter Zijlstra 2019-04-02 10:37 ` Peter Zijlstra 2019-04-03 17:06 ` Alex Kogan 2019-04-03 17:06 ` Alex Kogan 2019-03-29 15:20 ` [PATCH v2 5/5] locking/qspinlock: Introduce the shuffle reduction optimization " Alex Kogan 2019-03-29 15:20 ` Alex Kogan 2019-04-01 9:09 ` [PATCH v2 0/5] Add NUMA-awareness to qspinlock Peter Zijlstra 2019-04-01 9:09 ` Peter Zijlstra 2019-04-01 9:09 ` Peter Zijlstra 2019-04-03 17:13 ` Alex Kogan 2019-04-03 17:13 ` Alex Kogan 2019-07-03 11:57 ` Jan Glauber 2019-07-03 11:58 ` Jan Glauber 2019-07-12 8:12 ` Hanjun Guo 2019-07-12 8:12 ` Hanjun Guo
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=7A8A6827-BF79-47FC-99A1-C9EE00D9C3B1@oracle.com \ --to=alex.kogan@oracle.com \ --cc=arnd@arndb.de \ --cc=bp@alien8.de \ --cc=daniel.m.jordan@oracle.com \ --cc=dave.dice@oracle.com \ --cc=hpa@zytor.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=tglx@linutronix.de \ --cc=will.deacon@arm.com \ --cc=x86@kernel.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: linkBe 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.