Re: [PATCH V2] MIPS: change type of asid_cache to unsigned long
On Tue, May 27, 2014 at 01:07:20PM +0800, Li Zefan wrote: > On 2014/5/27 12:50, Yong Zhang wrote: > > BTW, I realy don't care who credits the patch and Ralf said that > > he will applied the one which moves the place of udelay_val. > > > > Anyway, if your company pays you more money if you contribute to > > the community, just take it and talk about it with Ralf ;-) > > > > We don't do contribution for money, and I don't think you do, > but crediting properly is one of the reason that our kernel > community keeps prosperous for so many years, and that's one > of the reason we introduced Reported-by and Tested-by tags. I'll reply this email for the last time. To me your action is just like Reported-by, but I admit that you also do analysis. If you don't the way change it to whatever you want. Thanks, Yong -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] MIPS: change type of asid_cache to unsigned long
BTW, I realy don't care who credits the patch and Ralf said that
he will applied the one which moves the place of udelay_val.
Anyway, if your company pays you more money if you contribute to
the community, just take it and talk about it with Ralf ;-)
Thanks,
Yong
On Tue, May 27, 2014 at 12:16:30PM +0800, Li Zefan wrote:
> I'm not quite happy about what happaned here. There's a story behind
> this patch.
>
> One of our Huawei product encountered a bug, and they're using WindRiver4,
> so the kernel is 2.6.34.
>
> Because they bought your licnece, they asked for your help, but
> you were reluctant on this issue, and the problem remained there
> for about one month.
>
> At last they turned to us for help. We're the kernel department in
> Huawei, but maintaining this product kernel isn't our job. Still
> Li Bin devoted his time to analyzing this bug, and he did a great
> job.
>
> Li Bin told the product team what was wrong and was about to send
> a fix for upstream kernel. They told you our analysis for further
> confirmation, and you were so reluctant to help but so quick to
> send the fix.
>
> Li Bin never reported this bug, but he fixed it. It's a shame that
> you took the credit from us.
>
> On 2014/5/21 13:36, Yong Zhang wrote:
> > asid_cache must be unsigned long otherwise on 64bit system
> > it will become 0 if the value in get_new_mmu_context()
> > reaches 0x and in the end the assumption of
> > ASID_FIRST_VERSION is not true anymore thus leads to
> > more dangerous things.
> >
>
> We should describe what problem this bug can lead to, which
> will help people who encounter the same problem and google it.
>
> > Reported-by: libin
> > Signed-off-by: Yong Zhang
>
> Should mark the patch for stable trees. Though 2.6.34 is EOL,
> the fix should be backported to other kernels.
>
> > ---
> >
> > V2<-V1: Add the reporter.
> >
> > arch/mips/include/asm/cpu-info.h |2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/mips/include/asm/cpu-info.h
> > b/arch/mips/include/asm/cpu-info.h
> > index f6299be..ebcc2ed 100644
> > --- a/arch/mips/include/asm/cpu-info.h
> > +++ b/arch/mips/include/asm/cpu-info.h
> > @@ -40,7 +40,7 @@ struct cache_desc {
> >
> > struct cpuinfo_mips {
> > unsigned intudelay_val;
> > - unsigned intasid_cache;
> > + unsigned long asid_cache;
> >
> > /*
> > * Capability and feature descriptor structure for MIPS CPU
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] MIPS: change type of asid_cache to unsigned long
On Tue, May 27, 2014 at 12:16:30PM +0800, Li Zefan wrote:
> I'm not quite happy about what happaned here. There's a story behind
> this patch.
>
> One of our Huawei product encountered a bug, and they're using WindRiver4,
> so the kernel is 2.6.34.
>
> Because they bought your licnece, they asked for your help, but
> you were reluctant on this issue, and the problem remained there
> for about one month.
>
> At last they turned to us for help. We're the kernel department in
> Huawei, but maintaining this product kernel isn't our job. Still
> Li Bin devoted his time to analyzing this bug, and he did a great
> job.
>
> Li Bin told the product team what was wrong and was about to send
> a fix for upstream kernel.
You have time to do that but you didn't.
> They told you our analysis for further
> confirmation,
So you realy didn't make the patch, right? Because you are not
sure the right fix.
> and you were so reluctant to help but so quick to
> send the fix.
We have responsed to you.
>
> Li Bin never reported this bug, but he fixed it. It's a shame that
> you took the credit from us.
I just saw a bug report and ananysis. And I agreed and confirmed it's
a bug.
Thanks,
Yong
>
> On 2014/5/21 13:36, Yong Zhang wrote:
> > asid_cache must be unsigned long otherwise on 64bit system
> > it will become 0 if the value in get_new_mmu_context()
> > reaches 0x and in the end the assumption of
> > ASID_FIRST_VERSION is not true anymore thus leads to
> > more dangerous things.
> >
>
> We should describe what problem this bug can lead to, which
> will help people who encounter the same problem and google it.
>
> > Reported-by: libin
> > Signed-off-by: Yong Zhang
>
> Should mark the patch for stable trees. Though 2.6.34 is EOL,
> the fix should be backported to other kernels.
>
> > ---
> >
> > V2<-V1: Add the reporter.
> >
> > arch/mips/include/asm/cpu-info.h |2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/mips/include/asm/cpu-info.h
> > b/arch/mips/include/asm/cpu-info.h
> > index f6299be..ebcc2ed 100644
> > --- a/arch/mips/include/asm/cpu-info.h
> > +++ b/arch/mips/include/asm/cpu-info.h
> > @@ -40,7 +40,7 @@ struct cache_desc {
> >
> > struct cpuinfo_mips {
> > unsigned intudelay_val;
> > - unsigned intasid_cache;
> > + unsigned long asid_cache;
> >
> > /*
> > * Capability and feature descriptor structure for MIPS CPU
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] MIPS: change type of asid_cache to unsigned long
On Tue, May 27, 2014 at 12:16:30PM +0800, Li Zefan wrote:
I'm not quite happy about what happaned here. There's a story behind
this patch.
One of our Huawei product encountered a bug, and they're using WindRiver4,
so the kernel is 2.6.34.
Because they bought your licnece, they asked for your help, but
you were reluctant on this issue, and the problem remained there
for about one month.
At last they turned to us for help. We're the kernel department in
Huawei, but maintaining this product kernel isn't our job. Still
Li Bin devoted his time to analyzing this bug, and he did a great
job.
Li Bin told the product team what was wrong and was about to send
a fix for upstream kernel.
You have time to do that but you didn't.
They told you our analysis for further
confirmation,
So you realy didn't make the patch, right? Because you are not
sure the right fix.
and you were so reluctant to help but so quick to
send the fix.
We have responsed to you.
Li Bin never reported this bug, but he fixed it. It's a shame that
you took the credit from us.
I just saw a bug report and ananysis. And I agreed and confirmed it's
a bug.
Thanks,
Yong
On 2014/5/21 13:36, Yong Zhang wrote:
asid_cache must be unsigned long otherwise on 64bit system
it will become 0 if the value in get_new_mmu_context()
reaches 0x and in the end the assumption of
ASID_FIRST_VERSION is not true anymore thus leads to
more dangerous things.
We should describe what problem this bug can lead to, which
will help people who encounter the same problem and google it.
Reported-by: libin huawei.li...@huawei.com
Signed-off-by: Yong Zhang yong.zh...@windriver.com
Should mark the patch for stable trees. Though 2.6.34 is EOL,
the fix should be backported to other kernels.
---
V2-V1: Add the reporter.
arch/mips/include/asm/cpu-info.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/mips/include/asm/cpu-info.h
b/arch/mips/include/asm/cpu-info.h
index f6299be..ebcc2ed 100644
--- a/arch/mips/include/asm/cpu-info.h
+++ b/arch/mips/include/asm/cpu-info.h
@@ -40,7 +40,7 @@ struct cache_desc {
struct cpuinfo_mips {
unsigned intudelay_val;
- unsigned intasid_cache;
+ unsigned long asid_cache;
/*
* Capability and feature descriptor structure for MIPS CPU
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] MIPS: change type of asid_cache to unsigned long
BTW, I realy don't care who credits the patch and Ralf said that
he will applied the one which moves the place of udelay_val.
Anyway, if your company pays you more money if you contribute to
the community, just take it and talk about it with Ralf ;-)
Thanks,
Yong
On Tue, May 27, 2014 at 12:16:30PM +0800, Li Zefan wrote:
I'm not quite happy about what happaned here. There's a story behind
this patch.
One of our Huawei product encountered a bug, and they're using WindRiver4,
so the kernel is 2.6.34.
Because they bought your licnece, they asked for your help, but
you were reluctant on this issue, and the problem remained there
for about one month.
At last they turned to us for help. We're the kernel department in
Huawei, but maintaining this product kernel isn't our job. Still
Li Bin devoted his time to analyzing this bug, and he did a great
job.
Li Bin told the product team what was wrong and was about to send
a fix for upstream kernel. They told you our analysis for further
confirmation, and you were so reluctant to help but so quick to
send the fix.
Li Bin never reported this bug, but he fixed it. It's a shame that
you took the credit from us.
On 2014/5/21 13:36, Yong Zhang wrote:
asid_cache must be unsigned long otherwise on 64bit system
it will become 0 if the value in get_new_mmu_context()
reaches 0x and in the end the assumption of
ASID_FIRST_VERSION is not true anymore thus leads to
more dangerous things.
We should describe what problem this bug can lead to, which
will help people who encounter the same problem and google it.
Reported-by: libin huawei.li...@huawei.com
Signed-off-by: Yong Zhang yong.zh...@windriver.com
Should mark the patch for stable trees. Though 2.6.34 is EOL,
the fix should be backported to other kernels.
---
V2-V1: Add the reporter.
arch/mips/include/asm/cpu-info.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/mips/include/asm/cpu-info.h
b/arch/mips/include/asm/cpu-info.h
index f6299be..ebcc2ed 100644
--- a/arch/mips/include/asm/cpu-info.h
+++ b/arch/mips/include/asm/cpu-info.h
@@ -40,7 +40,7 @@ struct cache_desc {
struct cpuinfo_mips {
unsigned intudelay_val;
- unsigned intasid_cache;
+ unsigned long asid_cache;
/*
* Capability and feature descriptor structure for MIPS CPU
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] MIPS: change type of asid_cache to unsigned long
On Tue, May 27, 2014 at 01:07:20PM +0800, Li Zefan wrote: On 2014/5/27 12:50, Yong Zhang wrote: BTW, I realy don't care who credits the patch and Ralf said that he will applied the one which moves the place of udelay_val. Anyway, if your company pays you more money if you contribute to the community, just take it and talk about it with Ralf ;-) We don't do contribution for money, and I don't think you do, but crediting properly is one of the reason that our kernel community keeps prosperous for so many years, and that's one of the reason we introduced Reported-by and Tested-by tags. I'll reply this email for the last time. To me your action is just like Reported-by, but I admit that you also do analysis. If you don't the way change it to whatever you want. Thanks, Yong -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH V2] MIPS: change type of asid_cache to unsigned long
asid_cache must be unsigned long otherwise on 64bit system
it will become 0 if the value in get_new_mmu_context()
reaches 0x and in the end the assumption of
ASID_FIRST_VERSION is not true anymore thus leads to
more dangerous things.
Reported-by: libin
Signed-off-by: Yong Zhang
---
V2<-V1: Add the reporter.
arch/mips/include/asm/cpu-info.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/mips/include/asm/cpu-info.h b/arch/mips/include/asm/cpu-info.h
index f6299be..ebcc2ed 100644
--- a/arch/mips/include/asm/cpu-info.h
+++ b/arch/mips/include/asm/cpu-info.h
@@ -40,7 +40,7 @@ struct cache_desc {
struct cpuinfo_mips {
unsigned intudelay_val;
- unsigned intasid_cache;
+ unsigned long asid_cache;
/*
* Capability and feature descriptor structure for MIPS CPU
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH V2] MIPS: change type of asid_cache to unsigned long
asid_cache must be unsigned long otherwise on 64bit system
it will become 0 if the value in get_new_mmu_context()
reaches 0x and in the end the assumption of
ASID_FIRST_VERSION is not true anymore thus leads to
more dangerous things.
Reported-by: libin huawei.li...@huawei.com
Signed-off-by: Yong Zhang yong.zh...@windriver.com
---
V2-V1: Add the reporter.
arch/mips/include/asm/cpu-info.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/mips/include/asm/cpu-info.h b/arch/mips/include/asm/cpu-info.h
index f6299be..ebcc2ed 100644
--- a/arch/mips/include/asm/cpu-info.h
+++ b/arch/mips/include/asm/cpu-info.h
@@ -40,7 +40,7 @@ struct cache_desc {
struct cpuinfo_mips {
unsigned intudelay_val;
- unsigned intasid_cache;
+ unsigned long asid_cache;
/*
* Capability and feature descriptor structure for MIPS CPU
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] sched: update sched_rt_[period|runtime]_us correctly
From: Yong Zhang The code fails to update tg->rt_bandwidth.[rt_period | rt_runtime] if we have RT_GROUP_SCHED enabled. And if we lower the value of sched_rt_runtime_us, we get error as "write error: Invalid argument". Fix it by calling tg_set_rt_bandwidth() instead when setting sched_rt_[period|runtime]_us. Signed-off-by: Yong Zhang Cc: Ingo Molnar Cc: Peter Zijlstra --- kernel/sched/core.c |6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 5ac63c9..c77d298 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -7035,11 +7035,7 @@ static int sched_rt_global_constraints(void) if (runtime > period && runtime != RUNTIME_INF) return -EINVAL; - mutex_lock(_constraints_mutex); - read_lock(_lock); - ret = __rt_schedulable(NULL, 0, 0); - read_unlock(_lock); - mutex_unlock(_constraints_mutex); + ret = tg_set_rt_bandwidth(_task_group, period, runtime); return ret; } -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] sched:rt: don't try to pull a running task
If we happen to pull a running task on other cpu, a WARNING
will happen. But the very task will be pulled to this cpu
finally. This pulling is dangerous since the task's private
data could be broken. Fix it by skipping this kind of task.
Signed-off-by: Yong Zhang
Cc: Ingo Molnar
Cc: Peter Zijlstra
---
kernel/sched/rt.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 01970c8..e76d278 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1665,7 +1665,15 @@ static int pull_rt_task(struct rq *this_rq)
* the to-be-scheduled task?
*/
if (p && (p->prio < this_rq->rt.highest_prio.curr)) {
- WARN_ON(p == src_rq->curr);
+ /*
+* if we happen to choose a running task on other
+* cpu, skip it and find another one.
+*/
+ if (unlikely(p == src_rq->curr)) {
+ WARN_ON_ONCE(1);
+ goto skip;
+ }
+
WARN_ON(!p->on_rq);
/*
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] sched:rt: don't try to pull a running task
If we happen to pull a running task on other cpu, a WARNING
will happen. But the very task will be pulled to this cpu
finally. This pulling is dangerous since the task's private
data could be broken. Fix it by skipping this kind of task.
Signed-off-by: Yong Zhang yong.zh...@windriver.com
Cc: Ingo Molnar mi...@redhat.com
Cc: Peter Zijlstra pet...@infradead.org
---
kernel/sched/rt.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 01970c8..e76d278 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1665,7 +1665,15 @@ static int pull_rt_task(struct rq *this_rq)
* the to-be-scheduled task?
*/
if (p (p-prio this_rq-rt.highest_prio.curr)) {
- WARN_ON(p == src_rq-curr);
+ /*
+* if we happen to choose a running task on other
+* cpu, skip it and find another one.
+*/
+ if (unlikely(p == src_rq-curr)) {
+ WARN_ON_ONCE(1);
+ goto skip;
+ }
+
WARN_ON(!p-on_rq);
/*
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] sched: update sched_rt_[period|runtime]_us correctly
From: Yong Zhang yong.zh...@windriver.com The code fails to update tg-rt_bandwidth.[rt_period | rt_runtime] if we have RT_GROUP_SCHED enabled. And if we lower the value of sched_rt_runtime_us, we get error as write error: Invalid argument. Fix it by calling tg_set_rt_bandwidth() instead when setting sched_rt_[period|runtime]_us. Signed-off-by: Yong Zhang yong.zha...@gmail.com Cc: Ingo Molnar mi...@redhat.com Cc: Peter Zijlstra pet...@infradead.org --- kernel/sched/core.c |6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 5ac63c9..c77d298 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -7035,11 +7035,7 @@ static int sched_rt_global_constraints(void) if (runtime period runtime != RUNTIME_INF) return -EINVAL; - mutex_lock(rt_constraints_mutex); - read_lock(tasklist_lock); - ret = __rt_schedulable(NULL, 0, 0); - read_unlock(tasklist_lock); - mutex_unlock(rt_constraints_mutex); + ret = tg_set_rt_bandwidth(root_task_group, period, runtime); return ret; } -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] sched: Fix 32bit race in sched_clock_remote()
On Fri, Apr 05, 2013 at 06:36:40PM +0200, Peter Zijlstra wrote:
> Thomas spotted a nasty 32bit race in sched_clock_remote() after way too
> many hours of debugging weirdness.
>
> What happens is that sched_clock_remote() does regular machine word
> reads of sched_clock_data::clock; this appears safe since we use
> cmpxchg64() to update the variable and any half-read value would
> trigger a retry.
>
> Except we don't validate the new value 'val' in the same way! Thus we
> can propagate non-atomic read errors into the clock value.
>
> Cc: Ingo Molnar
> Cc: Steven Rostedt
> Debugged-by: Thomas Gleixner
> Signed-off-by: Peter Zijlstra
> ---
> kernel/sched/clock.c | 19 +--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/clock.c b/kernel/sched/clock.c
> index c685e31..7042ef7 100644
> --- a/kernel/sched/clock.c
> +++ b/kernel/sched/clock.c
> @@ -170,6 +170,21 @@ static u64 sched_clock_local(struct sched_clock_data
> *scd)
> return clock;
> }
>
> +#ifndef CONFIG_64BIT
> +/*
> + * 32bit machines can't atomically read a u64 except using cmpxchg64()
> + */
> +static inline u64 scd_read_clock(struct sched_clock_data *scd)
> +{
> + return cmpxchg64(>clock, 0, 0);
> +}
> +#else
> +static inline u64 scd_read_clock(struct sched_clock_data *scd)
> +{
> + return scd->clock;
> +}
> +#endif
> +
> static u64 sched_clock_remote(struct sched_clock_data *scd)
> {
> struct sched_clock_data *my_scd = this_scd();
> @@ -178,8 +193,8 @@ static u64 sched_clock_remote(struct sched_clock_data
> *scd)
>
> sched_clock_local(my_scd);
> again:
> - this_clock = my_scd->clock;
> - remote_clock = scd->clock;
> + this_clock = scd_clock_read(my_scd);
> + remote_clock = scd_clock_read(scd);
^^
it doesn't match the declaration: scd_read_clock().
Thanks,
Yong
>
> /*
>* Use the opportunity that we have both locks
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] sched: Fix 32bit race in sched_clock_remote()
On Fri, Apr 05, 2013 at 06:36:40PM +0200, Peter Zijlstra wrote:
Thomas spotted a nasty 32bit race in sched_clock_remote() after way too
many hours of debugging weirdness.
What happens is that sched_clock_remote() does regular machine word
reads of sched_clock_data::clock; this appears safe since we use
cmpxchg64() to update the variable and any half-read value would
trigger a retry.
Except we don't validate the new value 'val' in the same way! Thus we
can propagate non-atomic read errors into the clock value.
Cc: Ingo Molnar mi...@kernel.org
Cc: Steven Rostedt rost...@goodmis.org
Debugged-by: Thomas Gleixner t...@linutronix.de
Signed-off-by: Peter Zijlstra a.p.zijls...@chello.nl
---
kernel/sched/clock.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/clock.c b/kernel/sched/clock.c
index c685e31..7042ef7 100644
--- a/kernel/sched/clock.c
+++ b/kernel/sched/clock.c
@@ -170,6 +170,21 @@ static u64 sched_clock_local(struct sched_clock_data
*scd)
return clock;
}
+#ifndef CONFIG_64BIT
+/*
+ * 32bit machines can't atomically read a u64 except using cmpxchg64()
+ */
+static inline u64 scd_read_clock(struct sched_clock_data *scd)
+{
+ return cmpxchg64(scd-clock, 0, 0);
+}
+#else
+static inline u64 scd_read_clock(struct sched_clock_data *scd)
+{
+ return scd-clock;
+}
+#endif
+
static u64 sched_clock_remote(struct sched_clock_data *scd)
{
struct sched_clock_data *my_scd = this_scd();
@@ -178,8 +193,8 @@ static u64 sched_clock_remote(struct sched_clock_data
*scd)
sched_clock_local(my_scd);
again:
- this_clock = my_scd-clock;
- remote_clock = scd-clock;
+ this_clock = scd_clock_read(my_scd);
+ remote_clock = scd_clock_read(scd);
^^
it doesn't match the declaration: scd_read_clock().
Thanks,
Yong
/*
* Use the opportunity that we have both locks
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] futex: fix unbalanced spin_lock/spin_unlock() in exit_pi_state_list()
On Fri, Mar 01, 2013 at 11:17:42AM +0100, Thomas Gleixner wrote:
> On Fri, 1 Mar 2013, Yong Zhang wrote:
>
> > From: Yong Zhang
> >
> > Otherwise, below warning is shown somtimes when running some test:
> >
> > WARNING: at kernel/sched/core.c:3423 migrate_disable+0xbf/0xd0()
> > Hardware name: OptiPlex 755
> > Modules linked in: floppy parport parport_pc minix
> > Pid: 1800, comm: tst-robustpi8 Tainted: GW3.4.28-rt40 #1
> > Call Trace:
> > [] warn_slowpath_common+0x7f/0xc0
> > [] warn_slowpath_null+0x1a/0x20
> > [] migrate_disable+0xbf/0xd0
> > [] exit_pi_state_list+0xa5/0x170
> > [] mm_release+0x12f/0x170
> > [] exit_mm+0x26/0x140
> > [] ? acct_collect+0x186/0x1c0
> > [] do_exit+0x146/0x930
> > [] ? get_parent_ip+0x11/0x50
> > [] do_group_exit+0x4d/0xc0
> > [] get_signal_to_deliver+0x23f/0x6a0
> > [] do_signal+0x65/0x5e0
> > [] ? group_send_sig_info+0x76/0x80
> > [] do_notify_resume+0x98/0xd0
> > [] int_signal+0x12/0x17
> > ---[ end trace 0004 ]---
> >
> > The reason is that spin_lock() is taken in atomic context, but
> > spin_unlock() is not.
>
> This doesn't make sense. The spin_lock() happens in non atomic
> context.
>
> spin_lock(>lock);
> raw_spin_lock_irq(>pi_lock);
>
> The unlock is in atomic context and the unlock does not call
> migrate_disable().
>
> Though on RT this is caused by the in_atomic check of migrate_enable()
> and this results in asymetry. See below.
Yeah, this is what I want to say. s/atomic/no-atomic/ in my words :)
>
> > Signed-off-by: Yong Zhang
> > Cc: Thomas Gleixner
> > Cc: Steven Rostedt
> > ---
> > kernel/futex.c |3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/futex.c b/kernel/futex.c
> > index 9e26e87..2b676a2 100644
> > --- a/kernel/futex.c
> > +++ b/kernel/futex.c
> > @@ -562,16 +562,17 @@ void exit_pi_state_list(struct task_struct *curr)
> >
> > spin_lock(>lock);
> >
> > - raw_spin_lock_irq(>pi_lock);
> > /*
> > * We dropped the pi-lock, so re-check whether this
> > * task still owns the PI-state:
> > */
>
> Did you read and understand this comment ?
>
> The logic here is
>
> raw_spin_lock_irq(>pi_lock);
> next = head->next;
> raw_spin_unlock_irq(>pi_lock);
> spin_lock(>lock);
> raw_spin_lock_irq(>pi_lock);
> if (head->next != next)
>
> We must drop pi_lock before locking the hash bucket lock. That opens a
> window for another task to modify head list. So we must relock pi_lock
> and verify whether head->next is unmodified. If it changed, we need to
> reevaluate.
>
> > if (head->next != next) {
> > spin_unlock(>lock);
> > + raw_spin_lock_irq(>pi_lock);
> > continue;
> > }
> >
> > + raw_spin_lock_irq(>pi_lock);
> > WARN_ON(pi_state->owner != curr);
> > WARN_ON(list_empty(_state->list));
> > list_del_init(_state->list);
>
> So both your patch description and your patch are patently wrong.
> Correct solution below.
Recognized it after sending my V1 out. So V2 is sent out to close the
race window and it happens to be the same patch with yours below.
http://marc.info/?l=linux-rt-users=136211763323758=2
Thanks,
Yong
>
> Thanks,
>
> tglx
> ---
> futex: Ensure lock/unlock symetry versus pi_lock and hash bucket lock
>
> In exit_pi_state_list() we have the following locking construct:
>
>spin_lock(>lock);
>raw_spin_lock_irq(>pi_lock);
>
>...
>spin_unlock(>lock);
>
> In !RT this works, but on RT the migrate_enable() function which is
> called from spin_unlock() sees atomic context due to the held pi_lock
> and just decrements the migrate_disable_atomic counter of the
> task. Now the next call to migrate_disable() sees the counter being
> negative and issues a warning. That check should be in
> migrate_enable() already.
>
> Fix this by dropping pi_lock before unlocking hb->lock and reaquire
> pi_lock after that again. This is safe as the loop code reevaluates
> head again under the pi_lock.
>
> Reported-by: Yong Zhang
> Signed-off-by: Thomas Gleixner
>
> diff --git a/kernel/futex.c b/kernel/futex.c
> index f15f0e4..c795c9c 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -568,7 +568,9
Re: [PATCH] futex: fix unbalanced spin_lock/spin_unlock() in exit_pi_state_list()
On Fri, Mar 01, 2013 at 11:17:42AM +0100, Thomas Gleixner wrote:
On Fri, 1 Mar 2013, Yong Zhang wrote:
From: Yong Zhang yong.zh...@windriver.com
Otherwise, below warning is shown somtimes when running some test:
WARNING: at kernel/sched/core.c:3423 migrate_disable+0xbf/0xd0()
Hardware name: OptiPlex 755
Modules linked in: floppy parport parport_pc minix
Pid: 1800, comm: tst-robustpi8 Tainted: GW3.4.28-rt40 #1
Call Trace:
[81031f3f] warn_slowpath_common+0x7f/0xc0
[81031f9a] warn_slowpath_null+0x1a/0x20
[81066eaf] migrate_disable+0xbf/0xd0
[81085d95] exit_pi_state_list+0xa5/0x170
[8102f71f] mm_release+0x12f/0x170
[81036906] exit_mm+0x26/0x140
[81090fc6] ? acct_collect+0x186/0x1c0
[81036b66] do_exit+0x146/0x930
[810658d1] ? get_parent_ip+0x11/0x50
[8103760d] do_group_exit+0x4d/0xc0
[8104828f] get_signal_to_deliver+0x23f/0x6a0
[810019e5] do_signal+0x65/0x5e0
[81047816] ? group_send_sig_info+0x76/0x80
[81002018] do_notify_resume+0x98/0xd0
[8165779b] int_signal+0x12/0x17
---[ end trace 0004 ]---
The reason is that spin_lock() is taken in atomic context, but
spin_unlock() is not.
This doesn't make sense. The spin_lock() happens in non atomic
context.
spin_lock(hb-lock);
raw_spin_lock_irq(curr-pi_lock);
The unlock is in atomic context and the unlock does not call
migrate_disable().
Though on RT this is caused by the in_atomic check of migrate_enable()
and this results in asymetry. See below.
Yeah, this is what I want to say. s/atomic/no-atomic/ in my words :)
Signed-off-by: Yong Zhang yong.zha...@gmail.com
Cc: Thomas Gleixner t...@linutronix.de
Cc: Steven Rostedt rost...@goodmis.org
---
kernel/futex.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/futex.c b/kernel/futex.c
index 9e26e87..2b676a2 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -562,16 +562,17 @@ void exit_pi_state_list(struct task_struct *curr)
spin_lock(hb-lock);
- raw_spin_lock_irq(curr-pi_lock);
/*
* We dropped the pi-lock, so re-check whether this
* task still owns the PI-state:
*/
Did you read and understand this comment ?
The logic here is
raw_spin_lock_irq(curr-pi_lock);
next = head-next;
raw_spin_unlock_irq(curr-pi_lock);
spin_lock(hb-lock);
raw_spin_lock_irq(curr-pi_lock);
if (head-next != next)
We must drop pi_lock before locking the hash bucket lock. That opens a
window for another task to modify head list. So we must relock pi_lock
and verify whether head-next is unmodified. If it changed, we need to
reevaluate.
if (head-next != next) {
spin_unlock(hb-lock);
+ raw_spin_lock_irq(curr-pi_lock);
continue;
}
+ raw_spin_lock_irq(curr-pi_lock);
WARN_ON(pi_state-owner != curr);
WARN_ON(list_empty(pi_state-list));
list_del_init(pi_state-list);
So both your patch description and your patch are patently wrong.
Correct solution below.
Recognized it after sending my V1 out. So V2 is sent out to close the
race window and it happens to be the same patch with yours below.
http://marc.info/?l=linux-rt-usersm=136211763323758w=2
Thanks,
Yong
Thanks,
tglx
---
futex: Ensure lock/unlock symetry versus pi_lock and hash bucket lock
In exit_pi_state_list() we have the following locking construct:
spin_lock(hb-lock);
raw_spin_lock_irq(curr-pi_lock);
...
spin_unlock(hb-lock);
In !RT this works, but on RT the migrate_enable() function which is
called from spin_unlock() sees atomic context due to the held pi_lock
and just decrements the migrate_disable_atomic counter of the
task. Now the next call to migrate_disable() sees the counter being
negative and issues a warning. That check should be in
migrate_enable() already.
Fix this by dropping pi_lock before unlocking hb-lock and reaquire
pi_lock after that again. This is safe as the loop code reevaluates
head again under the pi_lock.
Reported-by: Yong Zhang yong.zh...@windriver.com
Signed-off-by: Thomas Gleixner t...@linutronix.de
diff --git a/kernel/futex.c b/kernel/futex.c
index f15f0e4..c795c9c 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -568,7 +568,9 @@ void exit_pi_state_list(struct task_struct *curr)
* task still owns the PI-state:
*/
if (head-next != next) {
+ raw_spin_unlock_irq(curr-pi_lock);
spin_unlock(hb-lock);
+ raw_spin_lock_irq(curr-pi_lock);
continue;
}
--
To unsubscribe from
[V2 PATCH -rt] futex: fix unbalanced spin_lock/spin_unlock() in exit_pi_state_list()
From: Yong Zhang
Otherwise, below warning is shown somtimes when running some test:
WARNING: at kernel/sched/core.c:3423 migrate_disable+0xbf/0xd0()
Hardware name: OptiPlex 755
Modules linked in: floppy parport parport_pc minix
Pid: 1800, comm: tst-robustpi8 Tainted: GW3.4.28-rt40 #1
Call Trace:
[] warn_slowpath_common+0x7f/0xc0
[] warn_slowpath_null+0x1a/0x20
[] migrate_disable+0xbf/0xd0
[] exit_pi_state_list+0xa5/0x170
[] mm_release+0x12f/0x170
[] exit_mm+0x26/0x140
[] ? acct_collect+0x186/0x1c0
[] do_exit+0x146/0x930
[] ? get_parent_ip+0x11/0x50
[] do_group_exit+0x4d/0xc0
[] get_signal_to_deliver+0x23f/0x6a0
[] do_signal+0x65/0x5e0
[] ? group_send_sig_info+0x76/0x80
[] do_notify_resume+0x98/0xd0
[] int_signal+0x12/0x17
---[ end trace 0004 ]---
The reason is that spin_lock() is taken in atomic context, but
spin_unlock() is not.
Signed-off-by: Yong Zhang
Cc: Thomas Gleixner
Cc: Steven Rostedt
---
kernel/futex.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/futex.c b/kernel/futex.c
index 9e26e87..daada3d 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -568,7 +568,9 @@ void exit_pi_state_list(struct task_struct *curr)
* task still owns the PI-state:
*/
if (head->next != next) {
+ raw_spin_unlock_irq(>pi_lock);
spin_unlock(>lock);
+ raw_spin_lock_irq(>pi_lock);
continue;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] futex: fix unbalanced spin_lock/spin_unlock() in exit_pi_state_list()
On Fri, Mar 1, 2013 at 9:36 AM, Yong Zhang wrote:
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -562,16 +562,17 @@ void exit_pi_state_list(struct task_struct *curr)
>
> spin_lock(>lock);
>
> - raw_spin_lock_irq(>pi_lock);
> /*
> * We dropped the pi-lock, so re-check whether this
> * task still owns the PI-state:
> */
> if (head->next != next) {
Just ignore this patch, race window is opened here.
New patch comes soon.
Thanks,
Yong
> spin_unlock(>lock);
> + raw_spin_lock_irq(>pi_lock);
> continue;
> }
>
> + raw_spin_lock_irq(>pi_lock);
> WARN_ON(pi_state->owner != curr);
> WARN_ON(list_empty(_state->list));
> list_del_init(_state->list);
> --
> 1.7.9.5
>
--
Only stand for myself
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] futex: fix unbalanced spin_lock/spin_unlock() in exit_pi_state_list()
From: Yong Zhang
Otherwise, below warning is shown somtimes when running some test:
WARNING: at kernel/sched/core.c:3423 migrate_disable+0xbf/0xd0()
Hardware name: OptiPlex 755
Modules linked in: floppy parport parport_pc minix
Pid: 1800, comm: tst-robustpi8 Tainted: GW3.4.28-rt40 #1
Call Trace:
[] warn_slowpath_common+0x7f/0xc0
[] warn_slowpath_null+0x1a/0x20
[] migrate_disable+0xbf/0xd0
[] exit_pi_state_list+0xa5/0x170
[] mm_release+0x12f/0x170
[] exit_mm+0x26/0x140
[] ? acct_collect+0x186/0x1c0
[] do_exit+0x146/0x930
[] ? get_parent_ip+0x11/0x50
[] do_group_exit+0x4d/0xc0
[] get_signal_to_deliver+0x23f/0x6a0
[] do_signal+0x65/0x5e0
[] ? group_send_sig_info+0x76/0x80
[] do_notify_resume+0x98/0xd0
[] int_signal+0x12/0x17
---[ end trace 0004 ]---
The reason is that spin_lock() is taken in atomic context, but
spin_unlock() is not.
Signed-off-by: Yong Zhang
Cc: Thomas Gleixner
Cc: Steven Rostedt
---
kernel/futex.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/futex.c b/kernel/futex.c
index 9e26e87..2b676a2 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -562,16 +562,17 @@ void exit_pi_state_list(struct task_struct *curr)
spin_lock(>lock);
- raw_spin_lock_irq(>pi_lock);
/*
* We dropped the pi-lock, so re-check whether this
* task still owns the PI-state:
*/
if (head->next != next) {
spin_unlock(>lock);
+ raw_spin_lock_irq(>pi_lock);
continue;
}
+ raw_spin_lock_irq(>pi_lock);
WARN_ON(pi_state->owner != curr);
WARN_ON(list_empty(_state->list));
list_del_init(_state->list);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] futex: fix unbalanced spin_lock/spin_unlock() in exit_pi_state_list()
From: Yong Zhang yong.zh...@windriver.com
Otherwise, below warning is shown somtimes when running some test:
WARNING: at kernel/sched/core.c:3423 migrate_disable+0xbf/0xd0()
Hardware name: OptiPlex 755
Modules linked in: floppy parport parport_pc minix
Pid: 1800, comm: tst-robustpi8 Tainted: GW3.4.28-rt40 #1
Call Trace:
[81031f3f] warn_slowpath_common+0x7f/0xc0
[81031f9a] warn_slowpath_null+0x1a/0x20
[81066eaf] migrate_disable+0xbf/0xd0
[81085d95] exit_pi_state_list+0xa5/0x170
[8102f71f] mm_release+0x12f/0x170
[81036906] exit_mm+0x26/0x140
[81090fc6] ? acct_collect+0x186/0x1c0
[81036b66] do_exit+0x146/0x930
[810658d1] ? get_parent_ip+0x11/0x50
[8103760d] do_group_exit+0x4d/0xc0
[8104828f] get_signal_to_deliver+0x23f/0x6a0
[810019e5] do_signal+0x65/0x5e0
[81047816] ? group_send_sig_info+0x76/0x80
[81002018] do_notify_resume+0x98/0xd0
[8165779b] int_signal+0x12/0x17
---[ end trace 0004 ]---
The reason is that spin_lock() is taken in atomic context, but
spin_unlock() is not.
Signed-off-by: Yong Zhang yong.zha...@gmail.com
Cc: Thomas Gleixner t...@linutronix.de
Cc: Steven Rostedt rost...@goodmis.org
---
kernel/futex.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/futex.c b/kernel/futex.c
index 9e26e87..2b676a2 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -562,16 +562,17 @@ void exit_pi_state_list(struct task_struct *curr)
spin_lock(hb-lock);
- raw_spin_lock_irq(curr-pi_lock);
/*
* We dropped the pi-lock, so re-check whether this
* task still owns the PI-state:
*/
if (head-next != next) {
spin_unlock(hb-lock);
+ raw_spin_lock_irq(curr-pi_lock);
continue;
}
+ raw_spin_lock_irq(curr-pi_lock);
WARN_ON(pi_state-owner != curr);
WARN_ON(list_empty(pi_state-list));
list_del_init(pi_state-list);
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] futex: fix unbalanced spin_lock/spin_unlock() in exit_pi_state_list()
On Fri, Mar 1, 2013 at 9:36 AM, Yong Zhang yong.zha...@gmail.com wrote:
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -562,16 +562,17 @@ void exit_pi_state_list(struct task_struct *curr)
spin_lock(hb-lock);
- raw_spin_lock_irq(curr-pi_lock);
/*
* We dropped the pi-lock, so re-check whether this
* task still owns the PI-state:
*/
if (head-next != next) {
Just ignore this patch, race window is opened here.
New patch comes soon.
Thanks,
Yong
spin_unlock(hb-lock);
+ raw_spin_lock_irq(curr-pi_lock);
continue;
}
+ raw_spin_lock_irq(curr-pi_lock);
WARN_ON(pi_state-owner != curr);
WARN_ON(list_empty(pi_state-list));
list_del_init(pi_state-list);
--
1.7.9.5
--
Only stand for myself
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[V2 PATCH -rt] futex: fix unbalanced spin_lock/spin_unlock() in exit_pi_state_list()
From: Yong Zhang yong.zh...@windriver.com
Otherwise, below warning is shown somtimes when running some test:
WARNING: at kernel/sched/core.c:3423 migrate_disable+0xbf/0xd0()
Hardware name: OptiPlex 755
Modules linked in: floppy parport parport_pc minix
Pid: 1800, comm: tst-robustpi8 Tainted: GW3.4.28-rt40 #1
Call Trace:
[81031f3f] warn_slowpath_common+0x7f/0xc0
[81031f9a] warn_slowpath_null+0x1a/0x20
[81066eaf] migrate_disable+0xbf/0xd0
[81085d95] exit_pi_state_list+0xa5/0x170
[8102f71f] mm_release+0x12f/0x170
[81036906] exit_mm+0x26/0x140
[81090fc6] ? acct_collect+0x186/0x1c0
[81036b66] do_exit+0x146/0x930
[810658d1] ? get_parent_ip+0x11/0x50
[8103760d] do_group_exit+0x4d/0xc0
[8104828f] get_signal_to_deliver+0x23f/0x6a0
[810019e5] do_signal+0x65/0x5e0
[81047816] ? group_send_sig_info+0x76/0x80
[81002018] do_notify_resume+0x98/0xd0
[8165779b] int_signal+0x12/0x17
---[ end trace 0004 ]---
The reason is that spin_lock() is taken in atomic context, but
spin_unlock() is not.
Signed-off-by: Yong Zhang yong.zha...@gmail.com
Cc: Thomas Gleixner t...@linutronix.de
Cc: Steven Rostedt rost...@goodmis.org
---
kernel/futex.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/futex.c b/kernel/futex.c
index 9e26e87..daada3d 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -568,7 +568,9 @@ void exit_pi_state_list(struct task_struct *curr)
* task still owns the PI-state:
*/
if (head-next != next) {
+ raw_spin_unlock_irq(curr-pi_lock);
spin_unlock(hb-lock);
+ raw_spin_lock_irq(curr-pi_lock);
continue;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[tip:core/locking] lockdep: Selftest: convert spinlock to raw spinlock
Commit-ID: 9fb1b90ce0a847a8cc9492a6c1f347b5be1f33ff
Gitweb: http://git.kernel.org/tip/9fb1b90ce0a847a8cc9492a6c1f347b5be1f33ff
Author: Yong Zhang
AuthorDate: Mon, 16 Apr 2012 15:01:55 +0800
Committer: Ingo Molnar
CommitDate: Tue, 19 Feb 2013 08:43:35 +0100
lockdep: Selftest: convert spinlock to raw spinlock
To make the lockdep selftest working on RT we need to convert the
spinlock tests to a raw spinlock. Otherwise we cannot run the irq
context checks. For mainline this is just annotational as spinlocks
are mapped to raw_spinlocks anyway.
Signed-off-by: Yong Zhang
Link:
http://lkml.kernel.org/r/1334559716-18447-2-git-send-email-yong.zha...@gmail.com
Signed-off-by: Thomas Gleixner
---
lib/locking-selftest.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/lib/locking-selftest.c b/lib/locking-selftest.c
index 7aae0f2..c3eb261 100644
--- a/lib/locking-selftest.c
+++ b/lib/locking-selftest.c
@@ -47,10 +47,10 @@ __setup("debug_locks_verbose=", setup_debug_locks_verbose);
* Normal standalone locks, for the circular and irq-context
* dependency tests:
*/
-static DEFINE_SPINLOCK(lock_A);
-static DEFINE_SPINLOCK(lock_B);
-static DEFINE_SPINLOCK(lock_C);
-static DEFINE_SPINLOCK(lock_D);
+static DEFINE_RAW_SPINLOCK(lock_A);
+static DEFINE_RAW_SPINLOCK(lock_B);
+static DEFINE_RAW_SPINLOCK(lock_C);
+static DEFINE_RAW_SPINLOCK(lock_D);
static DEFINE_RWLOCK(rwlock_A);
static DEFINE_RWLOCK(rwlock_B);
@@ -73,12 +73,12 @@ static DECLARE_RWSEM(rwsem_D);
* but X* and Y* are different classes. We do this so that
* we do not trigger a real lockup:
*/
-static DEFINE_SPINLOCK(lock_X1);
-static DEFINE_SPINLOCK(lock_X2);
-static DEFINE_SPINLOCK(lock_Y1);
-static DEFINE_SPINLOCK(lock_Y2);
-static DEFINE_SPINLOCK(lock_Z1);
-static DEFINE_SPINLOCK(lock_Z2);
+static DEFINE_RAW_SPINLOCK(lock_X1);
+static DEFINE_RAW_SPINLOCK(lock_X2);
+static DEFINE_RAW_SPINLOCK(lock_Y1);
+static DEFINE_RAW_SPINLOCK(lock_Y2);
+static DEFINE_RAW_SPINLOCK(lock_Z1);
+static DEFINE_RAW_SPINLOCK(lock_Z2);
static DEFINE_RWLOCK(rwlock_X1);
static DEFINE_RWLOCK(rwlock_X2);
@@ -107,10 +107,10 @@ static DECLARE_RWSEM(rwsem_Z2);
*/
#define INIT_CLASS_FUNC(class) \
static noinline void \
-init_class_##class(spinlock_t *lock, rwlock_t *rwlock, struct mutex *mutex, \
-struct rw_semaphore *rwsem)\
+init_class_##class(raw_spinlock_t *lock, rwlock_t *rwlock, \
+ struct mutex *mutex, struct rw_semaphore *rwsem)\
{ \
- spin_lock_init(lock); \
+ raw_spin_lock_init(lock); \
rwlock_init(rwlock);\
mutex_init(mutex); \
init_rwsem(rwsem); \
@@ -168,10 +168,10 @@ static void init_shared_classes(void)
* Shortcuts for lock/unlock API variants, to keep
* the testcases compact:
*/
-#define L(x) spin_lock(_##x)
-#define U(x) spin_unlock(_##x)
+#define L(x) raw_spin_lock(_##x)
+#define U(x) raw_spin_unlock(_##x)
#define LU(x) L(x); U(x)
-#define SI(x) spin_lock_init(_##x)
+#define SI(x) raw_spin_lock_init(_##x)
#define WL(x) write_lock(_##x)
#define WU(x) write_unlock(_##x)
@@ -911,7 +911,7 @@ GENERATE_PERMUTATIONS_3_EVENTS(irq_read_recursion_soft)
#define I2(x) \
do {\
- spin_lock_init(_##x); \
+ raw_spin_lock_init(_##x); \
rwlock_init(_##x); \
mutex_init(_##x); \
init_rwsem(_##x); \
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[tip:core/locking] lockdep: Selftest: convert spinlock to raw spinlock
Commit-ID: 9fb1b90ce0a847a8cc9492a6c1f347b5be1f33ff
Gitweb: http://git.kernel.org/tip/9fb1b90ce0a847a8cc9492a6c1f347b5be1f33ff
Author: Yong Zhang yong.zha...@gmail.com
AuthorDate: Mon, 16 Apr 2012 15:01:55 +0800
Committer: Ingo Molnar mi...@kernel.org
CommitDate: Tue, 19 Feb 2013 08:43:35 +0100
lockdep: Selftest: convert spinlock to raw spinlock
To make the lockdep selftest working on RT we need to convert the
spinlock tests to a raw spinlock. Otherwise we cannot run the irq
context checks. For mainline this is just annotational as spinlocks
are mapped to raw_spinlocks anyway.
Signed-off-by: Yong Zhang yong.zha...@gmail.com
Link:
http://lkml.kernel.org/r/1334559716-18447-2-git-send-email-yong.zha...@gmail.com
Signed-off-by: Thomas Gleixner t...@linutronix.de
---
lib/locking-selftest.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/lib/locking-selftest.c b/lib/locking-selftest.c
index 7aae0f2..c3eb261 100644
--- a/lib/locking-selftest.c
+++ b/lib/locking-selftest.c
@@ -47,10 +47,10 @@ __setup(debug_locks_verbose=, setup_debug_locks_verbose);
* Normal standalone locks, for the circular and irq-context
* dependency tests:
*/
-static DEFINE_SPINLOCK(lock_A);
-static DEFINE_SPINLOCK(lock_B);
-static DEFINE_SPINLOCK(lock_C);
-static DEFINE_SPINLOCK(lock_D);
+static DEFINE_RAW_SPINLOCK(lock_A);
+static DEFINE_RAW_SPINLOCK(lock_B);
+static DEFINE_RAW_SPINLOCK(lock_C);
+static DEFINE_RAW_SPINLOCK(lock_D);
static DEFINE_RWLOCK(rwlock_A);
static DEFINE_RWLOCK(rwlock_B);
@@ -73,12 +73,12 @@ static DECLARE_RWSEM(rwsem_D);
* but X* and Y* are different classes. We do this so that
* we do not trigger a real lockup:
*/
-static DEFINE_SPINLOCK(lock_X1);
-static DEFINE_SPINLOCK(lock_X2);
-static DEFINE_SPINLOCK(lock_Y1);
-static DEFINE_SPINLOCK(lock_Y2);
-static DEFINE_SPINLOCK(lock_Z1);
-static DEFINE_SPINLOCK(lock_Z2);
+static DEFINE_RAW_SPINLOCK(lock_X1);
+static DEFINE_RAW_SPINLOCK(lock_X2);
+static DEFINE_RAW_SPINLOCK(lock_Y1);
+static DEFINE_RAW_SPINLOCK(lock_Y2);
+static DEFINE_RAW_SPINLOCK(lock_Z1);
+static DEFINE_RAW_SPINLOCK(lock_Z2);
static DEFINE_RWLOCK(rwlock_X1);
static DEFINE_RWLOCK(rwlock_X2);
@@ -107,10 +107,10 @@ static DECLARE_RWSEM(rwsem_Z2);
*/
#define INIT_CLASS_FUNC(class) \
static noinline void \
-init_class_##class(spinlock_t *lock, rwlock_t *rwlock, struct mutex *mutex, \
-struct rw_semaphore *rwsem)\
+init_class_##class(raw_spinlock_t *lock, rwlock_t *rwlock, \
+ struct mutex *mutex, struct rw_semaphore *rwsem)\
{ \
- spin_lock_init(lock); \
+ raw_spin_lock_init(lock); \
rwlock_init(rwlock);\
mutex_init(mutex); \
init_rwsem(rwsem); \
@@ -168,10 +168,10 @@ static void init_shared_classes(void)
* Shortcuts for lock/unlock API variants, to keep
* the testcases compact:
*/
-#define L(x) spin_lock(lock_##x)
-#define U(x) spin_unlock(lock_##x)
+#define L(x) raw_spin_lock(lock_##x)
+#define U(x) raw_spin_unlock(lock_##x)
#define LU(x) L(x); U(x)
-#define SI(x) spin_lock_init(lock_##x)
+#define SI(x) raw_spin_lock_init(lock_##x)
#define WL(x) write_lock(rwlock_##x)
#define WU(x) write_unlock(rwlock_##x)
@@ -911,7 +911,7 @@ GENERATE_PERMUTATIONS_3_EVENTS(irq_read_recursion_soft)
#define I2(x) \
do {\
- spin_lock_init(lock_##x); \
+ raw_spin_lock_init(lock_##x); \
rwlock_init(rwlock_##x); \
mutex_init(mutex_##x); \
init_rwsem(rwsem_##x); \
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] MIPS: fix access_ok()
From: Yong Zhang Current access_ok() will fail even if the address range is valid when it reaches to the end of TASK_SIZE. For exampe: addr = 0xf0; size = 16; the real address range it want to access is 0xf0~0xf; but addr + size = 0x100 which we will not and can't access. In current realization of access_ok(), the high bit will be 1 thus access_ok() indicates the operation is not allowed. The bug is found in old kerenl(before vdso is realized) in following typical call trace: sys_mount() copy_mount_options() exact_copy_from_user() When the parameter 'from' for exact_copy_from_user() residents in the last page of the task's virtual address, such as stack. But it's still in current kernel. Signed-off-by: Yong Zhang Cc: Ralf Baechle Cc: David Daney --- arch/mips/include/asm/uaccess.h |8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/arch/mips/include/asm/uaccess.h b/arch/mips/include/asm/uaccess.h index 3b92efe..55d4214 100644 --- a/arch/mips/include/asm/uaccess.h +++ b/arch/mips/include/asm/uaccess.h @@ -114,8 +114,12 @@ extern u64 __ua_limit; unsigned long __ok; \ \ __chk_user_ptr(addr); \ - __ok = (signed long)(__mask & (__addr | (__addr + __size) | \ - __ua_size(__size)));\ + if (likely(size)) \ + __ok = (signed long)(__mask & (__addr | \ + (__addr + __size - 1) | \ + __ua_size(__size)));\ + else\ + __ok = 0; \ __ok == 0; \ }) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] MIPS: fix access_ok()
From: Yong Zhang yong.zh...@windriver.com Current access_ok() will fail even if the address range is valid when it reaches to the end of TASK_SIZE. For exampe: addr = 0xf0; size = 16; the real address range it want to access is 0xf0~0xf; but addr + size = 0x100 which we will not and can't access. In current realization of access_ok(), the high bit will be 1 thus access_ok() indicates the operation is not allowed. The bug is found in old kerenl(before vdso is realized) in following typical call trace: sys_mount() copy_mount_options() exact_copy_from_user() When the parameter 'from' for exact_copy_from_user() residents in the last page of the task's virtual address, such as stack. But it's still in current kernel. Signed-off-by: Yong Zhang yong.zha...@gmail.com Cc: Ralf Baechle r...@linux-mips.org Cc: David Daney david.da...@cavium.com --- arch/mips/include/asm/uaccess.h |8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/arch/mips/include/asm/uaccess.h b/arch/mips/include/asm/uaccess.h index 3b92efe..55d4214 100644 --- a/arch/mips/include/asm/uaccess.h +++ b/arch/mips/include/asm/uaccess.h @@ -114,8 +114,12 @@ extern u64 __ua_limit; unsigned long __ok; \ \ __chk_user_ptr(addr); \ - __ok = (signed long)(__mask (__addr | (__addr + __size) | \ - __ua_size(__size)));\ + if (likely(size)) \ + __ok = (signed long)(__mask (__addr | \ + (__addr + __size - 1) | \ + __ua_size(__size)));\ + else\ + __ok = 0; \ __ok == 0; \ }) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
