On Thu, 2009-09-10 at 16:31 -0700, Andrew Morton wrote:
> On Wed, 09 Sep 2009 12:31:58 -0400
> Lee Schermerhorn <lee.schermerhorn@xxxxxx> wrote:
>
> > ...
> >
> > This patch adds the per huge page size control/query attributes
> > to the per node sysdevs:
> >
> > /sys/devices/system/node/node<ID>/hugepages/hugepages-<size>/
> > nr_hugepages - r/w
> > free_huge_pages - r/o
> > surplus_huge_pages - r/o
> >
> >
> > ...
> >
> > Index: linux-2.6.31-rc7-mmotm-090827-1651/drivers/base/node.c
> > ===================================================================
> > --- linux-2.6.31-rc7-mmotm-090827-1651.orig/drivers/base/node.c 2009-09-09 11:57:26.000000000 -0400
> > +++ linux-2.6.31-rc7-mmotm-090827-1651/drivers/base/node.c 2009-09-09 11:57:37.000000000 -0400
> > @@ -177,6 +177,37 @@ static ssize_t node_read_distance(struct
> > }
> > static SYSDEV_ATTR(distance, S_IRUGO, node_read_distance, NULL);
> >
> > +/*
> > + * hugetlbfs per node attributes registration interface:
> > + * When/if hugetlb[fs] subsystem initializes [sometime after this module],
> > + * it will register it's per node attributes for all nodes on-line at that
> > + * point. It will also call register_hugetlbfs_with_node(), below, to
> > + * register it's attribute registration functions with this node driver.
> > + * Once these hooks have been initialized, the node driver will call into
> > + * the hugetlb module to [un]register attributes for hot-plugged nodes.
> > + */
> > +NODE_REGISTRATION_FUNC __hugetlb_register_node;
> > +NODE_REGISTRATION_FUNC __hugetlb_unregister_node;
>
> WHAT THE HECK IS THAT THING?
>
> Oh. It's a typedef. It's not a kernel convention to upper-case those.
> it is a kerenl convention to lower-case them and stick a _t at the
> end.
Sorry. Old habits die hard. [and checkpatch didn't complain :)]
>
> There doesn't apepar to have been any reason to make these symbols
> global.
Nah.
>
> >
> > +#ifdef CONFIG_NUMA
> > +
> > +struct node_hstate {
> > + struct kobject *hugepages_kobj;
> > + struct kobject *hstate_kobjs[HUGE_MAX_HSTATE];
> > +};
> > +struct node_hstate node_hstates[MAX_NUMNODES];
> > +
> > +static struct attribute *per_node_hstate_attrs[] = {
> > + &nr_hugepages_attr.attr,
> > + &free_hugepages_attr.attr,
> > + &surplus_hugepages_attr.attr,
> > + NULL,
> > +};
>
> I assume this interface got documented in patch 6/6.
Of course!
>
> > +static struct attribute_group per_node_hstate_attr_group = {
> > + .attrs = per_node_hstate_attrs,
> > +};
> > +
> > +static struct hstate *kobj_to_node_hstate(struct kobject *kobj, int *nidp)
> > +{
> > + int nid;
> > +
> > + for (nid = 0; nid < nr_node_ids; nid++) {
> > + struct node_hstate *nhs = &node_hstates[nid];
> > + int i;
> > + for (i = 0; i < HUGE_MAX_HSTATE; i++)
> > + if (nhs->hstate_kobjs[i] == kobj) {
> > + if (nidp)
> > + *nidp = nid;
>
> Dammit, another function which has no callers. How am I supposed
> to find out if we really need to test for a NULL nidp?
It's called from kobj_to_hstate() further up [nearer the front] of this
patch.
>
> > + return &hstates[i];
> > + }
> > + }
> > +
> > + BUG();
> > + return NULL;
> > +}
> >
> >
> > ...
> >
> > +static struct hstate *kobj_to_node_hstate(struct kobject *kobj, int *nidp)
> > +{
> > + BUG();
> > + if (nidp)
> > + *nidp = -1;
> > + return NULL;
> > +}
>
> strange.
Yeah. The pre-existing kobj_to_hstate(), for global hstate attributes,
had the BUG() for when it can't match the kobj to an hstate [shouldn't
happen]. And, then it returns NULL. I followed suit in the per node
versions, including this !NUMA stub.
>
>
>
> fixlets:
I'll merge these into my working series. I've been perparing V7--mostly
cleanup: adding some comments. Now that you've added the first part of
the V6 series to the mmotm tree, would you prefer incremental patches to
those?
>
> drivers/base/node.c | 14 +++++++-------
> hugetlb.c | 0
> include/linux/node.h | 10 +++++-----
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff -puN drivers/base/node.c~hugetlb-add-per-node-hstate-attributes-fix drivers/base/node.c
> --- a/drivers/base/node.c~hugetlb-add-per-node-hstate-attributes-fix
> +++ a/drivers/base/node.c
> @@ -180,14 +180,14 @@ static SYSDEV_ATTR(distance, S_IRUGO, no
> /*
> * hugetlbfs per node attributes registration interface:
> * When/if hugetlb[fs] subsystem initializes [sometime after this module],
> - * it will register it's per node attributes for all nodes on-line at that
> - * point. It will also call register_hugetlbfs_with_node(), below, to
> - * register it's attribute registration functions with this node driver.
> + * it will register its per node attributes for all nodes online at that
> + * time. It will also call register_hugetlbfs_with_node(), below, to
> + * register its attribute registration functions with this node driver.
> * Once these hooks have been initialized, the node driver will call into
> * the hugetlb module to [un]register attributes for hot-plugged nodes.
> */
> -NODE_REGISTRATION_FUNC __hugetlb_register_node;
> -NODE_REGISTRATION_FUNC __hugetlb_unregister_node;
> +static node_registration_func_t __hugetlb_register_node;
> +static node_registration_func_t __hugetlb_unregister_node;
>
> static inline void hugetlb_register_node(struct node *node)
> {
> @@ -201,8 +201,8 @@ static inline void hugetlb_unregister_no
> __hugetlb_unregister_node(node);
> }
>
> -void register_hugetlbfs_with_node(NODE_REGISTRATION_FUNC doregister,
> - NODE_REGISTRATION_FUNC unregister)
> +void register_hugetlbfs_with_node(node_registration_func_t doregister,
> + node_registration_func_t unregister)
> {
> __hugetlb_register_node = doregister;
> __hugetlb_unregister_node = unregister;
> diff -puN include/linux/node.h~hugetlb-add-per-node-hstate-attributes-fix include/linux/node.h
> --- a/include/linux/node.h~hugetlb-add-per-node-hstate-attributes-fix
> +++ a/include/linux/node.h
> @@ -28,7 +28,7 @@ struct node {
>
> struct memory_block;
> extern struct node node_devices[];
> -typedef void (*NODE_REGISTRATION_FUNC)(struct node *);
> +typedef void (*node_registration_func_t)(struct node *);
>
> extern int register_node(struct node *, int, struct node *);
> extern void unregister_node(struct node *node);
> @@ -40,8 +40,8 @@ extern int unregister_cpu_under_node(uns
> extern int register_mem_sect_under_node(struct memory_block *mem_blk,
> int nid);
> extern int unregister_mem_sect_under_nodes(struct memory_block *mem_blk);
> -extern void register_hugetlbfs_with_node(NODE_REGISTRATION_FUNC doregister,
> - NODE_REGISTRATION_FUNC unregister);
> +extern void register_hugetlbfs_with_node(node_registration_func_t doregister,
> + node_registration_func_t unregister);
> #else
> static inline int register_one_node(int nid)
> {
> @@ -69,8 +69,8 @@ static inline int unregister_mem_sect_un
> return 0;
> }
>
> -static inline void register_hugetlbfs_with_node(NODE_REGISTRATION_FUNC reg,
> - NODE_REGISTRATION_FUNC unreg)
> +static inline void register_hugetlbfs_with_node(node_registration_func_t reg,
> + node_registration_func_t unreg)
> {
> }
> #endif
> diff -puN mm/hugetlb.c~hugetlb-add-per-node-hstate-attributes-fix mm/hugetlb.c
> _
>
--
To unsubscribe from this list: send the line "unsubscribe linux-numa" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
