2010-01-07 22:37:28

by Randy Dunlap

[permalink] [raw]
Subject: Re: + hugetlb-fix-section-mismatch-warning-in-hugetlbc.patch added to -mm tree

On Thu, 07 Jan 2010 14:18:25 -0800 [email protected] wrote:

>
> The patch titled
> hugetlb: fix section mismatch warning in hugetlb.c
> has been added to the -mm tree. Its filename is
> hugetlb-fix-section-mismatch-warning-in-hugetlbc.patch
>
>
> ------------------------------------------------------
> Subject: hugetlb: fix section mismatch warning in hugetlb.c
> From: Rakib Mullick <[email protected]>
>
> Since hugetlb_sysfs_add_hstate()'s caller is __init and it isn't
> referencing from any other function, we can do this.

Hi,

I looked at this section mismatch warning too.
Maybe I'm reading too much into it (so I have cc-ed linux-mm),
but it looks like hugetlbfs supports callbacks for node
hotplug & unplug:

in hugetlb_register_all_nodes():

/*
* Let the node sysdev driver know we're here so it can
* [un]register hstate attributes on node hotplug.
*/
register_hugetlbfs_with_node(hugetlb_register_node,
hugetlb_unregister_node);

If so, then hugetlb_register_node() could be called at any time
(like after system init), and it would then call
hugetlb_sysfs_add_hstate(), which would be bad.

Am I misunderstanding the hotplug callbacks?
or can you explain just a bit better, please?
Thanks.


> Signed-off-by: Rakib Mullick <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> ---
>
> mm/hugetlb.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff -puN mm/hugetlb.c~hugetlb-fix-section-mismatch-warning-in-hugetlbc mm/hugetlb.c
> --- a/mm/hugetlb.c~hugetlb-fix-section-mismatch-warning-in-hugetlbc
> +++ a/mm/hugetlb.c
> @@ -1650,7 +1650,7 @@ static void hugetlb_unregister_all_nodes
> * Register hstate attributes for a single node sysdev.
> * No-op if attributes already registered.
> */
> -void hugetlb_register_node(struct node *node)
> +void __init hugetlb_register_node(struct node *node)
> {
> struct hstate *h;
> struct node_hstate *nhs = &node_hstates[node->sysdev.id];
> @@ -1683,7 +1683,7 @@ void hugetlb_register_node(struct node *
> * sysdevs of nodes that have memory. All on-line nodes should have
> * registered their associated sysdev by this time.
> */
> -static void hugetlb_register_all_nodes(void)
> +static void __init hugetlb_register_all_nodes(void)
> {
> int nid;
>
> @@ -1712,7 +1712,7 @@ static struct hstate *kobj_to_node_hstat
>
> static void hugetlb_unregister_all_nodes(void) { }
>
> -static void hugetlb_register_all_nodes(void) { }
> +static void __init hugetlb_register_all_nodes(void) { }
>
> #endif
>
> _


---
~Randy


2010-01-08 06:34:18

by Rakib Mullick

[permalink] [raw]
Subject: Re: + hugetlb-fix-section-mismatch-warning-in-hugetlbc.patch added to -mm tree

On 1/8/10, Randy Dunlap <[email protected]> wrote:

Hi,
>
> Hi,
>
> If so, then hugetlb_register_node() could be called at any time
> (like after system init), and it would then call
> hugetlb_sysfs_add_hstate(), which would be bad.
>
But - hugetlb_register_node is only called from hugetlb_register_all_nodes.
The call sequence is :
hugetlb_init ---------------> was __init
\-> hugetlb_register_all_nodes ----> we make it __init
\-> hugetlb_register_node
---> we make it __init
\->
hugetlb_sysfs_add_hstate -> this was __init

Above all happens in __init context. So - hugetlb_register_node is
called only at
system init. But I don't think __init is used for hotplug support, for
proper hotplug
support we might use __meminit.

And register_hugetlbfs_with_node is called from hugetlb_un/register_node.
But hugetlb_unregister_node doesn't use callback function. It's not referencing
hugetlb_sysfs_add_hstate(). So - it's safe. Unless I'm not missing
anything, too.

thanks,

> Thanks.
> ---
>
> ~Randy
>

2010-01-08 14:06:38

by Lee Schermerhorn

[permalink] [raw]
Subject: Re: + hugetlb-fix-section-mismatch-warning-in-hugetlbc.patch added to -mm tree

On Fri, 2010-01-08 at 12:34 +0600, Rakib Mullick wrote:
> On 1/8/10, Randy Dunlap <[email protected]> wrote:
>
> Hi,
> >
> > Hi,
> >
> > If so, then hugetlb_register_node() could be called at any time
> > (like after system init), and it would then call
> > hugetlb_sysfs_add_hstate(), which would be bad.
> >
> But - hugetlb_register_node is only called from hugetlb_register_all_nodes.

No, hugetlb_register_node() is also called from the similarly named
function in drivers/base/node.c. mm/hugetlb.c registers its version of
"hugetlb_register_node()" at start up for use at node hotplug. See
register_hugetlbfs_with_node() in node.c and its use in hugetlb.c.

We had to do it this way because node.c is part of the base kernel and
hugetlb support can be built as a module. All sysfs node kobjs have
been registered by the time hugetlb inits, so hugetlb registers the
nodes' hstate attributes when it initializes. However, when a node is
hotplugged later, the node driver calls into hugetlb to register the
attributes.

So, we need to keep mm/hugetlb.c:hugetlb_register_node() around during
run time when mem/node hot-plug is supported.

Lee


> The call sequence is :
> hugetlb_init ---------------> was __init
> \-> hugetlb_register_all_nodes ----> we make it __init
> \-> hugetlb_register_node
> ---> we make it __init
> \->
> hugetlb_sysfs_add_hstate -> this was __init
>
> Above all happens in __init context. So - hugetlb_register_node is
> called only at
> system init. But I don't think __init is used for hotplug support, for
> proper hotplug
> support we might use __meminit.
>
> And register_hugetlbfs_with_node is called from hugetlb_un/register_node.
> But hugetlb_unregister_node doesn't use callback function. It's not referencing
> hugetlb_sysfs_add_hstate(). So - it's safe. Unless I'm not missing
> anything, too.
>
> thanks,
>
> > Thanks.
> > ---
> >
> > ~Randy
> >
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>