.. which modpost started warning about.
Signed-off-by: Jan Beulich <[email protected]>
kernel/kthread.c | 2 +-
mm/page_alloc.c | 18 +++++++++---------
2 files changed, 10 insertions(+), 10 deletions(-)
--- linux-2.6.22-rc4/kernel/kthread.c 2007-06-11 18:09:47.000000000 +0200
+++ 2.6.22-rc4-init-attribs/kernel/kthread.c 2007-06-11 09:10:42.000000000 +0200
@@ -215,7 +215,7 @@ int kthread_stop(struct task_struct *k)
EXPORT_SYMBOL(kthread_stop);
-static __init void kthreadd_setup(void)
+static noinline __init_refok void kthreadd_setup(void)
{
struct task_struct *tsk = current;
--- linux-2.6.22-rc4/mm/page_alloc.c 2007-06-11 18:09:49.000000000 +0200
+++ 2.6.22-rc4-init-attribs/mm/page_alloc.c 2007-06-11 09:10:42.000000000 +0200
@@ -126,13 +126,13 @@ static unsigned long __meminitdata dma_r
#endif
#endif
- struct node_active_region __meminitdata early_node_map[MAX_ACTIVE_REGIONS];
- int __meminitdata nr_nodemap_entries;
- unsigned long __meminitdata arch_zone_lowest_possible_pfn[MAX_NR_ZONES];
- unsigned long __meminitdata arch_zone_highest_possible_pfn[MAX_NR_ZONES];
+ static struct node_active_region __meminitdata early_node_map[MAX_ACTIVE_REGIONS];
+ static int __meminitdata nr_nodemap_entries;
+ static unsigned long __meminitdata arch_zone_lowest_possible_pfn[MAX_NR_ZONES];
+ static unsigned long __meminitdata arch_zone_highest_possible_pfn[MAX_NR_ZONES];
#ifdef CONFIG_MEMORY_HOTPLUG_RESERVE
- unsigned long __initdata node_boundary_start_pfn[MAX_NUMNODES];
- unsigned long __initdata node_boundary_end_pfn[MAX_NUMNODES];
+ static unsigned long __meminitdata node_boundary_start_pfn[MAX_NUMNODES];
+ static unsigned long __meminitdata node_boundary_end_pfn[MAX_NUMNODES];
#endif /* CONFIG_MEMORY_HOTPLUG_RESERVE */
#endif /* CONFIG_ARCH_POPULATES_NODE_MAP */
@@ -1968,7 +1968,7 @@ void zone_init_free_lists(struct pglist_
memmap_init_zone((size), (nid), (zone), (start_pfn), MEMMAP_EARLY)
#endif
-static int __cpuinit zone_batchsize(struct zone *zone)
+static int __devinit zone_batchsize(struct zone *zone)
{
int batch;
@@ -2370,7 +2370,7 @@ void __init push_node_boundaries(unsigne
}
/* If necessary, push the node boundary out for reserve hotadd */
-static void __init account_node_boundary(unsigned int nid,
+static void __meminit account_node_boundary(unsigned int nid,
unsigned long *start_pfn, unsigned long *end_pfn)
{
printk(KERN_DEBUG "Entering account_node_boundary(%u, %lu, %lu)\n",
@@ -2390,7 +2390,7 @@ static void __init account_node_boundary
void __init push_node_boundaries(unsigned int nid,
unsigned long start_pfn, unsigned long end_pfn) {}
-static void __init account_node_boundary(unsigned int nid,
+static void __meminit account_node_boundary(unsigned int nid,
unsigned long *start_pfn, unsigned long *end_pfn) {}
#endif
Hi Jan,
On 6/12/07, Jan Beulich <[email protected]> wrote:
> .. which modpost started warning about.
>
> Signed-off-by: Jan Beulich <[email protected]>
>
> kernel/kthread.c | 2 +-
> mm/page_alloc.c | 18 +++++++++---------
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
> --- linux-2.6.22-rc4/kernel/kthread.c 2007-06-11 18:09:47.000000000 +0200
> +++ 2.6.22-rc4-init-attribs/kernel/kthread.c 2007-06-11 09:10:42.000000000 +0200
> @@ -215,7 +215,7 @@ int kthread_stop(struct task_struct *k)
> EXPORT_SYMBOL(kthread_stop);
>
>
> -static __init void kthreadd_setup(void)
> +static noinline __init_refok void kthreadd_setup(void)
> {
> struct task_struct *tsk = current;
This isn't ok. There isn't any __init function that is (safely) referenced
by kthreadd_setup(), so we shouldn't really be marking it as such.
Also, kthreadd_setup() is really only ever called at init time, so we'd
want it to remain __init.
I believe the correct fix to silence modpost here would be to mark its
caller kthreadd() also as __init, because it too is used only at init time?
[ Cc:'ing Sam. ]
> --- linux-2.6.22-rc4/mm/page_alloc.c 2007-06-11 18:09:49.000000000 +0200
> +++ 2.6.22-rc4-init-attribs/mm/page_alloc.c 2007-06-11 09:10:42.000000000 +0200
> @@ -126,13 +126,13 @@ static unsigned long __meminitdata dma_r
> #endif
> #endif
>
> - struct node_active_region __meminitdata early_node_map[MAX_ACTIVE_REGIONS];
> - int __meminitdata nr_nodemap_entries;
> - unsigned long __meminitdata arch_zone_lowest_possible_pfn[MAX_NR_ZONES];
> - unsigned long __meminitdata arch_zone_highest_possible_pfn[MAX_NR_ZONES];
> + static struct node_active_region __meminitdata early_node_map[MAX_ACTIVE_REGIONS];
> + static int __meminitdata nr_nodemap_entries;
> + static unsigned long __meminitdata arch_zone_lowest_possible_pfn[MAX_NR_ZONES];
> + static unsigned long __meminitdata arch_zone_highest_possible_pfn[MAX_NR_ZONES];
> #ifdef CONFIG_MEMORY_HOTPLUG_RESERVE
> - unsigned long __initdata node_boundary_start_pfn[MAX_NUMNODES];
> - unsigned long __initdata node_boundary_end_pfn[MAX_NUMNODES];
> + static unsigned long __meminitdata node_boundary_start_pfn[MAX_NUMNODES];
> + static unsigned long __meminitdata node_boundary_end_pfn[MAX_NUMNODES];
Umm, can't really comment on mm/ code, but this could've been a
separate patch ... (the simple non-static to static conversions, at least)
Satyam
>> -static __init void kthreadd_setup(void)
>> +static noinline __init_refok void kthreadd_setup(void)
>> {
>> struct task_struct *tsk = current;
>
>This isn't ok. There isn't any __init function that is (safely) referenced
>by kthreadd_setup(), so we shouldn't really be marking it as such.
>Also, kthreadd_setup() is really only ever called at init time, so we'd
>want it to remain __init.
Oh, I see, I misunderstood the purpose of the tag - I assumed it would
mark an __init function that is known to only be referenced from init-only
code paths inside non-init functions (i.e. I didn't pay attention that the
resulting section's name is .text.init.refok, not .init.text.refok).
I have to admit I have some difficulty understanding when the tags
are going to be useful the way they are implemented right now.
>I believe the correct fix to silence modpost here would be to mark its
>caller kthreadd() also as __init, because it too is used only at init time?
I don't think so - it is my understanding that this is the body of a thread
that never dies.
Jan
On 6/12/07, Jan Beulich <[email protected]> wrote:
> >> -static __init void kthreadd_setup(void)
> >> +static noinline __init_refok void kthreadd_setup(void)
> >> {
> >> struct task_struct *tsk = current;
> >
> >This isn't ok. There isn't any __init function that is (safely) referenced
> >by kthreadd_setup(), so we shouldn't really be marking it as such.
> >Also, kthreadd_setup() is really only ever called at init time, so we'd
> >want it to remain __init.
>
> Oh, I see, I misunderstood the purpose of the tag - I assumed it would
> mark an __init function that is known to only be referenced from init-only
> code paths inside non-init functions (i.e. I didn't pay attention that the
> resulting section's name is .text.init.refok, not .init.text.refok).
>
> I have to admit I have some difficulty understanding when the tags
> are going to be useful the way they are implemented right now.
Yup, I had discussed precisely the same issue (whether to associate
__init_refok with callers or callees) with Sam earlier, but he thought
it'd be more useful to have normal-caller-can-ref-init-callees semantics
for the same.
> >I believe the correct fix to silence modpost here would be to mark its
> >caller kthreadd() also as __init, because it too is used only at init time?
>
> I don't think so - it is my understanding that this is the body of a thread
> that never dies.
Ugh, yes, I'm smoking God-knows-what, and you're absolutely correct!
So we should be marking kthreadd() as __init_refok instead, it seems.
Satyam
On 6/12/07, Satyam Sharma <[email protected]> wrote:
> On 6/12/07, Jan Beulich <[email protected]> wrote:
> [...]
> > >I believe the correct fix to silence modpost here would be to mark its
> > >caller kthreadd() also as __init, because it too is used only at init time?
> >
> > I don't think so - it is my understanding that this is the body of a thread
> > that never dies.
>
> Ugh, yes, I'm smoking God-knows-what, and you're absolutely correct!
>
> So we should be marking kthreadd() as __init_refok instead, it seems.
Actually, kthreadd_setup() is a 4-liner with only one callsite. Not much
point marking it as __init. In fact, does it even need to be a separate
function at all? We could just get rid of kthreadd_setup() and open-code
those 4 lines in kthreadd() itself ...