|
|
| Next: Cross compiler ITP (armel) |
| Author |
Message |
Jiri Slaby External

Since: Nov 04, 2006 Posts: 645
|
Posted: Sun Nov 01, 2009 6:10 pm Post subject: [PATCH 1/1] MM: slqb, fix per_cpu access Archived from groups: linux>kernel (more info?) |
|
|
We cannot use the same local variable name as the declared per_cpu
variable since commit "percpu: remove per_cpu__ prefix."
Otherwise we would see crashes like:
general protection fault: 0000 [#1] SMP
last sysfs file:
CPU 1
Modules linked in:
Pid: 1, comm: swapper Tainted: G W 2.6.32-rc5-mm1_64 #860
RIP: 0010:[<ffffffff8142ff94>] [<ffffffff8142ff94>] start_cpu_timer+0x2b/0x87
....
Signed-off-by: Jiri Slaby <jirislaby.TakeThisOut@gmail.com>
Cc: Nick Piggin <npiggin.TakeThisOut@suse.de>
Cc: Tejun Heo <tj.TakeThisOut@kernel.org>
Cc: Rusty Russell <rusty.TakeThisOut@rustcorp.com.au>
Cc: Christoph Lameter <cl.TakeThisOut@linux-foundation.org>
---
mm/slqb.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/mm/slqb.c b/mm/slqb.c
index e745d9a..27f5025 100644
--- a/mm/slqb.c
+++ b/mm/slqb.c
@@ -2770,16 +2770,16 @@ static DEFINE_PER_CPU(struct delayed_work, cache_trim_work);
static void __cpuinit start_cpu_timer(int cpu)
{
- struct delayed_work *cache_trim_work = &per_cpu(cache_trim_work, cpu);
+ struct delayed_work *_cache_trim_work = &per_cpu(cache_trim_work, cpu);
/*
* When this gets called from do_initcalls via cpucache_init(),
* init_workqueues() has already run, so keventd will be setup
* at that time.
*/
- if (keventd_up() && cache_trim_work->work.func == NULL) {
- INIT_DELAYED_WORK(cache_trim_work, cache_trim_worker);
- schedule_delayed_work_on(cpu, cache_trim_work,
+ if (keventd_up() && _cache_trim_work->work.func == NULL) {
+ INIT_DELAYED_WORK(_cache_trim_work, cache_trim_worker);
+ schedule_delayed_work_on(cpu, _cache_trim_work,
__round_jiffies_relative(HZ, cpu));
}
}
--
1.6.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/ |
|
| Back to top |
|
 |
Tejun Heo External

Since: Jan 27, 2009 Posts: 62
|
Posted: Mon Nov 02, 2009 12:10 am Post subject: Re: [PATCH 1/1] MM: slqb, fix per_cpu access [Login to view extended thread Info.] Archived from groups: per prev. post (more info?) |
|
|
Hello,
Jiri Slaby wrote:
> @@ -2770,16 +2770,16 @@ static DEFINE_PER_CPU(struct delayed_work, cache_trim_work);
How about renaming cache_trim_work to slqb_cache_trim_work? Another
percpu name requirement is global uniqueness (for s390 and alpha
support), so prefixing perpcu variables with subsystem name usually
resolves situations like this better.
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo DeleteThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/ |
|
| Back to top |
|
 |
Jiri Slaby External

Since: Nov 04, 2006 Posts: 645
|
Posted: Mon Nov 02, 2009 4:10 am Post subject: [PATCH 1/1] MM: slqb, fix per_cpu access [Login to view extended thread Info.] Archived from groups: per prev. post (more info?) |
|
|
We cannot use the same local variable name as the declared per_cpu
variable since commit "percpu: remove per_cpu__ prefix."
Otherwise we would see crashes like:
general protection fault: 0000 [#1] SMP
last sysfs file:
CPU 1
Modules linked in:
Pid: 1, comm: swapper Tainted: G W 2.6.32-rc5-mm1_64 #860
RIP: 0010:[<ffffffff8142ff94>] [<ffffffff8142ff94>] start_cpu_timer+0x2b/0x87
....
Use slqb_ prefix for the global variable so that we don't collide
even with the rest of the kernel (s390 and alpha need this).
Signed-off-by: Jiri Slaby <jirislaby RemoveThis @gmail.com>
Cc: Nick Piggin <npiggin RemoveThis @suse.de>
Cc: Tejun Heo <tj RemoveThis @kernel.org>
Cc: Rusty Russell <rusty RemoveThis @rustcorp.com.au>
Cc: Christoph Lameter <cl RemoveThis @linux-foundation.org>
---
mm/slqb.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/mm/slqb.c b/mm/slqb.c
index e745d9a..e4bb53f 100644
--- a/mm/slqb.c
+++ b/mm/slqb.c
@@ -2766,11 +2766,12 @@ out:
schedule_delayed_work(work, round_jiffies_relative(3*HZ));
}
-static DEFINE_PER_CPU(struct delayed_work, cache_trim_work);
+static DEFINE_PER_CPU(struct delayed_work, slqb_cache_trim_work);
static void __cpuinit start_cpu_timer(int cpu)
{
- struct delayed_work *cache_trim_work = &per_cpu(cache_trim_work, cpu);
+ struct delayed_work *cache_trim_work = &per_cpu(slqb_cache_trim_work,
+ cpu);
/*
* When this gets called from do_initcalls via cpucache_init(),
@@ -3136,8 +3137,9 @@ static int __cpuinit slab_cpuup_callback(struct notifier_block *nfb,
case CPU_DOWN_PREPARE:
case CPU_DOWN_PREPARE_FROZEN:
- cancel_rearming_delayed_work(&per_cpu(cache_trim_work, cpu));
- per_cpu(cache_trim_work, cpu).work.func = NULL;
+ cancel_rearming_delayed_work(&per_cpu(slqb_cache_trim_work,
+ cpu));
+ per_cpu(slqb_cache_trim_work, cpu).work.func = NULL;
break;
case CPU_UP_CANCELED:
--
1.6.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo RemoveThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/ |
|
| Back to top |
|
 |
Dave Young External

Since: Jun 14, 2007 Posts: 28
|
Posted: Mon Nov 02, 2009 7:10 am Post subject: Re: [PATCH 1/1] MM: slqb, fix per_cpu access [Login to view extended thread Info.] Archived from groups: per prev. post (more info?) |
|
|
On Mon, Nov 2, 2009 at 4:49 PM, Jiri Slaby <jirislaby.RemoveThis@gmail.com> wrote:
> We cannot use the same local variable name as the declared per_cpu
> variable since commit "percpu: remove per_cpu__ prefix."
>
> Otherwise we would see crashes like:
> general protection fault: 0000 [#1] SMP
> last sysfs file:
> CPU 1
> Modules linked in:
> Pid: 1, comm: swapper Tainted: G W 2.6.32-rc5-mm1_64 #860
> RIP: 0010:[<ffffffff8142ff94>] [<ffffffff8142ff94>] start_cpu_timer+0x2b/0x87
> ...
>
> Use slqb_ prefix for the global variable so that we don't collide
> even with the rest of the kernel (s390 and alpha need this).
>
> Signed-off-by: Jiri Slaby <jirislaby.RemoveThis@gmail.com>
> Cc: Nick Piggin <npiggin.RemoveThis@suse.de>
> Cc: Tejun Heo <tj.RemoveThis@kernel.org>
> Cc: Rusty Russell <rusty.RemoveThis@rustcorp.com.au>
> Cc: Christoph Lameter <cl.RemoveThis@linux-foundation.org>
Tested-by: Dave Young <hidave.darkstar.RemoveThis@gmail.com>
> ---
> mm/slqb.c | 10 ++++++----
> 1 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/mm/slqb.c b/mm/slqb.c
> index e745d9a..e4bb53f 100644
> --- a/mm/slqb.c
> +++ b/mm/slqb.c
> @@ -2766,11 +2766,12 @@ out:
> schedule_delayed_work(work, round_jiffies_relative(3*HZ));
> }
>
> -static DEFINE_PER_CPU(struct delayed_work, cache_trim_work);
> +static DEFINE_PER_CPU(struct delayed_work, slqb_cache_trim_work);
>
> static void __cpuinit start_cpu_timer(int cpu)
> {
> - struct delayed_work *cache_trim_work = &per_cpu(cache_trim_work, cpu);
> + struct delayed_work *cache_trim_work = &per_cpu(slqb_cache_trim_work,
> + cpu);
>
> /*
> * When this gets called from do_initcalls via cpucache_init(),
> @@ -3136,8 +3137,9 @@ static int __cpuinit slab_cpuup_callback(struct notifier_block *nfb,
>
> case CPU_DOWN_PREPARE:
> case CPU_DOWN_PREPARE_FROZEN:
> - cancel_rearming_delayed_work(&per_cpu(cache_trim_work, cpu));
> - per_cpu(cache_trim_work, cpu).work.func = NULL;
> + cancel_rearming_delayed_work(&per_cpu(slqb_cache_trim_work,
> + cpu));
> + per_cpu(slqb_cache_trim_work, cpu).work.func = NULL;
> break;
>
> case CPU_UP_CANCELED:
> --
> 1.6.4.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo.RemoveThis@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Regards
dave
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.RemoveThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/ |
|
| Back to top |
|
 |
Rusty Russell External

Since: May 24, 2006 Posts: 528
|
Posted: Mon Nov 02, 2009 9:10 am Post subject: Re: [PATCH 1/1] MM: slqb, fix per_cpu access [Login to view extended thread Info.] Archived from groups: per prev. post (more info?) |
|
|
On Mon, 2 Nov 2009 08:42:58 am Jiri Slaby wrote:
> We cannot use the same local variable name as the declared per_cpu
> variable since commit "percpu: remove per_cpu__ prefix."
>
> Otherwise we would see crashes like:
> general protection fault: 0000 [#1] SMP
> last sysfs file:
> CPU 1
> Modules linked in:
> Pid: 1, comm: swapper Tainted: G W 2.6.32-rc5-mm1_64 #860
> RIP: 0010:[<ffffffff8142ff94>] [<ffffffff8142ff94>] start_cpu_timer+0x2b/0x87
> ...
>
> Signed-off-by: Jiri Slaby <jirislaby DeleteThis @gmail.com>
> Cc: Nick Piggin <npiggin DeleteThis @suse.de>
> Cc: Tejun Heo <tj DeleteThis @kernel.org>
> Cc: Rusty Russell <rusty DeleteThis @rustcorp.com.au>
> Cc: Christoph Lameter <cl DeleteThis @linux-foundation.org>
> ---
> mm/slqb.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/mm/slqb.c b/mm/slqb.c
> index e745d9a..27f5025 100644
> --- a/mm/slqb.c
> +++ b/mm/slqb.c
> @@ -2770,16 +2770,16 @@ static DEFINE_PER_CPU(struct delayed_work, cache_trim_work);
>
> static void __cpuinit start_cpu_timer(int cpu)
> {
> - struct delayed_work *cache_trim_work = &per_cpu(cache_trim_work, cpu);
> + struct delayed_work *_cache_trim_work = &per_cpu(cache_trim_work, cpu);
>
> /*
> * When this gets called from do_initcalls via cpucache_init(),
> * init_workqueues() has already run, so keventd will be setup
> * at that time.
> */
> - if (keventd_up() && cache_trim_work->work.func == NULL) {
> - INIT_DELAYED_WORK(cache_trim_work, cache_trim_worker);
> - schedule_delayed_work_on(cpu, cache_trim_work,
> + if (keventd_up() && _cache_trim_work->work.func == NULL) {
> + INIT_DELAYED_WORK(_cache_trim_work, cache_trim_worker);
> + schedule_delayed_work_on(cpu, _cache_trim_work,
> __round_jiffies_relative(HZ, cpu));
How about calling the local var "trim"?
This actually makes the code more readable, IMHO.
Thanks,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo DeleteThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/ |
|
| Back to top |
|
 |
Tejun Heo External

Since: Jan 27, 2009 Posts: 62
|
Posted: Mon Nov 02, 2009 11:10 am Post subject: Re: [PATCH 1/1] MM: slqb, fix per_cpu access [Login to view extended thread Info.] Archived from groups: per prev. post (more info?) |
|
|
2009년 11월 02일 17:49, Jiri Slaby wrote:
> We cannot use the same local variable name as the declared per_cpu
> variable since commit "percpu: remove per_cpu__ prefix."
>
> Otherwise we would see crashes like:
> general protection fault: 0000 [#1] SMP
> last sysfs file:
> CPU 1
> Modules linked in:
> Pid: 1, comm: swapper Tainted: G W 2.6.32-rc5-mm1_64 #860
> RIP: 0010:[<ffffffff8142ff94>] [<ffffffff8142ff94>] start_cpu_timer+0x2b/0x87
> ...
>
> Use slqb_ prefix for the global variable so that we don't collide
> even with the rest of the kernel (s390 and alpha need this).
>
> Signed-off-by: Jiri Slaby <jirislaby.TakeThisOut@gmail.com>
> Cc: Nick Piggin <npiggin.TakeThisOut@suse.de>
> Cc: Tejun Heo <tj.TakeThisOut@kernel.org>
> Cc: Rusty Russell <rusty.TakeThisOut@rustcorp.com.au>
> Cc: Christoph Lameter <cl.TakeThisOut@linux-foundation.org>
Acked-by: Tejun Heo <tj.TakeThisOut@kernel.org>
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/ |
|
| Back to top |
|
 |
Jiri Slaby External

Since: Nov 04, 2006 Posts: 645
|
Posted: Mon Nov 02, 2009 11:10 am Post subject: Re: [PATCH 1/1] MM: slqb, fix per_cpu access [Login to view extended thread Info.] Archived from groups: per prev. post (more info?) |
|
|
On 11/02/2009 02:23 PM, Rusty Russell wrote:
>> --- a/mm/slqb.c
>> +++ b/mm/slqb.c
>> @@ -2770,16 +2770,16 @@ static DEFINE_PER_CPU(struct delayed_work, cache_trim_work);
>>
>> static void __cpuinit start_cpu_timer(int cpu)
>> {
>> - struct delayed_work *cache_trim_work = &per_cpu(cache_trim_work, cpu);
>> + struct delayed_work *_cache_trim_work = &per_cpu(cache_trim_work, cpu);
>>
>> /*
>> * When this gets called from do_initcalls via cpucache_init(),
>> * init_workqueues() has already run, so keventd will be setup
>> * at that time.
>> */
>> - if (keventd_up() && cache_trim_work->work.func == NULL) {
>> - INIT_DELAYED_WORK(cache_trim_work, cache_trim_worker);
>> - schedule_delayed_work_on(cpu, cache_trim_work,
>> + if (keventd_up() && _cache_trim_work->work.func == NULL) {
>> + INIT_DELAYED_WORK(_cache_trim_work, cache_trim_worker);
>> + schedule_delayed_work_on(cpu, _cache_trim_work,
>> __round_jiffies_relative(HZ, cpu));
>
> How about calling the local var "trim"?
>
> This actually makes the code more readable, IMHO.
Please ignore this version of the patch. After this I sent a new one
which changes the global var name.
So the local variable is untouched there. If you want me to perform the
cleanup, let me know. In any case I'd make it trim_work instead of trim
which makes more sense to me.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo DeleteThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/ |
|
| Back to top |
|
 |
Christoph Lameter External

Since: Mar 03, 2009 Posts: 61
|
Posted: Mon Nov 02, 2009 12:10 pm Post subject: Re: [PATCH 1/1] MM: slqb, fix per_cpu access [Login to view extended thread Info.] Archived from groups: per prev. post (more info?) |
|
|
Reviewed-by: Christoph Lameter <cl DeleteThis @linux-foundation.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo DeleteThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/ |
|
| Back to top |
|
 |
Rusty Russell External

Since: May 24, 2006 Posts: 528
|
Posted: Wed Nov 04, 2009 3:10 am Post subject: Re: [PATCH 1/1] MM: slqb, fix per_cpu access [Login to view extended thread Info.] Archived from groups: per prev. post (more info?) |
|
|
On Tue, 3 Nov 2009 02:01:41 am Jiri Slaby wrote:
> >> - struct delayed_work *cache_trim_work = &per_cpu(cache_trim_work, cpu);
> >> + struct delayed_work *_cache_trim_work = &per_cpu(cache_trim_work, cpu);
> >>
> >> /*
> >> * When this gets called from do_initcalls via cpucache_init(),
> >> * init_workqueues() has already run, so keventd will be setup
> >> * at that time.
> >> */
> >> - if (keventd_up() && cache_trim_work->work.func == NULL) {
> >> - INIT_DELAYED_WORK(cache_trim_work, cache_trim_worker);
> >> - schedule_delayed_work_on(cpu, cache_trim_work,
> >> + if (keventd_up() && _cache_trim_work->work.func == NULL) {
> >> + INIT_DELAYED_WORK(_cache_trim_work, cache_trim_worker);
> >> + schedule_delayed_work_on(cpu, _cache_trim_work,
> >> __round_jiffies_relative(HZ, cpu));
> >
> > How about calling the local var "trim"?
> >
> > This actually makes the code more readable, IMHO.
>
> Please ignore this version of the patch. After this I sent a new one
> which changes the global var name.
OK, sure. It's not worth changing unless you were doing a rename anyway.
> So the local variable is untouched there. If you want me to perform the
> cleanup, let me know. In any case I'd make it trim_work instead of trim
> which makes more sense to me.
This is getting pedantic and marginal, but the word "work" already appears
everywhere this var is used. Either "XXX->work", or "INIT_DELAYED_WORK(XXX"
or "scheduled_delayed_work_on(cpu, XXX".
That's why I think the word "work" in unnecessary.
Hope that clarifies why I preferred "trim".
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/ |
|
| Back to top |
|
 |
|
|
|
You can post new topics in this forum You can reply to topics in this forum You cannot edit your posts in this forum You cannot delete your posts in this forum You cannot vote in polls in this forum
|
| |
|
|