On 01/22, Srivatsa S. Bhat wrote:
>
> Wait a min, that _will_ actually work for all cases because I have provided
> an option to invoke _any_ arbitrary function as the "setup" routine.
And probably the generic solution makes sense. I am not sure I actually
understand the semantics of register_allcpu_notifier(), but the problem
it tries to solve looks clear/valid.
But as for a quick fix for raid5_alloc_percpu(), can't it simply call
register_cpu_notifier() before get_online_cpus/for_each_present_cpu ?
This probably means that raid456_cpu_notify() should be modified because
it obviously can be called before get_online_cpus(). Hmm, it already
has safe_put_page(), so this looks really simple? Something like below,
although of course I can miss easily something.
Oleg.
--- x/drivers/md/raid5.c
+++ x/drivers/md/raid5.c
@@ -5542,6 +5542,24 @@ static void free_conf(struct r5conf *con
kfree(conf);
}
+static int alloc_xxx(struct r5conf *conf, struct raid5_percpu *percpu)
+{
+ if (conf->level == 6 && !percpu->spare_page)
+ percpu->spare_page = alloc_page(GFP_KERNEL);
+ if (!percpu->scribble)
+ percpu->scribble = kmalloc(conf->scribble_len, GFP_KERNEL);
+
+ if (!percpu->scribble || (conf->level == 6 && !percpu->spare_page)) {
+ safe_put_page(percpu->spare_page);
+ kfree(percpu->scribble);
+ pr_err("%s: failed memory allocation for cpu%ld\n",
+ __func__, cpu);
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
#ifdef CONFIG_HOTPLUG_CPU
static int raid456_cpu_notify(struct notifier_block *nfb, unsigned long action,
void *hcpu)
@@ -5553,19 +5571,8 @@ static int raid456_cpu_notify(struct not
switch (action) {
case CPU_UP_PREPARE:
case CPU_UP_PREPARE_FROZEN:
- if (conf->level == 6 && !percpu->spare_page)
- percpu->spare_page = alloc_page(GFP_KERNEL);
- if (!percpu->scribble)
- percpu->scribble = kmalloc(conf->scribble_len,
GFP_KERNEL);
-
- if (!percpu->scribble ||
- (conf->level == 6 && !percpu->spare_page)) {
- safe_put_page(percpu->spare_page);
- kfree(percpu->scribble);
- pr_err("%s: failed memory allocation for cpu%ld\n",
- __func__, cpu);
+ if (alloc_xxx(conf, percpu))
return notifier_from_errno(-ENOMEM);
- }
break;
case CPU_DEAD:
case CPU_DEAD_FROZEN:
@@ -5585,39 +5592,27 @@ static int raid5_alloc_percpu(struct r5c
{
unsigned long cpu;
struct page *spare_page;
- struct raid5_percpu __percpu *allcpus;
void *scribble;
- int err;
+ int err = 0;
- allcpus = alloc_percpu(struct raid5_percpu);
- if (!allcpus)
+ conf->percpu = alloc_percpu(struct raid5_percpu);
+ if (!conf->percpu)
return -ENOMEM;
- conf->percpu = allcpus;
- get_online_cpus();
- err = 0;
- for_each_present_cpu(cpu) {
- if (conf->level == 6) {
- spare_page = alloc_page(GFP_KERNEL);
- if (!spare_page) {
- err = -ENOMEM;
- break;
- }
- per_cpu_ptr(conf->percpu, cpu)->spare_page = spare_page;
- }
- scribble = kmalloc(conf->scribble_len, GFP_KERNEL);
- if (!scribble) {
- err = -ENOMEM;
- break;
- }
- per_cpu_ptr(conf->percpu, cpu)->scribble = scribble;
- }
#ifdef CONFIG_HOTPLUG_CPU
conf->cpu_notify.notifier_call = raid456_cpu_notify;
conf->cpu_notify.priority = 0;
- if (err == 0)
- err = register_cpu_notifier(&conf->cpu_notify);
+ err = register_cpu_notifier(&conf->cpu_notify);
+ if (err)
+ return err;
#endif
+
+ get_online_cpus();
+ for_each_present_cpu(cpu) {
+ err = alloc_xxx(conf, per_cpu_ptr(conf->percpu, cpu));
+ if (err)
+ break;
+ }
put_online_cpus();
return err;
--
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/
