From: Waiman Long <waiman.long@hp.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
Jeff Layton <jlayton@redhat.com>,
Miklos Szeredi <mszeredi@suse.cz>, Ingo Molnar <mingo@redhat.com>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Andi Kleen <andi@firstfloor.org>,
"Chandramouleeswaran, Aswin" <aswin@hp.com>,
"Norton, Scott J" <scott.norton@hp.com>,
Thomas Gleixner <tglx@linutronix.de>,
Peter Zijlstra <peterz@infradead.org>,
Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [PATCH v2 1/2] spinlock: New spinlock_refcount.h for lockless update of refcount
Date: Sat, 29 Jun 2013 16:23:10 -0400 [thread overview]
Message-ID: <51CF422E.7030803@hp.com> (raw)
In-Reply-To: <CA+55aFzo5Jy064pbvEZh5cg_RJiSxe1R+tSgByQTs4rc=4SN3Q@mail.gmail.com>
On 06/29/2013 01:45 PM, Linus Torvalds wrote:
> Sorry for not commenting earlier, I was traveling and keeping email to
> a minimum..
>
> On Wed, Jun 26, 2013 at 10:43 AM, Waiman Long<Waiman.Long@hp.com> wrote:
>> This patch introduces a new spinlock_refcount.h header file to be
>> included by kernel code that want to do a lockless update of reference
>> count protected by a spinlock.
> So I really like the concept, but the implementation is a mess, and
> tries to do too much, while actually achieving too little.
>
> I do not believe you should care about debug spinlocks at all, and
> just leave them be. Have a simple fallback code that defaults to
> regular counts and spinlocks, and have any debug cases just use that.
I was concern that people might want to have the same behavior even when
spinlock debugging was on. Apparently, this is not really needed. Now I
can just disable the optimization and fall back to the old path when
spinlock debugging is on.
> But more importantly, I think this needs to be architecture-specific,
> and using<linux/spinlock_refcount.h> to try to do some generic 64-bit
> cmpxchg() version is a bad bad idea.
Yes, I can put the current implementation into
asm-generic/spinlock_refcount.h. Now I need to put an
asm/spinlock_refcount.h into every arch's include/asm directory. Right?
I don't think there is a mechanism in the build script to create a
symlink from asm to generic-asm when a header file is missing. Is it the
general rule that we should have a linux/spinlock_refcount.h that
include asm/spinlock_refcount.h instead of including
asm/spinlock_refcount.h directly?
> We have several architectures coming up that have memory transaction
> support, and the "spinlock with refcount" is a perfect candidate for a
> transactional memory implementation. So when introducing a new atomic
> like this that is very performance-critical and used for some very
> core code, I really think architectures would want to make their own
> optimized versions.
>
> These things should also not be inlined, I think.
>
> So I think the concept is good, but I think the implementation needs
> more thought.
>
> Linus
Thank for the comment. I will try to come up with a version that is
acceptable to all stakeholders.
Regards,
Longman
next prev parent reply other threads:[~2013-06-29 20:23 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-26 17:43 [PATCH v2 0/2] Lockless update of reference count protected by spinlock Waiman Long
2013-06-26 17:43 ` [PATCH v2 1/2] spinlock: New spinlock_refcount.h for lockless update of refcount Waiman Long
2013-06-26 20:17 ` Andi Kleen
2013-06-26 21:07 ` Waiman Long
2013-06-26 21:22 ` Andi Kleen
2013-06-26 23:26 ` Waiman Long
2013-06-27 1:06 ` Andi Kleen
2013-06-27 1:15 ` Waiman Long
2013-06-27 1:24 ` Waiman Long
2013-06-27 1:37 ` Andi Kleen
2013-06-27 14:56 ` Waiman Long
2013-06-28 13:46 ` Thomas Gleixner
2013-06-29 20:30 ` Waiman Long
2013-06-26 23:27 ` Thomas Gleixner
2013-06-26 23:06 ` Thomas Gleixner
2013-06-27 0:16 ` Waiman Long
2013-06-27 14:44 ` Thomas Gleixner
2013-06-29 21:03 ` Waiman Long
2013-06-27 0:26 ` Waiman Long
2013-06-29 17:45 ` Linus Torvalds
2013-06-29 20:23 ` Waiman Long [this message]
2013-06-29 21:34 ` Waiman Long
2013-06-29 22:11 ` Linus Torvalds
2013-06-29 22:34 ` Waiman Long
2013-06-29 21:58 ` Linus Torvalds
2013-06-29 22:47 ` Linus Torvalds
2013-07-01 13:40 ` Waiman Long
2013-06-26 17:43 ` [PATCH v2 2/2] dcache: Locklessly update d_count whenever possible Waiman Long
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=51CF422E.7030803@hp.com \
--to=waiman.long@hp.com \
--cc=andi@firstfloor.org \
--cc=aswin@hp.com \
--cc=benh@kernel.crashing.org \
--cc=jlayton@redhat.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=mszeredi@suse.cz \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=scott.norton@hp.com \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
/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.