Messages in this thread | | | Date | Tue, 23 Dec 2003 02:10:39 -0800 | From | Paul Jackson <> | Subject | Re: [PATCH] another minor bit of cpumask cleanup |
| |
Rusty writes: > I have a patch which changes these iterators [for_each_cpu] anyway,
Cool.
> I'd rather have 12 crossing patches than silence
yup
> For the hotplug CPU stuff,
Makes sense to me that the hotplug CPU work would be in the drivers seat on these cpu looping macros. I like your patch. Since your more substantial patch negates my trivial patch to remove the old for_each_online_cpu(), I'll forget about my patch.
Andrew:
You lucked out in dropping my patch to remove for_each_online_cpu(). Good.
Speaking of trivial patches, didn't you (Rusty) used to be the Trivial Patch Monkey, and what has become of that esteemed role in 2.6 land?
I do have a more substantial patch that is yet widely published to provide an alternative underlying implementation of the cpumask macros with something that can be used for both cpu and node masks, that takes one file to express instead of 5 or 6, and that has one base type (struct of array of unsigned longs) rather than a choice of three or so implementations.
For at least one architecture, sparc64 (IIRC), wli informs me that davem is quite certain this alternative can't be used (resulting machine code way too painful). But I am hopeful that we can make it cleaner source code and just as good machine code, at least for the architectures that can use recent gcc optimizing.
The basic goal of this patch-to-be will be to provide a solid 'mask' type that can be used within the kernel for various bit masks of known (compiled in) lengths of one to quite a few words. This work stems from a suggestion by Christoph Hellwig that I implement numamask_t using an ADT shared with cpumask_t. I originally told Christoph I would have that ADT patch up and published in a week. That was two months ago ;)
Unfortunately, I've just started on vacation for the next month or so, so won't likely get much further publishing this right now. If someone has possibly conflicting work, or would like to join in, or otherwise raise issues, please speak up.
I have the patch in a healthy state and being used on work that I am doing to support the 'cpuset' feature being developed by Simon and Sylvain of Bull (France).
There are some performance tests that Ray Bryant has graciously volunteered to attempt, which will be critical to my final recommendation of this alternative for at least the ia64 arch, which tests will not happen until Feb, when I can set Ray up to make the comparisons of this new implementation with the current one. We need to know whether for the > 256 bit case, seemingly replacing a call by reference (pointer to mask) with a call by value will actually matter to performance of the code we care about.
> possible cpu ... online cpus
I'm not quite sure of the meaning to you of these terms. Is it that possible cpus are the union of online and offline cpus?
> noone uses them that way (except for arch/i386/mach-voyager, which > D: uses for_each_cpu(cpu_online_mask)
What about the one remaining usage of for_each_cpu(), also in voyager, but not using cpu_online_mask:
arch/i386/mach-voyager/voyager_smp.c:
============================================================= #ifdef VOYAGER_DEBUG ... if((isr & (1<<irq) && !(status & IRQ_REPLAY)) == 0) { ... int mask;
printk("VOYAGER SMP: CPU%d lost interrupt %d\n", cpu, irq); for_each_cpu(real_cpu, mask) { =============================================================
You noted that 'mask' needed initializing in a comment in your patch, but I don't see that you change the calling signature of for_each_cpu(), not that it is clear to what it should be changed ;(.
> so the iterators are moved > D: from linux/cpumask.h to linux/smp.h, where that is asm/smp.h is included.
This comment says the iterators are moved to smp.h, but the patch seems to still show them in cpumask.h. I suspect that I prefer them in smp.h better.
> D: Followup patches will convert users.
Looks to me like this here patch is converting some users, such as in fork.c and sched.c. Is this the conversion you speak of, or is there more to come in a followup?
Good work, Rusty. Thanks.
-- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <pj@sgi.com> 1.650.933.1373 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |