Re: BUG_ON(!cpus_equal(cpumask, tmp));

Randy.Dunlap
2004-04-02T18:37:20Z

On Thu, 01 Apr 2004 12:38:40 +0100 Andy Whitcroft wrote:

| --On 01 April 2004 10:34 +0530 Srivatsa Vaddagiri <vatsa@in.ibm.com> wrote:
|
| > Hmm ..Doesn't it need to drop tlbstate_lock before returning?
| > The second lock should be call_lock?
|
| Yes and Yes. I don't know how Andrew copes with 300 odd patches.
| I don't seem to be able to keep track of the versions on 3 of them?
| Seems I sent out an old version. Doh. Explicit version numbers
| from now on.
|
| Below is tested version of the patch. If anyone can reproduce the
| issue I would be interested in knowing if this passes a reboot on
| that system.
|
| Apologies for the confusion. And thanks for reviewing!

This version works well, thank you. Without it I still see the
BUG_ON() in smp.c (line 359).

I noted a few comments corrections and style changes below.
Want a patch for them instead?

| @@ -367,16 +365,24 @@ static void flush_tlb_others(cpumask_t c
| * detected by the NMI watchdog.
| */
| spin_lock(&tlbstate_lock);
| +
| + /* Subtle, mask the request mask with the currently online cpu's.
| + * Sample this under the lock; cpus in the the middle of going
x.x
| + * offline will wait until there is noone in this critical section
| + * before disabling IPI handling. */
| + cpus_and(tmp, cpumask, cpu_online_map);
| + if(cpus_empty(tmp))
if (cpus_empty(tmp))
| + goto out_unlock;

| @@ -527,6 +531,15 @@ int smp_call_function (void (*func) (voi
| atomic_set(&data.finished, 0);
|
| spin_lock(&call_lock);
| +
| + /* Subtle, get the current number of online cpus.
| + * Sample this under the lock; cpus in the the middle of going
x.x
| + * offline will wait until there is noone in this critical section
| + * before disabling IPI handling. */

| @@ -551,6 +565,20 @@ static void stop_this_cpu (void * dummy)
| * Remove this CPU:
| */
| cpu_clear(smp_processor_id(), cpu_online_map);
| +
| + /* Subtle, IPI users assume that they will be able to get IPI's
| + * though to the cpus listed in cpu_online_map. To ensure this
through
| + * we add the requirement that they check cpu_online_map within
| + * the IPI critical sections. Here we remove ourselves from the
| + * map, then ensure that all other cpus have left the relevant
| + * critical sections since the change. We do this by aquiring
acquiring
| + * the relevant section locks, if we have them none else is in
noone
| + * them. Once this is done we can go offline. */

--
~Randy
(Again. Sometimes I think ln -s /usr/src/linux/.config .signature)