* [PATCH 1/2] sched_rt: don't start timer when rt bandwidth disabled
@ 2009-02-05 23:36 Hiroshi Shimamoto
2009-02-05 23:38 ` [PATCH 2/2] sched_rt: protect rt_rq->rt_time by rt_runtime_lock Hiroshi Shimamoto
0 siblings, 1 reply; 5+ messages in thread
From: Hiroshi Shimamoto @ 2009-02-05 23:36 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra; +Cc: linux-kernel
From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
Impact: fix incorrect condition check
No need to start rt bandwidth timer when rt bandwidth is disabled.
Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
---
kernel/sched.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/sched.c b/kernel/sched.c
index 1e965b7..4c03c88 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -223,7 +223,7 @@ static void start_rt_bandwidth(struct rt_bandwidth *rt_b)
{
ktime_t now;
- if (rt_bandwidth_enabled() && rt_b->rt_runtime == RUNTIME_INF)
+ if (!rt_bandwidth_enabled() || rt_b->rt_runtime == RUNTIME_INF)
return;
if (hrtimer_active(&rt_b->rt_period_timer))
--
1.6.1.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] sched_rt: protect rt_rq->rt_time by rt_runtime_lock
2009-02-05 23:36 [PATCH 1/2] sched_rt: don't start timer when rt bandwidth disabled Hiroshi Shimamoto
@ 2009-02-05 23:38 ` Hiroshi Shimamoto
2009-02-05 23:59 ` Peter Zijlstra
0 siblings, 1 reply; 5+ messages in thread
From: Hiroshi Shimamoto @ 2009-02-05 23:38 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra; +Cc: linux-kernel
From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
Impact: fix possible race condition
rt_rq->rt_time should be protected.
Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
---
kernel/sched_rt.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/sched_rt.c b/kernel/sched_rt.c
index 299d012..d7654a3 100644
--- a/kernel/sched_rt.c
+++ b/kernel/sched_rt.c
@@ -496,10 +496,10 @@ static int do_sched_rt_period_timer(struct rt_bandwidth *rt_b, int overrun)
struct rq *rq = rq_of_rt_rq(rt_rq);
spin_lock(&rq->lock);
+ spin_lock(&rt_rq->rt_runtime_lock);
if (rt_rq->rt_time) {
u64 runtime;
- spin_lock(&rt_rq->rt_runtime_lock);
if (rt_rq->rt_throttled)
balance_runtime(rt_rq);
runtime = rt_rq->rt_runtime;
@@ -510,9 +510,9 @@ static int do_sched_rt_period_timer(struct rt_bandwidth *rt_b, int overrun)
}
if (rt_rq->rt_time || rt_rq->rt_nr_running)
idle = 0;
- spin_unlock(&rt_rq->rt_runtime_lock);
} else if (rt_rq->rt_nr_running)
idle = 0;
+ spin_unlock(&rt_rq->rt_runtime_lock);
if (enqueue)
sched_rt_rq_enqueue(rt_rq);
--
1.6.1.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 2/2] sched_rt: protect rt_rq->rt_time by rt_runtime_lock
2009-02-05 23:38 ` [PATCH 2/2] sched_rt: protect rt_rq->rt_time by rt_runtime_lock Hiroshi Shimamoto
@ 2009-02-05 23:59 ` Peter Zijlstra
2009-02-06 0:06 ` Hiroshi Shimamoto
0 siblings, 1 reply; 5+ messages in thread
From: Peter Zijlstra @ 2009-02-05 23:59 UTC (permalink / raw)
To: Hiroshi Shimamoto; +Cc: Ingo Molnar, linux-kernel
On Thu, 2009-02-05 at 15:38 -0800, Hiroshi Shimamoto wrote:
> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
>
> Impact: fix possible race condition
>
> rt_rq->rt_time should be protected.
Don't think so, all we do is check the value for non-zero outside the
lock, that should be ok.
If there's still a race, your changelog utterly fails to mention the
race scenario.
> Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> ---
> kernel/sched_rt.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched_rt.c b/kernel/sched_rt.c
> index 299d012..d7654a3 100644
> --- a/kernel/sched_rt.c
> +++ b/kernel/sched_rt.c
> @@ -496,10 +496,10 @@ static int do_sched_rt_period_timer(struct rt_bandwidth *rt_b, int overrun)
> struct rq *rq = rq_of_rt_rq(rt_rq);
>
> spin_lock(&rq->lock);
> + spin_lock(&rt_rq->rt_runtime_lock);
> if (rt_rq->rt_time) {
> u64 runtime;
>
> - spin_lock(&rt_rq->rt_runtime_lock);
> if (rt_rq->rt_throttled)
> balance_runtime(rt_rq);
> runtime = rt_rq->rt_runtime;
> @@ -510,9 +510,9 @@ static int do_sched_rt_period_timer(struct rt_bandwidth *rt_b, int overrun)
> }
> if (rt_rq->rt_time || rt_rq->rt_nr_running)
> idle = 0;
> - spin_unlock(&rt_rq->rt_runtime_lock);
> } else if (rt_rq->rt_nr_running)
> idle = 0;
> + spin_unlock(&rt_rq->rt_runtime_lock);
>
> if (enqueue)
> sched_rt_rq_enqueue(rt_rq);
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 2/2] sched_rt: protect rt_rq->rt_time by rt_runtime_lock
2009-02-05 23:59 ` Peter Zijlstra
@ 2009-02-06 0:06 ` Hiroshi Shimamoto
2009-02-06 18:50 ` Hiroshi Shimamoto
0 siblings, 1 reply; 5+ messages in thread
From: Hiroshi Shimamoto @ 2009-02-06 0:06 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Ingo Molnar, linux-kernel
Peter Zijlstra wrote:
> On Thu, 2009-02-05 at 15:38 -0800, Hiroshi Shimamoto wrote:
>> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
>>
>> Impact: fix possible race condition
>>
>> rt_rq->rt_time should be protected.
>
> Don't think so, all we do is check the value for non-zero outside the
> lock, that should be ok.
>
> If there's still a race, your changelog utterly fails to mention the
> race scenario.
OK. I just noticed rt_time is accessed with rt_runtime_lock held except
this point. Will look at this again.
Thanks,
Hiroshi
>
>> Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
>> ---
>> kernel/sched_rt.c | 4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/sched_rt.c b/kernel/sched_rt.c
>> index 299d012..d7654a3 100644
>> --- a/kernel/sched_rt.c
>> +++ b/kernel/sched_rt.c
>> @@ -496,10 +496,10 @@ static int do_sched_rt_period_timer(struct rt_bandwidth *rt_b, int overrun)
>> struct rq *rq = rq_of_rt_rq(rt_rq);
>>
>> spin_lock(&rq->lock);
>> + spin_lock(&rt_rq->rt_runtime_lock);
>> if (rt_rq->rt_time) {
>> u64 runtime;
>>
>> - spin_lock(&rt_rq->rt_runtime_lock);
>> if (rt_rq->rt_throttled)
>> balance_runtime(rt_rq);
>> runtime = rt_rq->rt_runtime;
>> @@ -510,9 +510,9 @@ static int do_sched_rt_period_timer(struct rt_bandwidth *rt_b, int overrun)
>> }
>> if (rt_rq->rt_time || rt_rq->rt_nr_running)
>> idle = 0;
>> - spin_unlock(&rt_rq->rt_runtime_lock);
>> } else if (rt_rq->rt_nr_running)
>> idle = 0;
>> + spin_unlock(&rt_rq->rt_runtime_lock);
>>
>> if (enqueue)
>> sched_rt_rq_enqueue(rt_rq);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 2/2] sched_rt: protect rt_rq->rt_time by rt_runtime_lock
2009-02-06 0:06 ` Hiroshi Shimamoto
@ 2009-02-06 18:50 ` Hiroshi Shimamoto
0 siblings, 0 replies; 5+ messages in thread
From: Hiroshi Shimamoto @ 2009-02-06 18:50 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Ingo Molnar, linux-kernel
Hiroshi Shimamoto wrote:
> Peter Zijlstra wrote:
>> On Thu, 2009-02-05 at 15:38 -0800, Hiroshi Shimamoto wrote:
>>> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
>>>
>>> Impact: fix possible race condition
>>>
>>> rt_rq->rt_time should be protected.
>> Don't think so, all we do is check the value for non-zero outside the
>> lock, that should be ok.
OK, now I see. I didn't find any real race.
Thanks,
Hiroshi
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-02-06 18:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-02-05 23:36 [PATCH 1/2] sched_rt: don't start timer when rt bandwidth disabled Hiroshi Shimamoto
2009-02-05 23:38 ` [PATCH 2/2] sched_rt: protect rt_rq->rt_time by rt_runtime_lock Hiroshi Shimamoto
2009-02-05 23:59 ` Peter Zijlstra
2009-02-06 0:06 ` Hiroshi Shimamoto
2009-02-06 18:50 ` Hiroshi Shimamoto
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).