Re: Re: [PATCH 4/4] trace_workqueue: Add worklet information

From: Frederic Weisbecker
Date: Tue Apr 21 2009 - 19:39:52 EST


On Tue, Apr 21, 2009 at 09:57:02AM +0800, Zhaolei wrote:
> Hello, Frederic
>
> Thanks for your review and advice.
>
> >
> > The above test will take the total number of cpus as
> > the last cpu number. This assumption may be false
> > if the possible cpus are not contiguous.
> >
> > Perhaps you'd better use:
> >
> >
> > for_each_possible_cpu(cpu) {
> > ret = workqueue_stat_start_cpu(cpu);
> > if (ret)
> > break;
> > }
> IMHO, this code is to choose first cpu who have workqueue.
> But we need to choose next cpu, may be we need to:
>
> int nextcpu;
> for_each_possible_cpu(nextcpu) {
> /* bypass prev cpus */
> if (nextcpu <= cpu)
> continue;
>
> ret = workqueue_stat_start_cpu(cpu);
> if (ret) {
> break;
> }
> }
>
> By looking cpumask.h, for_each_possible_cpu(nextcpu) always make nextcpu
> increase, but if not(in future?), above code is wrong.
>
> Thanks
> Zhaolei
>


Ok.

BTW, I've applied your patches in
git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git tracing/workqueue

Once I have some time, I will also address Oleg's comments.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/