linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Waiman Long <waiman.long@hp.com>
To: Jason Low <jason.low2@hp.com>
Cc: Davidlohr Bueso <davidlohr@hp.com>,
	Peter Zijlstra <peterz@infradead.org>,
	torvalds@linux-foundation.org, paulmck@linux.vnet.ibm.com,
	mingo@kernel.org, linux-kernel@vger.kernel.org, riel@redhat.com,
	akpm@linux-foundation.org, hpa@zytor.com, andi@firstfloor.org,
	James.Bottomley@hansenpartnership.com, rostedt@goodmis.org,
	tim.c.chen@linux.intel.com, aswin@hp.com, scott.norton@hp.com,
	chegu_vinod@hp.com
Subject: Re: [RFC] Cancellable MCS spinlock rework
Date: Thu, 03 Jul 2014 17:35:01 -0400	[thread overview]
Message-ID: <53B5CC85.1040603@hp.com> (raw)
In-Reply-To: <1404420708.8764.54.camel@j-VirtualBox>

On 07/03/2014 04:51 PM, Jason Low wrote:
> On Thu, 2014-07-03 at 16:35 -0400, Waiman Long wrote:
>> On 07/03/2014 02:34 PM, Jason Low wrote:
>>> On Thu, 2014-07-03 at 10:09 -0700, Davidlohr Bueso wrote:
>>>> On Thu, 2014-07-03 at 09:31 +0200, Peter Zijlstra wrote:
>>>>> On Wed, Jul 02, 2014 at 10:30:03AM -0700, Jason Low wrote:
>>>>>> Would potentially reducing the size of the rw semaphore structure by 32
>>>>>> bits (for all architectures using optimistic spinning) be a nice
>>>>>> benefit?
>>>>> Possibly, although I had a look at the mutex structure and we didn't
>>>>> have a hole to place it in, unlike what you found with the rwsem.
>>>> Yeah, and currently struct rw_semaphore is the largest lock we have in
>>>> the kernel. Shaving off space is definitely welcome.
>>> Right, especially if it could help things like xfs inode.
>>>
>> I do see a point in reducing the size of the rwsem structure. However, I
>> don't quite understand the point of converting pointers in the
>> optimistic_spin_queue structure to atomic_t.
> Converting the pointers in the optimistic_spin_queue to atomic_t would
> mean we're fully operating on atomic operations instead of using the
> potentially racy cmpxchg + ACCESS_ONCE stores on the pointers.

Yes, the ACCESS_ONCE macro for data store does have problem on some 
architectures. However, I prefer a more holistic solution to solve this 
problem rather than a workaround by changing the pointers to atomic_t's. 
It is because even if we make the change,  we are still not sure if that 
will work for those architectures as we have no machine to verify that. 
Why not let the champions of those architectures to propose changes 
instead of making some untested changes now and penalize commonly used 
architectures like x86.

> If we're in the process of using the CPU numbers in atomic_t, I thought
> we might as well fix that as well since it has actually been shown to
> result in lockups on some architectures. We can then avoid needing to
> implement the tricky architecture workarounds for optimistic spinning.
> Wouldn't that be a "nice-have"?
>
> Jason
>

I am not aware of any tricky architectural workarounds other than 
disabling optimistic spinning for those that don't support it.

-Longman


  reply	other threads:[~2014-07-03 21:35 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-02 16:21 [RFC] Cancellable MCS spinlock rework Jason Low
2014-07-02 16:27 ` Peter Zijlstra
2014-07-02 16:59   ` Jason Low
2014-07-02 17:23     ` Peter Zijlstra
2014-07-02 17:30       ` Jason Low
2014-07-03  4:39         ` Jason Low
2014-07-03  7:31           ` Peter Zijlstra
2014-07-03 15:31           ` Linus Torvalds
2014-07-03 15:35             ` Linus Torvalds
2014-07-03 18:22               ` Jason Low
2014-07-03  7:31         ` Peter Zijlstra
2014-07-03 17:09           ` Davidlohr Bueso
2014-07-03 18:34             ` Jason Low
2014-07-03 20:35               ` Waiman Long
2014-07-03 20:51                 ` Jason Low
2014-07-03 21:35                   ` Waiman Long [this message]
2014-07-03 21:54                     ` Jason Low
2014-07-04  7:49                   ` Peter Zijlstra
2014-07-04  1:07                 ` Jason Low
2014-07-04  7:51                   ` Peter Zijlstra
2014-07-07 17:22                     ` Jason Low
2014-07-04  9:17                   ` Peter Zijlstra

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=53B5CC85.1040603@hp.com \
    --to=waiman.long@hp.com \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=aswin@hp.com \
    --cc=chegu_vinod@hp.com \
    --cc=davidlohr@hp.com \
    --cc=hpa@zytor.com \
    --cc=jason.low2@hp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=scott.norton@hp.com \
    --cc=tim.c.chen@linux.intel.com \
    --cc=torvalds@linux-foundation.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 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).