[PATCH] ppc64 cpu hotplug

jschopp at austin.ibm.com jschopp at austin.ibm.com
Thu Mar 25 04:59:54 EST 2004


I was aiming for the simple fix.  Comments on your more serious rewrite
below.

On Wed, 24 Mar 2004, Nathan Lynch wrote:

> Hi-
>
> > @@ -371,6 +371,7 @@
> >  			wait_time = rtas_extended_busy_delay_time(status);
> >  			set_current_state(TASK_UNINTERRUPTIBLE);
> >  			schedule_timeout(wait_time);
> > +			break;
> >  		    }
> >  		    default:	/* shouldn't happen */
> >  			return -ENOMSG;
>
> While I think this is technically correct, I don't understand why
> check_dr_state() is there at all.  There is a similar rtas_get_sensor()
> function which basically does the same thing, except that it uses udelay
> instead of schedule_timeout.  Why don't we use that?

check_dr_state has a built it 5 second timeout so it doesn't keep retrying
forever.  rtas_get_sensor will keep trying forever.  I am not opposed to
using rtas_get_sensor (though we should change it to check 9900..9905
instead of 9900..9909 for extended delay).

> patch; however, I do not understand why find_physical_cpu_to_start is
> potentially polling every possible drc-index.  If a cpu is present in
> the device tree, it belongs to the partition and it is configured
> (except when we've *just* released a cpu to the hypervisor - we
> currently rely on userspace to remove the node from the device tree -
> yuck).  Seems to me that we should simply query each logical cpu for
> each cpu device node.  To be safe, we could check the dr state of each
> cpu node using its ibm,my-drc-index property.

Sure, that would work.

>
> I am including a patch which implements my suggestions and adapts the
> second part of Joel's patch to it:
>
> o  use rtas_get_sensor, ditch check_dr_state
> o  poll a drc-index only when it is attached to a known cpu
> o  handle SMT cpus properly by using the "ibm,ppc-interrupt-server#s"
> property when available instead of the "reg" property
>
> I have tested this on a 2-way Power4 lpar, but I am not able to test the
> SMT case at this time.  Patch is against a current-ish Ameslab tree
> (2.6.5-rc2).
>
> Nathan
>

+               while (nr_threads--) {
+                       if (0 == query_cpu_stopped(tid[nr_threads])) {
+                               best = tid[nr_threads];
+                               if (best == old_hwindex)
                                        goto out;

This part is a little hard to read.  I actually had to think about
nr_threads being 1 for the while and 0 for the indexing tid.  If you clean
this bit up to be more readable I will love the patch.


** Sent via the linuxppc64-dev mail list. See http://lists.linuxppc.org/





More information about the Linuxppc64-dev mailing list