Re: [(RT RFC) PATCH v2 3/9] rearrange rt_spin_lock sleep
>>> On Mon, Feb 25, 2008 at 4:54 PM, in message
<[EMAIL PROTECTED]>, Pavel Machek <[EMAIL PROTECTED]> wrote:
> Hi!
>
>> @@ -720,7 +728,8 @@ rt_spin_lock_slowlock(struct rt_mutex *lock)
>> * saved_state accordingly. If we did not get a real wakeup
>> * then we return with the saved state.
>> */
>> -saved_state = xchg(>state, TASK_UNINTERRUPTIBLE);
>> +saved_state = current->state;
>> +smp_mb();
>>
>> for (;;) {
>> unsigned long saved_flags;
>
> Please document what the barrier is good for.
Yeah, I think you are right that this isn't needed. I think that is a relic
from back when I was debugging some other problems. Let me wrap my head around
the implications of removing it, and either remove it or document appropriately.
>
> Plus, you are replacing atomic operation with nonatomic; is that ok?
Yeah, I think so. We are substituting a write with a read, and word reads are
always atomic anyway IIUC (or is that only true on certain architectures)?
Note that we are moving the atomic-write to be done later in the
update_current() calls.
-Greg
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [(RT RFC) PATCH v2 3/9] rearrange rt_spin_lock sleep
Hi!
> @@ -720,7 +728,8 @@ rt_spin_lock_slowlock(struct rt_mutex *lock)
>* saved_state accordingly. If we did not get a real wakeup
>* then we return with the saved state.
>*/
> - saved_state = xchg(>state, TASK_UNINTERRUPTIBLE);
> + saved_state = current->state;
> + smp_mb();
>
> for (;;) {
> unsigned long saved_flags;
Please document what the barrier is good for.
Plus, you are replacing atomic operation with nonatomic; is that ok?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[(RT RFC) PATCH v2 3/9] rearrange rt_spin_lock sleep
The current logic makes rather coarse adjustments to current->state since
it is planning on sleeping anyway. We want to eventually move to an
adaptive (e.g. optional sleep) algorithm, so we tighten the scope of the
adjustments to bracket the schedule(). This should yield correct behavior
with or without the adaptive features that are added later in the series.
We add it here as a separate patch for greater review clarity on smaller
changes.
Signed-off-by: Gregory Haskins <[EMAIL PROTECTED]>
---
kernel/rtmutex.c | 20 +++-
1 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/kernel/rtmutex.c b/kernel/rtmutex.c
index cd39c26..ef52db6 100644
--- a/kernel/rtmutex.c
+++ b/kernel/rtmutex.c
@@ -681,6 +681,14 @@ rt_spin_lock_fastunlock(struct rt_mutex *lock,
slowfn(lock);
}
+static inline void
+update_current(unsigned long new_state, unsigned long *saved_state)
+{
+ unsigned long state = xchg(>state, new_state);
+ if (unlikely(state == TASK_RUNNING))
+ *saved_state = TASK_RUNNING;
+}
+
/*
* Slow path lock function spin_lock style: this variant is very
* careful not to miss any non-lock wakeups.
@@ -720,7 +728,8 @@ rt_spin_lock_slowlock(struct rt_mutex *lock)
* saved_state accordingly. If we did not get a real wakeup
* then we return with the saved state.
*/
- saved_state = xchg(>state, TASK_UNINTERRUPTIBLE);
+ saved_state = current->state;
+ smp_mb();
for (;;) {
unsigned long saved_flags;
@@ -752,14 +761,15 @@ rt_spin_lock_slowlock(struct rt_mutex *lock)
debug_rt_mutex_print_deadlock();
- schedule_rt_mutex(lock);
+ update_current(TASK_UNINTERRUPTIBLE, _state);
+ if (waiter.task)
+ schedule_rt_mutex(lock);
+ else
+ update_current(TASK_RUNNING_MUTEX, _state);
spin_lock_irqsave(>wait_lock, flags);
current->flags |= saved_flags;
current->lock_depth = saved_lock_depth;
- state = xchg(>state, TASK_UNINTERRUPTIBLE);
- if (unlikely(state == TASK_RUNNING))
- saved_state = TASK_RUNNING;
}
state = xchg(>state, saved_state);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[(RT RFC) PATCH v2 3/9] rearrange rt_spin_lock sleep
The current logic makes rather coarse adjustments to current-state since
it is planning on sleeping anyway. We want to eventually move to an
adaptive (e.g. optional sleep) algorithm, so we tighten the scope of the
adjustments to bracket the schedule(). This should yield correct behavior
with or without the adaptive features that are added later in the series.
We add it here as a separate patch for greater review clarity on smaller
changes.
Signed-off-by: Gregory Haskins [EMAIL PROTECTED]
---
kernel/rtmutex.c | 20 +++-
1 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/kernel/rtmutex.c b/kernel/rtmutex.c
index cd39c26..ef52db6 100644
--- a/kernel/rtmutex.c
+++ b/kernel/rtmutex.c
@@ -681,6 +681,14 @@ rt_spin_lock_fastunlock(struct rt_mutex *lock,
slowfn(lock);
}
+static inline void
+update_current(unsigned long new_state, unsigned long *saved_state)
+{
+ unsigned long state = xchg(current-state, new_state);
+ if (unlikely(state == TASK_RUNNING))
+ *saved_state = TASK_RUNNING;
+}
+
/*
* Slow path lock function spin_lock style: this variant is very
* careful not to miss any non-lock wakeups.
@@ -720,7 +728,8 @@ rt_spin_lock_slowlock(struct rt_mutex *lock)
* saved_state accordingly. If we did not get a real wakeup
* then we return with the saved state.
*/
- saved_state = xchg(current-state, TASK_UNINTERRUPTIBLE);
+ saved_state = current-state;
+ smp_mb();
for (;;) {
unsigned long saved_flags;
@@ -752,14 +761,15 @@ rt_spin_lock_slowlock(struct rt_mutex *lock)
debug_rt_mutex_print_deadlock(waiter);
- schedule_rt_mutex(lock);
+ update_current(TASK_UNINTERRUPTIBLE, saved_state);
+ if (waiter.task)
+ schedule_rt_mutex(lock);
+ else
+ update_current(TASK_RUNNING_MUTEX, saved_state);
spin_lock_irqsave(lock-wait_lock, flags);
current-flags |= saved_flags;
current-lock_depth = saved_lock_depth;
- state = xchg(current-state, TASK_UNINTERRUPTIBLE);
- if (unlikely(state == TASK_RUNNING))
- saved_state = TASK_RUNNING;
}
state = xchg(current-state, saved_state);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [(RT RFC) PATCH v2 3/9] rearrange rt_spin_lock sleep
Hi!
@@ -720,7 +728,8 @@ rt_spin_lock_slowlock(struct rt_mutex *lock)
* saved_state accordingly. If we did not get a real wakeup
* then we return with the saved state.
*/
- saved_state = xchg(current-state, TASK_UNINTERRUPTIBLE);
+ saved_state = current-state;
+ smp_mb();
for (;;) {
unsigned long saved_flags;
Please document what the barrier is good for.
Plus, you are replacing atomic operation with nonatomic; is that ok?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [(RT RFC) PATCH v2 3/9] rearrange rt_spin_lock sleep
On Mon, Feb 25, 2008 at 4:54 PM, in message
[EMAIL PROTECTED], Pavel Machek [EMAIL PROTECTED] wrote:
Hi!
@@ -720,7 +728,8 @@ rt_spin_lock_slowlock(struct rt_mutex *lock)
* saved_state accordingly. If we did not get a real wakeup
* then we return with the saved state.
*/
-saved_state = xchg(current-state, TASK_UNINTERRUPTIBLE);
+saved_state = current-state;
+smp_mb();
for (;;) {
unsigned long saved_flags;
Please document what the barrier is good for.
Yeah, I think you are right that this isn't needed. I think that is a relic
from back when I was debugging some other problems. Let me wrap my head around
the implications of removing it, and either remove it or document appropriately.
Plus, you are replacing atomic operation with nonatomic; is that ok?
Yeah, I think so. We are substituting a write with a read, and word reads are
always atomic anyway IIUC (or is that only true on certain architectures)?
Note that we are moving the atomic-write to be done later in the
update_current() calls.
-Greg
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
