Thread (16 messages) 16 messages, 6 authors, 2008-05-21

Re: [PATCH 1/2] Add thread_info_cache_init() to all archs

From: Andrew Morton <akpm@linux-foundation.org>
Date: 2008-04-14 00:20:31
Also in: linux-arch, lkml

On Thu, 10 Apr 2008 13:22:56 +1000 Benjamin Herrenschmidt [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Some architecture need to maintain a kmem cache for thread info
structures. (next patch adds that to powerpc to fix an alignment
problem).

There is no good arch callback to use to initialize that cache
that I can find, so this adds a new one and adds an empty macro
for when it's not implemented.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

So we have the choice here between:

 - the ifdef on the func name that I did, consistent with what
I did before for iomap, which iirc Linus liked

 - add some more ARCH_HAS_* or HAVE_* (yuck)

 - add an empty definition to all archs .h (pain in the neck but I
can do it, though it will be an annoying patch to keep around)

 - do a weak function (will slightly bloat everybody for no good reason)

So unless there is strong complaints, I'd like to stick to my
current approach.

 include/linux/sched.h |    4 ++++
 init/main.c           |    1 +
 2 files changed, 5 insertions(+)
--- linux-work.orig/init/main.c	2008-04-10 13:11:06.000000000 +1000
+++ linux-work/init/main.c	2008-04-10 13:11:19.000000000 +1000
@@ -623,6 +623,7 @@ asmlinkage void __init start_kernel(void
 	if (efi_enabled)
 		efi_enter_virtual_mode();
 #endif
+	thread_info_cache_init();
 	fork_init(num_physpages);
 	proc_caches_init();
 	buffer_init();
Index: linux-work/include/linux/sched.h
===================================================================
--- linux-work.orig/include/linux/sched.h	2008-04-10 13:11:44.000000000 +1000
+++ linux-work/include/linux/sched.h	2008-04-10 13:12:05.000000000 +1000
@@ -1893,6 +1893,10 @@ static inline unsigned long *end_of_stac
 
 #endif
 
+#ifndef thread_info_cache_init
+#define thread_info_cache_init	do { } while(0)
+#endif
This trick does cause a bit of a problem: it is undefined which arch header
file is to provide the alternative definition of thread_info_cache_init.

So we can (and have) ended up in the situation where the override appears
in different files on different architectures and various screwups ensue.

So I'd suggest that we have a bigfatcomment telling implementors which file
the override should be implemented in.  And make sure that this arch file is
directly included from within sched.h.

I have a suspicion that we can still get in a mess if .c files include the
per-arch file and don't include sched.h, but I forget where this happened
and why it broke stuff.

Sigh.  A nice, coded-in-C implementation within each and every architecture
remains the best implementation, and all the little tricks-to-save-typing
have failure modes.

otoh, if only one .c file will ever call this function then I think that
all problems are solved by

a) moving the above ifdeffery into the .c file
b) adding a comment explaining which arch file must provide the override
c) directly including that file from within the .c file.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help