Messages in this thread Patch in this message | | | Date | Fri, 26 Mar 2004 14:36:48 -0800 | From | Paul Jackson <> | Subject | Sparc64, cpumask_t and struct arguments (was: [PATCH] Introduce nodemask_t ADT) |
| |
David,
I understand that you are the Sparc64 expert. I am doing some more work refining the cpumask_t (and nodemask_t) implementation, in an effort to obtain a common "mask_t" underlying ADT that can be useful for both cpu and node masks.
As part of this, I'm looking at having only one data representation for these masks - an array of unsigned longs encapsulated inside a struct. For the processors and compilation environment that I am most focused on - gcc 3.2 and above on Intel CPUs, this generates just as good code as the other variants.
However, as I recall, you recommend against passing structs on the Sparc64 processor and compilation environment. Am I recalling correctly?
The two places that I see sparc64 passing a cpumask_t across a real function call boundary (as opposed to an inline function) is in the file arch/sparc64/kernel/smp.c, by the functions cheetah_xcall_deliver() and smp_cross_call_masked().
Could you describe to me the requirements here.
1) Are the above the two places of interest, currently, for sparc64 work?
2) What sizes of cpumask_t are needed?
3) Which of passing by pointer, passing as a struct, and/or passing by simple value, are required or acceptable or unacceptable?
4) Does your answer to (3) vary with the sizes needed in (2)?
5) The cpu_clear(i, mask) on about line 534 of smp.c confuses me, as it seems to be changing a local copy of 'mask' that no one will examine later. What purpose does it serve? See this line annotated with "No affect??" in the changes, below.
6) I'm not done yet, but so far I have the following changes in my workarea for sparc64 (along with larger changes elsewhere). These changes are untested, unbuilt, incomplete and unreviewed. They were generated on a 2.6.4 base, with Matthew Dobson's nodemask_t patch of this thread applied. Do you see anything that looks wrong in the following so far?
===== smp.c 1.66 vs edited ===== --- 1.66/arch/sparc64/kernel/smp.c Tue Feb 24 16:11:02 2004 +++ edited/smp.c Fri Mar 26 14:01:23 2004 @@ -409,14 +409,8 @@ int i;
__asm__ __volatile__("rdpr %%pstate, %0" : "=r" (pstate)); - for (i = 0; i < NR_CPUS; i++) { - if (cpu_isset(i, mask)) { - spitfire_xcall_helper(data0, data1, data2, pstate, i); - cpu_clear(i, mask); - if (cpus_empty(mask)) - break; - } - } + for_each_cpu_mask(i, mask) + spitfire_xcall_helper(data0, data1, data2, pstate, i); }
/* Cheetah now allows to send the whole 64-bytes of data in the interrupt @@ -459,25 +453,19 @@
nack_busy_id = 0; { - cpumask_t work_mask = mask; int i;
- for (i = 0; i < NR_CPUS; i++) { - if (cpu_isset(i, work_mask)) { - u64 target = (i << 14) | 0x70; - - if (!is_jalapeno) - target |= (nack_busy_id << 24); - __asm__ __volatile__( - "stxa %%g0, [%0] %1\n\t" - "membar #Sync\n\t" - : /* no outputs */ - : "r" (target), "i" (ASI_INTR_W)); - nack_busy_id++; - cpu_clear(i, work_mask); - if (cpus_empty(work_mask)) - break; - } + for_each_cpu_mask(i, mask) { + u64 target = (i << 14) | 0x70; + + if (!is_jalapeno) + target |= (nack_busy_id << 24); + __asm__ __volatile__( + "stxa %%g0, [%0] %1\n\t" + "membar #Sync\n\t" + : /* no outputs */ + : "r" (target), "i" (ASI_INTR_W)); + nack_busy_id++; } }
@@ -521,6 +509,8 @@ /* Clear out the mask bits for cpus which did not * NACK us. */ + /* Could replace with for_each_cpu_mask(i, mask) loop, + * were it not for the cpu_clear(i, mask) below. -pj */ for (i = 0; i < NR_CPUS; i++) { if (cpu_isset(i, work_mask)) { u64 check_mask; @@ -531,7 +521,7 @@ check_mask = (0x2UL << this_busy_nack); if ((dispatch_stat & check_mask) == 0) - cpu_clear(i, mask); + cpu_clear(i, mask); /* No affect?? -pj */ this_busy_nack += 2; cpu_clear(i, work_mask); if (cpus_empty(work_mask))
Thank-you for your considerations.
-- 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/
| |