Re: [PATCH 1/4] cpuhog: implement cpuhog

[Tejun Heo <tj@kernel.org>]

Hello, Oleg.

On 03/09/2010 04:01 AM, Oleg Nesterov wrote:
> On 03/09, Tejun Heo wrote:
>> +struct cpuhog_done {
>> +	atomic_t		nr_todo;	/* nr left to execute */
>> +	bool			executed;	/* actually executed? */
>> +	int			ret;		/* collected return value */
>> +	struct completion	completion;	/* fired if nr_todo reaches 0 */
>> +};
>> +
>> +static void cpuhog_signal_done(struct cpuhog_done *done, bool executed)
>> +{
>> +	if (done) {
>> +		if (executed)
>> +			done->executed = true;
>> +		if (atomic_dec_and_test(&done->nr_todo))
>> +			complete(&done->completion);
>> +	}
>> +}
> 
> So, ->executed becomes T if at least one cpuhog_thread() thread calls ->fn(),
> 
>> +int __hog_cpus(const struct cpumask *cpumask, cpuhog_fn_t fn, void *arg)
>> +{
>> ...
>> +
>> +	wait_for_completion(&done.completion);
>> +	return done.executed ? done.ret : -ENOENT;
>> +}
> 
> Is this really right?
> 
> I mean, perhaps it makes more sense if ->executed was set only if _all_
> CPUs from cpumask "ack" this call?
> 
> I guess, currently this doesn't matter, stop_machine() uses cpu_online_mask
> and we can't race with hotplug.

cpuhog itself doesn't care whether the cpus go on or offline and
ensuring cpus stay online is the caller's responsibility.  The return
value check is mostly added for hog_one_cpu() users which is much more
light weight than hog_cpus() and thus is much more likely to be used
w/o disabling cpu hotplugging.  For hog_cpus(), I wanted it not to
care about cpu onliness itself.  Requests will be executed for online
ones while ignored for others, so the only exceptional one is the case
where none gets executed.

Thanks.

-- 
tejun