On Thu, Aug 23, 2007 at 07:52:11PM +0530, Gautham R Shenoy wrote:

On Thu, Aug 23, 2007 at 06:15:01AM -0700, Paul E. McKenney wrote:

quoted

On Thu, Aug 23, 2007 at 03:44:44PM +0530, Gautham R Shenoy wrote:

quoted

On Thu, Aug 23, 2007 at 01:54:56AM -0700, Paul E. McKenney wrote:

quoted

On Thu, Aug 23, 2007 at 09:56:39AM +0530, Gautham R Shenoy wrote:

quoted

I feel we should still be able to use for_each_online_cpu(cpu) instead
of for_each_possible_cpu. Again, there's a good chance that I might
be mistaken!

How about the following ?

	preempt_disable(); /* We Dont want cpus going down here */
	for_each_online_cpu(cpu) 
		for (i = 0; i < RCU_BOOST_ELEMENTS; i++) {
			rbdp = per_cpu(rcu_boost_dat, cpu);
			sum.rbs_blocked += rbdp[i].rbs_blocked;
			sum.rbs_boost_attempt += rbdp[i].rbs_boost_attempt;
			sum.rbs_boost += rbdp[i].rbs_boost;
			sum.rbs_unlock += rbdp[i].rbs_unlock;
			sum.rbs_unboosted += rbdp[i].rbs_unboosted;
		}
	preempt_enable(); 


	static int rcu_boost_cpu_callback(struct notifier_bloack *nb, 
					unsigned long action, void *hcpu) 
	{
		int this_cpu, cpu;
		rcu_boost_data *rbdp, *this_rbdp;

		switch (action) {
		case CPU_DEAD:
			this_cpu = get_cpu();
			cpu = (long)hcpu;
			this_cpu = smp_processor_id();
			rbdp = per_cpu(rcu_boost_dat, cpu);
			this_rbdp = per_cpu(rcu_boost_dat, cpu);
			/* 
			 *  Transfer all of rbdp's statistics to
			 *  this_rbdp here.
			 */	
			 put_cpu();
	
			return NOTIFY_OK;
		}
	}


Won't this work in this case?

Hello, Gautham,

We could do something similar.  If there was a global rcu_boost_data
variable that held the sums of the fields of the rcu_boost_data
structures for all offline CPUs, and if we used a new lock to protect
that global rcu_boost data variable (both when reading and when
CPU hotplugging), then we could indeed scan only the online CPUs'
rcu_boost_data elements.

We would also have to maintain a cpumask_t for this purpose, and
we would need to add a CPU's contribution when it went offline and
subtract it when that CPU came back online.

The additional cpumask_t beats me though! Doesn't the cpu_online_map
suffice here? 
The addition and subtraction of a hotplugged cpu's
contribution from the global rcu_boost_data could be done while
handling the CPU_ONLINE and CPU_DEAD (or CPU_UP_PREPARE
and CPU_DOWN_PREPARE, whichever suits better), in the cpu hotplug
callback. 

Am I missing something ?

Don't we need to synchronize the manipulation of the hotplugged CPU's
contribution and the manipulation of cpu_online_map?  Otherwise, if
stats are called for just before (or just after, depending on the
ordering of hotplug operations) the invocation will get the wrong
statistics.

Oh, yes we need to synchronize that :-)

Can't we use lock_cpu_hotplug/unlock_cpu_hotplug (or it's variant when 
it is available) around any access to cpu_online_map ? With that, it's
guaranteed that no cpu-hotplug operation will be permitted while you're
iterating over the cpu_online_map, and hence you have a  
consistent view of global rcu_boost_data.

Even if we use another cpumask_t, whenever a cpu goes down or comes up,
that will be reflected in this map, no? So what's the additional
advantage of using it?

The additional map allows the code to use something other than the
lock_cpu_hotplug/unlock_cpu_hotplug, and also is robust against any
changes to the hotplug synchronization mechanism.  Might well be
better just to use the current hotplug synchronization mechanism,
but I was feeling paranoid.  ;-)

						Thanx, Paul

quoted

quoted

quoted

The lock should not be a problem even on very large systems because
of the low frequency of statistics printing -- and of hotplug operations,
for that matter.

The lock is not a problem, so long as somebody else doesn't call
the function taking the lock from their cpu-hotplug callback path :-)
Though I don't see it happening here.

There are some ways to decrease its utilization if it should become
a problem in any case.

Cool!

quoted

						Thanx, Paul

Thanks and Regards
gautham.
-- 
Gautham R Shenoy
Linux Technology Center
IBM India.
"Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!"