[patch[ Simple Topology API

Here is a very rudimentary topology API for NUMA systems. It uses prctl() for the userland calls, and exposes some useful things to userland. It would be nice to expose these simple structures to both users and the kernel itself. Any architecture wishing to use this API simply has to write a .h file that defines the 5 calls defined in core_ibmnumaq.h and include it in asm/mmzone.h.

Matt,

I suspect what happens when these patches come out is that most people simply don't have the knowledge/time/experience/context to judge them, and nothing ends up happening. No way would I pretend to be able to comment on the big picture, that's for sure.

If the code is clean, the interfaces make sense, the impact on other platforms is minimised and the stakeholders are OK with it then that should be sufficient, yes?

AFAIK, the interested parties with this and the memory binding API are ia32-NUMA, ia64, PPC, some MIPS and x86-64-soon. It would be helpful if the owners of those platforms could review this work and say "yes, this is something we can use and build upon". Have they done that?

I'd have a few micro-observations:

kernel/membind.c Fri Jul 12 16:13:17 2002

Quoted 5 lines

...

kernel/membind.c Fri Jul 12 16:13:17 2002 renamed from kernel/membind.c Wed Dec 31 16:00:00 1969 +1 -0

..
+inline int memblk_to_node(int memblk)

The inlines with global scope in this file seem strange?

Quoted 3 lines · Matthew Dobson

Here is a Memory Binding API
...
+ memblk_binding: { MEMBLK_NO_BINDING, MPOL_STRICT }, \

kernel/membind.c Fri Jul 12 16:13:17 2002

Quoted 6 lines

...

kernel/membind.c Fri Jul 12 16:13:17 2002 renamed from kernel/membind.c Wed Dec 31 16:00:00 1969 +5 -0

+typedef struct memblk_list {
+       memblk_bitmask_t bitmask;
+       int behavior;
+       rwlock_t lock;
+} memblk_list_t;

Is is possible to reduce this type to something smaller for CONFIG_NUMA=n?

In the above task_struct initialiser you should initialise the rwlock to RWLOCK_LOCK_UNLOCKED.

It's nice to use the `name:value' initialiser format in there, too.

kernel/membind.c Fri Jul 12 16:13:17 2002

Quoted 5 lines

...

kernel/membind.c Fri Jul 12 16:13:17 2002 renamed from kernel/membind.c Wed Dec 31 16:00:00 1969 +3 -0

+int set_memblk_binding(memblk_bitmask_t memblks, int behavior)
+{
...
+       read_lock_irqsave(&current->memblk_binding.lock, flags);

Your code accesses current' a lot. You'll find that the code generation is fairly poor - evaluating current' chews 10-15 bytes of code. You can perform a manual CSE by copying current into a local, and save a few cycles.

kernel/membind.c Fri Jul 12 16:13:17 2002

Quoted 7 lines

...

kernel/membind.c Fri Jul 12 16:13:17 2002 renamed from kernel/membind.c Wed Dec 31 16:00:00 1969 +5 -0

+struct page * _alloc_pages(unsigned int gfp_mask, unsigned int order)
+{
...
+       spin_lock_irqsave(&node_lock, flags);
+       temp = pgdat_list;
+       spin_unlock_irqrestore(&node_lock, flags);

Not sure what you're trying to lock here, but you're not locking it ;) This is either racy code or unneeded locking.

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

Here is a very rudimentary topology API for NUMA systems. It uses prctl() for the userland calls, and exposes some useful things to userland. It would be nice to expose these simple structures to both users and the kernel itself. Any architecture wishing to use this API simply has to write a .h file that defines the 5 calls defined in core_ibmnumaq.h and include it in asm/mmzone.h.
Voila! Instant inclusion in the topology!

Enjoy!

It's hard to enjoy the use of prctl(). Especially for things like "give me the number of the first CPU in node <n>" - it ain't no process controll, no matter how you stretch it.

<soapbox> That's yet another demonstration of the evil of multiplexing syscalls. They hide the broken APIs and make them easy to introduce. And broken APIs get introduced - through each of these. prctl(), fcntl(), ioctl() - you name it. Please, don't do that. </soapbox>

Quoted 4 lines · Matthew Dobson

Here is a very rudimentary topology API for NUMA systems. It uses prctl() for the userland calls, and exposes some useful things to userland. It would be nice to expose these simple structures to both users and the kernel itself. Any architecture wishing to use this API simply has to write a .h file that defines the 5 calls defined in core_ibmnumaq.h and include it in asm/mmzone.h.
Voila! Instant inclusion in the topology!

Enjoy!

It's hard to enjoy the use of prctl(). Especially for things like "give me the number of the first CPU in node <n>" - it ain't no process controll, no matter how you stretch it.

<soapbox> That's yet another demonstration of the evil of multiplexing syscalls. They hide the broken APIs and make them easy to introduce. And broken APIs get introduced - through each of these. prctl(), fcntl(), ioctl() - you name it. Please, don't do that. </soapbox>

Please, replace that API with something sane. "Current processor" and _maybe_ "current node" are reasonable per-process things (even though the latter is obviously redundant). They are inherently racy, however - if you get scheduled on the return from syscall the value may have nothing to reality by the time you return to userland. The rest is obviously system-wide _and_ not process-related (it's "tell me about the configuration of machine"). Implementing them as prctls makes absolutely no sense. If anything, that's sysctl material.