On Tue, 2009-01-06 at 14:41 -0500, Chris Mason wrote:
Hello everyone,
Thanks for all of the comments so far. I've pushed out a number of
fixes for btrfs mainline, covering most of the comments from this
thread.
* All LINUX_KERNEL_VERSION checks are gone.
* checkpatch.pl fixes
* Extra
Change mutex contention behaviour such that it will sometimes busy wait on
acquisition - moving its behaviour closer to that of spinlocks.
This concept got ported to mainline from the -rt tree, where it was originally
implemented for rtmutexes by Steven Rostedt, based on work by Gregory Haskins.
* Chris Mason chris.ma...@oracle.com wrote:
On Tue, 2009-01-06 at 01:47 +1100, Nick Piggin wrote:
[ adaptive locking in btrfs ]
adaptive locks have traditionally (read: Linus says) indicated the locking
is suboptimal from a performance perspective and should be reworked. This
is
On Wed, Jan 07, 2009 at 02:07:42PM +0100, Ingo Molnar wrote:
* Chris Mason chris.ma...@oracle.com wrote:
All of this is a long way of saying the btrfs locking scheme is far from
perfect. I'll look harder at the loop and ways to get rid of it.
ob'plug
adaptive spinning mutexes perhaps?
On Wed, Jan 07, 2009 at 03:34:47PM +0800, Lai Jiangshan wrote:
So I think current task should sleep earlier when it detects that
mutex owner start schedule().
How do you propose it detects this?
--
Matthew Wilcox Intel Open Source Technology Centre
Bill, look, we
On Wed, 2009-01-07 at 14:45 +0100, Kay Sievers wrote:
On Wed, Jan 7, 2009 at 10:35, David Woodhouse dw...@infradead.org wrote:
Lifted the first paragraphs of btrfs.txt straight from the wiki...
+debug-tree: print all of the FS metadata in text form. Example:
Can we please rename that
Hello everyone,
There will be a btrfs conference call today January 7. Topics will
include mainline merging and testing results.
Time: 1:30pm US Eastern (10:30am Pacific)
* Dial-in Number(s):
* Toll Free: +1-888-967-2253
* Toll +1-650-607-2253
* Meeting id: 665734
* Passcode: 428737 (which
2009/1/7 Peter Zijlstra pet...@infradead.org:
Change mutex contention behaviour such that it will sometimes busy wait on
acquisition - moving its behaviour closer to that of spinlocks.
This concept got ported to mainline from the -rt tree, where it was originally
implemented for rtmutexes by
* Matthew Wilcox matt...@wil.cx wrote:
On Wed, Jan 07, 2009 at 02:07:42PM +0100, Ingo Molnar wrote:
* Chris Mason chris.ma...@oracle.com wrote:
All of this is a long way of saying the btrfs locking scheme is far from
perfect. I'll look harder at the loop and ways to get rid of it.
On Wed, 2009-01-07 at 15:50 +0100, Frédéric Weisbecker wrote:
2009/1/7 Peter Zijlstra pet...@infradead.org:
Change mutex contention behaviour such that it will sometimes busy wait on
acquisition - moving its behaviour closer to that of spinlocks.
This concept got ported to mainline from
On Wed, 7 Jan 2009, Steven Rostedt wrote:
On Wed, 7 Jan 2009, Peter Zijlstra wrote:
--- a/kernel/mutex.c
+++ b/kernel/mutex.c
@@ -10,6 +10,10 @@
* Many thanks to Arjan van de Ven, Thomas Gleixner, Steven Rostedt and
* David Howells for suggestions and improvements.
*
+ * -
On Wed, 2009-01-07 at 10:22 -0500, Steven Rostedt wrote:
Peter, nice work!
Thanks!
+ }
+
+ if (!spin) {
+ schedstat_inc(this_rq(), mtx_sched);
+ __set_task_state(task, state);
I still do not know why you set state here instead of in the mutex code.
Yes, you
On Wed, 7 Jan 2009, Peter Zijlstra wrote:
Change mutex contention behaviour such that it will sometimes busy wait on
acquisition - moving its behaviour closer to that of spinlocks.
Ok, this one looks _almost_ ok.
The only problem is that I think you've lost the UP case.
In UP, you
On Wed, 2009-01-07 at 08:25 -0800, Linus Torvalds wrote:
On Wed, 7 Jan 2009, Peter Zijlstra wrote:
Change mutex contention behaviour such that it will sometimes busy wait on
acquisition - moving its behaviour closer to that of spinlocks.
Ok, this one looks _almost_ ok.
The only
On Wed, 2009-01-07 at 08:25 -0800, Linus Torvalds wrote:
On Wed, 7 Jan 2009, Peter Zijlstra wrote:
Change mutex contention behaviour such that it will sometimes busy wait on
acquisition - moving its behaviour closer to that of spinlocks.
Ok, this one looks _almost_ ok.
The only
On Wed, 7 Jan 2009, Chris Mason wrote:
So far I haven't found any btrfs benchmarks where this is slower than
mutexes without any spinning. But, it isn't quite as fast as the btrfs
spin.
Quite frankly, from our history with ext3 and other filesystems, using a
mutex in the filesystem is
On Wed, 2009-01-07 at 09:50 -0800, Linus Torvalds wrote:
On Wed, 7 Jan 2009, Chris Mason wrote:
So far I haven't found any btrfs benchmarks where this is slower than
mutexes without any spinning. But, it isn't quite as fast as the btrfs
spin.
Quite frankly, from our history with
On Wed, 2009-01-07 at 09:33 +, David Woodhouse wrote:
On Tue, 2009-01-06 at 14:41 -0500, Chris Mason wrote:
One more thing I'd suggest is removing the INSTALL file. The parts about
having to build libcrc32c aren't relevant when it's part of the kernel
tree and you have 'select LIBCRC32C',
Ok a few more issues. This never stops.
Here's the basic spinloop:
On Wed, 7 Jan 2009, Peter Zijlstra wrote:
+ for (;;) {
+ struct task_struct *l_owner;
+
+ /* Stop spinning when there's a pending signal. */
+ if (signal_pending_state(task-state,
On Fri, 2008-12-19 at 13:14 -0800, Yehuda Sadeh Weinraub wrote:
This adds a new ioctl that requests for the csums of file's extent.
The csums of contiguous extents can also be requested. The call will
not return anything for extents that don't have csum information for
their data, or that the
On Wed, 2009-01-07 at 23:10 +0800, Yan Zheng wrote:
Hello,
This patch makes btrfsck check more things, including
directory items, file extents, checksumming, inode link
counts etc.
This is great work, thanks! I've pushed it out to the unstable tree.
-chris
--
To unsubscribe from this
Add call to LSM security initialization and save
resulting security xattr for new inodes.
Add xattr support to symlink inode ops.
Set inode-i_op for existing special files.
Signed-off-by: jim owens jow...@hp.com
---
fs/btrfs/inode.c | 23 +++
fs/btrfs/xattr.c | 32
On Wed, 7 Jan 2009, Linus Torvalds wrote:
So we can do all that locklessly and optimistically, just going back and
verifying the results later. This is why thread_info is actually a
better thing to use than task_struct - we can look up the cpu in it with
a simple dereference. We knew the
On Wed, Jan 07, 2009 at 03:19:38PM -0500, jim owens wrote:
Add call to LSM security initialization and save
resulting security xattr for new inodes.
Add xattr support to symlink inode ops.
Set inode-i_op for existing special files.
Signed-off-by: jim owens jow...@hp.com
---
On Wed, 7 Jan 2009, Steven Rostedt wrote:
Next comes the issue to know if the owner is still running. Wouldn't we
need to do something like
if (task_thread_info(cpu_rq(cpu)-curr) == owner)
Yes. After verifying that cpu is in a valid range.
I understand that this should not be a
On Wed, Jan 07, 2009 at 12:55:49PM -0800, Linus Torvalds wrote:
void loop_while_oncpu(struct mutex *lock, struct thread_struct *thread)
{
for (;;) {
unsigned cpu;
struct runqueue *rq;
if (lock-owner
On Wed, 7 Jan 2009, Matthew Wilcox wrote:
I appreciate this is sample code, but using __get_user() on
non-userspace pointers messes up architectures which have separate
user/kernel spaces (eg the old 4G/4G split for x86-32). Do we have an
appropriate function for kernel space pointers?
I appreciate this is sample code, but using __get_user() on
non-userspace pointers messes up architectures which have separate
user/kernel spaces (eg the old 4G/4G split for x86-32). Do we have an
appropriate function for kernel space pointers?
probe_kernel_address().
But it's slow.
-Andi
* Linus Torvalds torva...@linux-foundation.org wrote:
/*
* Even if the access succeeded (likely case),
* the cpu field may no longer be valid. FIXME:
* this needs to validate that we can do a
On Wed, 7 Jan 2009 22:37:40 +0100
Andi Kleen a...@firstfloor.org wrote:
But we can do that with __get_user(thread_info-cpu) (very unlikely page
fault protection due to the possibility of CONFIG_DEBUG_PAGEALLOC) and
then validating the cpu. It it's in range, we can use it and verify
On Wed, 7 Jan 2009, Linus Torvalds wrote:
We've needed that before (and yes, we've simply mis-used __get_user() on
x86 before rather than add it).
Ahh, yeah, we have a really broken form of this in probe_kernel_address(),
but it's disgustingly slow. And it actually does a whole lot more,
On Wed, 7 Jan 2009 22:32:22 +0100
Ingo Molnar mi...@elte.hu wrote:
We could do the whole oldfs = get_fs(); set_fs(KERNEL_DS); ..
set_fs(oldfs); crud, but it would probably be better to just add an
architected accessor. Especially since it's going to generally just be a
#define
On Wed, 2009-01-07 at 12:55 -0800, Linus Torvalds wrote:
/*
* Look out! thread is an entirely speculative pointer
* access and not reliable.
*/
void loop_while_oncpu(struct mutex *lock, struct thread_struct *thread)
{
for (;;) {
* Andrew Morton a...@linux-foundation.org wrote:
On Wed, 7 Jan 2009 22:32:22 +0100
Ingo Molnar mi...@elte.hu wrote:
We could do the whole oldfs = get_fs(); set_fs(KERNEL_DS); ..
set_fs(oldfs); crud, but it would probably be better to just add an
architected accessor. Especially
On Wed, 7 Jan 2009, Peter Zijlstra wrote:
Do we really have to re-do all that code every loop?
No, you're right, we can just look up the cpu once. Which makes Andrew's
argument that probe_kernel_address() isn't in any hot path even more
true.
Also, it would still need to do the funny:
On Wed, 7 Jan 2009, Linus Torvalds wrote:
We don't actually care that it only happens once: this all has _known_
races, and the cpu_relax() is a barrier.
I phrased that badly. It's not that it has known races, it's really that
the whole code sequence is very much written and intended to
On Wed, 2009-01-07 at 13:58 -0800, Linus Torvalds wrote:
On Wed, 7 Jan 2009, Peter Zijlstra wrote:
Do we really have to re-do all that code every loop?
No, you're right, we can just look up the cpu once. Which makes Andrew's
argument that probe_kernel_address() isn't in any hot path
Andi Kleen wrote:
I appreciate this is sample code, but using __get_user() on
non-userspace pointers messes up architectures which have separate
user/kernel spaces (eg the old 4G/4G split for x86-32). Do we have an
appropriate function for kernel space pointers?
Hi Ingo,
Ingo Molnar wrote:
* Gregory Haskins ghask...@novell.com wrote:
Can I ask a simple question in light of all this discussion?
Is get_task_struct() really that bad?
it dirties a cacheline and it also involves atomics.
Yes, understood. But we should note we are always
On Wed, 2009-01-07 at 13:58 -0800, Linus Torvalds wrote:
On Wed, 7 Jan 2009, Peter Zijlstra wrote:
Do we really have to re-do all that code every loop?
No, you're right, we can just look up the cpu once. Which makes Andrew's
argument that probe_kernel_address() isn't in any hot path
On Wed, 7 Jan 2009, Gregory Haskins wrote:
Hi Ingo,
Ingo Molnar wrote:
* Gregory Haskins ghask...@novell.com wrote:
Can I ask a simple question in light of all this discussion?
Is get_task_struct() really that bad?
it dirties a cacheline and it also involves
On Wed, 7 Jan 2009, Peter Zijlstra wrote:
Hmm, still wouldn't the spin_on_owner() loopyness and the above need
that need_resched() check you mentioned to that it can fall into the
slow path and go to sleep?
Yes, I do think that the outer loop at least should have a test for
On Wed, 2009-01-07 at 15:51 -0700, Peter W. Morreale wrote:
On Wed, 2009-01-07 at 23:33 +0100, Ingo Molnar wrote:
* Gregory Haskins ghask...@novell.com wrote:
Can I ask a simple question in light of all this discussion?
Is get_task_struct() really that bad?
it dirties a
On Wed, 7 Jan 2009, Gregory Haskins wrote:
Can I ask a simple question in light of all this discussion?
Is get_task_struct() really that bad?
Yes. It's an atomic access (two, in fact, since you need to release it
too), which is a huge deal if we're talking about a timing-critical
On Wed, 7 Jan 2009, Linus Torvalds wrote:
Is get_task_struct() really that bad?
Yes. It's an atomic access (two, in fact, since you need to release it
too), which is a huge deal if we're talking about a timing-critical
section of code.
There's another issue: you also need to lock
On Wed, 7 Jan 2009, Dave Kleikamp wrote:
Do you need to even do that if CONFIG_DEBUG_PAGEALLOC is unset?
No.
Something like:
#ifdef CONFIG_DEBUG_PAGEALLOC
/*
* Need to access the cpu field knowing that
* DEBUG_PAGEALLOC could have
On Wed, 7 Jan 2009, Steven Rostedt wrote:
What would be interesting is various benchmarks against all three.
1) no mutex spinning.
2) get_task_struct() implementation.
3) spin_or_sched implementation.
One of the issues is that I cannot convince myself that (2) is even
necessarily
On Wed, 7 Jan 2009, Steven Rostedt wrote:
True. I need to keep looking at the code that is posted. In -rt, we force
the fast path into the slowpath as soon as another task fails to get the
lock. Without that, as you pointed out, the code can be racy.
I mean we force the fast unlock path
On Wed, 7 Jan 2009, Steven Rostedt wrote:
On Wed, 7 Jan 2009, Steven Rostedt wrote:
True. I need to keep looking at the code that is posted. In -rt, we force
the fast path into the slowpath as soon as another task fails to get the
lock. Without that, as you pointed out, the code
On Wed, 7 Jan 2009, Steven Rostedt wrote:
Should that be:
#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_MEMORY_HOTPLUG)
Well, probably CONFIG_MEMORY_HOTREMOVE, no? And I'd actually suggest that
unplugging should have a stop-machine if it doesn't already, just because
it's such a
Josef Bacik wrote:
On Wed, Jan 07, 2009 at 03:19:38PM -0500, jim owens wrote:
+int btrfs_xattr_security_init(struct inode *inode, struct inode *dir)
+{
+ int err;
+ size_t len;
+ void *value;
+ char *suffix;
+ char *name;
+
+ err =
On Wed, Jan 07, 2009 at 07:22:58PM -0500, jim owens wrote:
Josef Bacik wrote:
On Wed, Jan 07, 2009 at 03:19:38PM -0500, jim owens wrote:
+int btrfs_xattr_security_init(struct inode *inode, struct inode *dir)
+{
+ int err;
+ size_t len;
+ void *value;
+ char *suffix;
+ char
On Wed, 7 Jan 2009 15:57:06 -0800 (PST)
Linus Torvalds torva...@linux-foundation.org wrote:
On Wed, 7 Jan 2009, Steven Rostedt wrote:
Should that be:
#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_MEMORY_HOTPLUG)
Well, probably CONFIG_MEMORY_HOTREMOVE, no? And I'd
On Wed, 7 Jan 2009, Linus Torvalds wrote:
Should that be:
#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_MEMORY_HOTPLUG)
Well, probably CONFIG_MEMORY_HOTREMOVE, no? And I'd actually suggest that
unplugging should have a stop-machine if it doesn't already, just because
it's
On Wed, 7 Jan 2009 21:33:31 -0500 (EST)
Steven Rostedt rost...@goodmis.org wrote:
On Wed, 7 Jan 2009, Linus Torvalds wrote:
Should that be:
#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_MEMORY_HOTPLUG)
Well, probably CONFIG_MEMORY_HOTREMOVE, no? And I'd actually suggest
[resend: i fat fingered the reply-to-all for a few messages]
Linus Torvalds torva...@linux-foundation.org 01/07/09 6:20 PM
On Wed, 7 Jan 2009, Linus Torvalds wrote:
Is get_task_struct() really that bad?
Yes. It's an atomic access (two, in fact, since you need to release it
too), which
[resend: restore CC list]
Linus Torvalds torva...@linux-foundation.org 01/07/09 6:33 PM
On Wed, 7 Jan 2009, Steven Rostedt wrote:
What would be interesting is various benchmarks against all three.
1) no mutex spinning.
2) get_task_struct() implementation.
3) spin_or_sched
Steven Rostedt wrote:
On Wed, 7 Jan 2009, Gregory Haskins wrote:
In my defense, the -rt versions of the patches guarantee this is ok
based on a little hack:
The -rt versions worry about much more than what the mutex code in
mainline does. Linus is correct in his arguments. The
On Wed, 2009-01-07 at 15:32 -0800, Linus Torvalds wrote:
On Wed, 7 Jan 2009, Steven Rostedt wrote:
What would be interesting is various benchmarks against all three.
1) no mutex spinning.
2) get_task_struct() implementation.
3) spin_or_sched implementation.
One of the issues is
59 matches
Mail list logo
