Messages in this thread | | | Date | Thu, 14 Jun 2012 14:01:50 +0530 | From | "Srivatsa S. Bhat" <> | Subject | Re: [RFC patch 2/5] smpboot: Provide infrastructure for percpu hotplug threads |
| |
On 06/13/2012 04:30 PM, Thomas Gleixner wrote:
> Provide a generic interface for setting up and tearing down percpu > threads. > > On registration the threads for already online cpus are created and > started. On deregistration (modules) the threads are stoppped. > > During hotplug operations the threads are created, started, parked and > unparked. The datastructure for registration provides a pointer to > percpu storage space and optional setup, cleanup, park, unpark > functions. These functions are called when the thread state changes. > > Thread functions should look like the following: > > int thread_fn(void *cookie) > { > while (!smpboot_thread_check_parking(cookie)) { > do_stuff(); > } > return 0; > } >
This patchset looks great! But I have some comments below:
> Index: tip/kernel/cpu.c > =================================================================== > --- tip.orig/kernel/cpu.c > +++ tip/kernel/cpu.c > @@ -280,6 +280,7 @@ static int __ref _cpu_down(unsigned int > __func__, cpu); > goto out_release; > } > + smpboot_park_threads(cpu); >
If cpu_down fails further down, don't we want to unpark these threads as part of error recovery?
> err = __stop_machine(take_cpu_down, &tcd_param, cpumask_of(cpu)); > if (err) { > @@ -354,6 +355,10 @@ static int __cpuinit _cpu_up(unsigned in > goto out; > } > > + ret = smpboot_create_threads(cpu); > + if (ret) > + goto out; > +
Here also, we might want to clean up on error right?
> ret = __cpu_notify(CPU_UP_PREPARE | mod, hcpu, -1, &nr_calls); > if (ret) { > nr_calls--; > @@ -370,6 +375,7 @@ static int __cpuinit _cpu_up(unsigned in > > /* Now call notifier in preparation. */ > cpu_notify(CPU_ONLINE | mod, hcpu); > + smpboot_unpark_threads(cpu); > > out_notify: > if (ret != 0)
Regards, Srivatsa S. Bhat
| |