On Thu, 11 Jun 2009, Chris Mason wrote:
Existing filesystems will be upgraded to the new format on the first
mount. All of your old data will still be there and still work
properly, but I strongly recommend a full backup before going to the new
code.
Auugh.
This is horrible. I just
On Thu, 15 Jan 2009, Ingo Molnar wrote:
btw., i think spin-mutexes have a design advantage here: in a lot of code
areas it's quite difficult to use spinlocks - cannot allocate memory,
cannot call any code that can sporadically block (but does not _normally_
block), etc.
With mutexes
On Thu, 15 Jan 2009, Paul E. McKenney wrote:
On Thu, Jan 15, 2009 at 10:16:53AM -0800, Linus Torvalds wrote:
IOW, if you do pre-allocation instead of holding a lock over the
allocation, you win. So yes, spin-mutexes makes it easier to write the
code, but it also makes it easier
On Wed, 14 Jan 2009, Ingo Molnar wrote:
* Chris Mason chris.ma...@oracle.com wrote:
v10 is better that not spinning, but its in the 5-10% range. So, I've
been trying to find ways to close the gap, just to understand exactly
where it is different.
If I take out:
/*
On Tue, 13 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.
Okey, dokey. Looks reasonable, but I wonder if this part came from v8 and
wasn't intentional:
+
On Tue, 13 Jan 2009, Peter Zijlstra wrote:
Ok, tested only 1, but that was the one I remember lockups from -- and
that seems to be good with the cmpxchg.
Do you fancy me sending v10 or will you make that change locally?
I'd like to get this in, but I'm not going to apply it in any case
On Tue, 13 Jan 2009, Ingo Molnar wrote:
And v8 is rock solid in all my testing - and i'll give v10 a similar
workout as well.
The differences between v8 and v10 are very fundamental, since v8 does the
spinning inside the spinlock'ed loop (the spinning itself is not inside
the spinlock,
On Tue, 13 Jan 2009, Ingo Molnar wrote:
below is the v8 - v10 delta patch - for all who'd like to review the
changes.
Hmm. This does seem to indicate that v10 lost many of the preempt changes.
Probably because Peter went back to v7 to create it.
I also wonder about the timing numbers -
On Mon, 12 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.
This version seems worse in so many ways.
You made the mutex bigger, for no obvious reason.
You made it
On Mon, 12 Jan 2009, Andi Kleen wrote:
What I find nonsensical is that -fno-strict-aliasing generates
better code here. Normally one would expect the compiler seeing
more aliases with that option and then be more conservative
regarding any sharing. But it seems to be the other way round
On Mon, 12 Jan 2009, H. Peter Anvin wrote:
This is about storage allocation, not aliases. Storage allocation only
depends on lifetime.
Well, the thing is, code motion does extend life-times, and if you think
you can move stores across each other (even when you can see that they
alias
On Mon, 12 Jan 2009, Bernd Schmidt wrote:
However, if the compiler chooses to put them into the same stack
location, the RTL-based alias analysis will happily conclude (based on
the differing types) that the reads from A and the writes to B can't
possibly conflict, and some passes may
On Mon, 12 Jan 2009, Bernd Schmidt wrote:
Too lazy to construct one myself, I googled for examples, and here's a
trivial one that shows how it affects the ability of the compiler to
eliminate memory references:
Do you really think this is realistic or even relevant?
The fact is
(a)
On Sun, 11 Jan 2009, Andi Kleen wrote:
Was -- i think that got fixed in gcc. But again only in newer versions.
I doubt it. People have said that about a million times, it has never
gotten fixed, and I've never seen any actual proof.
I think that what got fixed was that gcc now at least
On Sat, 10 Jan 2009, Andi Kleen wrote:
I think that's mostly because kexec from arbitary context is a
somewhat unstable concept.
I think that's the understatement of the year.
We have tons of problems with standard suspend-to-ram, and that's when the
suspend sequence has done its best to
On Fri, 9 Jan 2009, Ingo Molnar wrote:
-static inline int constant_test_bit(int nr, const volatile unsigned long
*addr)
+static __asm_inline int
+constant_test_bit(int nr, const volatile unsigned long *addr)
{
return ((1UL (nr % BITS_PER_LONG))
(((unsigned long
On Fri, 9 Jan 2009, Steven Rostedt wrote:
I was going to say a while ago...
In PREEMPT=y the need_resched() is not needed at all. If you have
preemption enabled, you will get preempted in that loop. No need for the
need_resched() in the outer loop. Although I'm not sure how it would even
On Fri, 9 Jan 2009, H. Peter Anvin wrote:
__asm_inline was my suggestion, to distinguish inline this
unconditionally because gcc screws up in the presence of asm()
THERE IS NO ASM IN THERE!
Guys, look at the code. No asm. The whole notion that gcc gets confused by
inline asms IS BOGUS.
On Fri, 9 Jan 2009, H. Peter Anvin wrote:
As far as naming is concerned, gcc effectively supports four levels,
which *currently* map onto macros as follows:
__always_inline Inline unconditionally
inlineInlining hint
nothing Standard
On Fri, 9 Jan 2009, Andi Kleen wrote:
There's also one alternative: gcc's inlining algorithms are extensibly
tunable with --param. We might be able to find a set of numbers that
make it roughly work like we want it by default.
We tried that.
IIRC, the numbers mean different things for
On Fri, 9 Jan 2009, Steven Rostedt wrote:
I vote for the, get rid of the current inline, rename __always_inline to
inline, and then remove all non needed inlines from the kernel.
This is what we do all the time, and historically have always done.
But
- CONFIG_OPTIMIZE_INLINING=y screws
On Fri, 9 Jan 2009, Matthew Wilcox wrote:
That seems like valuable feedback to give to the GCC developers.
Well, one thing we should remember is that the kernel really _is_ special.
The kernel not only does things no other program tends to do (inline asms
are unusual in the first place -
On Fri, 9 Jan 2009, Andi Kleen wrote:
Universal noinline would also be a bad idea because of its
costs (4.1% text size increase). Perhaps should make it
a CONFIG option for debugging though.
That's _totally_ the wrong way.
If you can reproduce an issue on your machine, you generally don't
On Fri, 9 Jan 2009, Matthew Wilcox wrote:
Now, I'm not going to argue the directIO code is a shining example of
how we want things to look, but we don't really want ten arguments
being marshalled into a function call; we want gcc to inline the
direct_io_worker() and do its best to optimise
On Fri, 9 Jan 2009, Nicholas Miell wrote:
Maybe the kernel's backtrace code should be fixed instead of blaming
gcc.
And maybe people who don't know what they are talking about shouldn't
speak?
You just loaded the whole f*cking debug info just to do that exact
analysis. Guess how big it
On Fri, 9 Jan 2009, Ingo Molnar wrote:
So, should we not remove CONFIG_OPTIMIZE_INLINING, then the correct one
would be to mark it __always_inline [__asm_inline is senseless there], or
the second patch below that changes the bit parameter to unsigned int.
Well, I certainly don't want to
On Fri, 9 Jan 2009, Harvey Harrison wrote:
__needs_inline? That would imply that it's for correctness reasons.
.. but the point is, we have _thousands_ of inlines, and do you know which
is which? We've historically forced them to be inlined, and every time
somebody does that
On Fri, 9 Jan 2009, Harvey Harrison wrote:
Of course, at that point you might as well argue that the thing should not
exist at all, and that such a flag should just be removed entirely. Which
I certainly agree with - I think the only flag we need is inline, and I
think it should
On Sat, 10 Jan 2009, Ingo Molnar wrote:
may_inline/inline_hint is a longer, less known and uglier keyword.
Hey, your choice, should you decide to accept it, is to just get rid of
them entirely.
You claim that we're back to square one, but that's simply the way things
are. Either inline
On Fri, 9 Jan 2009, Nicholas Miell wrote:
So take your complaint about gcc's decision to inline functions called
once.
Actually, the called once really is a red herring. The big complaint is
too aggressively when not asked for. It just so happens that the called
once logic is right now
On Sat, 10 Jan 2009, Andi Kleen wrote:
What's the cost/benefit of that 4%? Does it actually improve performance?
Especially if you then want to keep DWARF unwind information in memory in
order to fix up some of the problems it causes? At that point, you lost
dwarf unwind information
On Sat, 10 Jan 2009, Jamie Lokier wrote:
Does always_inline complain if the function isn't inlinable, while
inline allows it? That would explain the alpha comment.
I suspect it dates back to gcc-3.1 days. It's from 2004. And the author of
that comment is a part-time gcc hacker who was
On Sat, 10 Jan 2009, Ingo Molnar wrote:
Well, it's not totally meaningless. To begin with, defining 'inline' to
mean 'always inline' is a Linux kernel definition. So we already changed
the behavior - in the hope of getting it right most of the time and in the
hope of thus improving the
On Fri, 9 Jan 2009, Harvey Harrison wrote:
On Sat, 2009-01-10 at 02:01 +0100, Ingo Molnar wrote:
- Headers could probably go back to 'extern inline' again. At not small
expense - we just finished moving to 'static inline'. We'd need to
guarantee a library instantiation for
On Fri, 9 Jan 2009, H. Peter Anvin wrote:
I was thinking about experimenting with this, to see what level of
upside it might add. Ingo showed me numbers which indicate that a
fairly significant fraction of the cases where removing inline helps is
in .h files, which would require code
On Thu, 8 Jan 2009, Peter Zijlstra wrote:
This was done because the interaction between trylock_slowpath and that
-1000 state hurt my head.
Yeah, it was a stupid hacky thing to avoid the list_empty(), but doing
it explicitly is fine (we don't hold the lock, so the list isn't
necessarily
On Thu, 8 Jan 2009, Steven Rostedt wrote:
We keep spinning if the owner changes.
I think we want to - if you have multiple CPU's and a heavily contended
lock that acts as a spinlock, we still _do_ want to keep spinning even if
another CPU gets the lock.
And I don't even believe that is
Unrelated:
On Thu, 8 Jan 2009, Chris Mason wrote:
RIP: 0010:[8024f4de] [8024f4de] __cmpxchg+0x36/0x3f
Ouch. HOW THE HELL DID THAT NOT GET INLINED?
cmpxchg() is a _single_ instruction if it's inlined, but it's a horrible
mess of dynamic conditionals on the (constant - per
On Thu, 8 Jan 2009, Steven Rostedt wrote:
Ouch! I think you are on to something:
Yeah, there's somethign there, but looking at Chris' backtrace, there's
nothing there to disable preemption. So if it was this simple case, it
should still have preempted him to let the other process run and
On Thu, 8 Jan 2009, H. Peter Anvin wrote:
Right. gcc simply doesn't have any way to know how heavyweight an
asm() statement is
I don't think that's relevant.
First off, gcc _does_ have a perfectly fine notion of how heavy-weight an
asm statement is: just count it as a single instruction
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, 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
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 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, 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?
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, 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
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, 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:
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
On Tue, 6 Jan 2009, Linus Torvalds wrote:
The other way around, you mean: we spin until the owner is no longer
holding a cpu.
Btw, I hate the name of the helper function for that. task_is_current()?
current means something totally different in the linux kernel: it means
that the task
On Tue, 6 Jan 2009, Linus Torvalds wrote:
Right now, if some process deadlocks on a mutex, we get hung process, but
with a nice backtrace and hopefully other things (that don't need that
lock) still continue to work.
Clarification: the nice backtrace we only get with something like
On Tue, 6 Jan 2009, Ingo Molnar wrote:
So instead of the BUG_ON() we could emit a WARN_ONCE() perhaps, plus not
do any spinning and just block - resulting in an uninterruptible task
(that the user will probably notice) and a scary message in the syslog?
[all in the slowpath]
Sure. You
Ok, last comment, I promise.
On Tue, 6 Jan 2009, Peter Zijlstra wrote:
@@ -175,11 +199,19 @@ __mutex_lock_common(struct mutex *lock,
debug_mutex_free_waiter(waiter);
return -EINTR;
}
- __set_task_state(task, state);
On Tue, 6 Jan 2009, Peter Zijlstra wrote:
With Linus' mutex_spin_or_schedule() function the whole - keeping
owner's task_struct alive issue goes away,.. now if only the thing would
boot...
Can you post the patch, so that we can see if we can find some silly error
that we can ridicule you
On Tue, 6 Jan 2009, Peter Zijlstra wrote:
One of the things the previous patch did wrong was that it never tracked
the owner in the mutex fast path -- I initially didn't notice because I
had all debugging infrastructure enabled, and that short circuits all
the fast paths.
Why even worry?
On Tue, 6 Jan 2009, Peter Zijlstra wrote:
In fact, I suspect that's the real bug you're hitting: you're enabling
preemption while holding a spinlock. That is NOT a good idea.
spinlocks also fiddle with preempt_count, that should all work out -
although granted, it does look funny.
On Tue, 6 Jan 2009, Peter Zijlstra wrote:
Indeed, the below does boot -- which means I get to sleep now ;-)
Well, if you didn't go to sleep, a few more questions..
int __sched
mutex_lock_killable_nested(struct mutex *lock, unsigned int subclass)
{
+ int ret;
+
101 - 163 of 163 matches
Mail list logo
