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

["Randy.Dunlap" <rddunlap@osdl.org>]

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)