Re: [PATCH 03/10] mm: make hugetlb.c explicitly non-modular

2015-08-26 Thread Mike Kravetz
On 08/24/2015 03:14 PM, Paul Gortmaker wrote:
> The Kconfig currently controlling compilation of this code is:
> 
> config HUGETLBFS
> bool "HugeTLB file system support"
> 
> ...meaning that it currently is not being built as a module by anyone.
> 
> Lets remove the modular code that is essentially orphaned, so that
> when reading the file there is no doubt it is builtin-only.
> 
> Since module_init translates to device_initcall in the non-modular
> case, the init ordering remains unchanged with this commit.  However
> one could argue that fs_initcall() would make more sense here.

I would prefer that it NOT be changed to fs_initcall() as this is more
about generic mm code than fs code.  If this was in a hugetlbfs specific
file, fs_initcall() might make more sense.

More about changing initcall below.

> Cc: Andrew Morton 
> Cc: Naoya Horiguchi 
> Cc: Mike Kravetz 
> Cc: David Rientjes 
> Cc: Hillf Danton 
> Cc: Davidlohr Bueso 
> Cc: linux...@kvack.org
> Signed-off-by: Paul Gortmaker 
> ---
>  mm/hugetlb.c | 39 +--
>  1 file changed, 1 insertion(+), 38 deletions(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 586aa69df900..1154152c8b99 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -4,7 +4,6 @@
>   */
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -2439,25 +2438,6 @@ static void hugetlb_unregister_node(struct node *node)
>   nhs->hugepages_kobj = NULL;
>  }
>  
> -/*
> - * hugetlb module exit:  unregister hstate attributes from node devices
> - * that have them.
> - */
> -static void hugetlb_unregister_all_nodes(void)
> -{
> - int nid;
> -
> - /*
> -  * disable node device registrations.
> -  */
> - register_hugetlbfs_with_node(NULL, NULL);
> -
> - /*
> -  * remove hstate attributes from any nodes that have them.
> -  */
> - for (nid = 0; nid < nr_node_ids; nid++)
> - hugetlb_unregister_node(node_devices[nid]);
> -}
>  
>  /*
>   * Register hstate attributes for a single node device.
> @@ -2522,27 +2502,10 @@ static struct hstate *kobj_to_node_hstate(struct 
> kobject *kobj, int *nidp)
>   return NULL;
>  }
>  
> -static void hugetlb_unregister_all_nodes(void) { }
> -
>  static void hugetlb_register_all_nodes(void) { }
>  
>  #endif
>  
> -static void __exit hugetlb_exit(void)
> -{
> - struct hstate *h;
> -
> - hugetlb_unregister_all_nodes();
> -
> - for_each_hstate(h) {
> - kobject_put(hstate_kobjs[hstate_index(h)]);
> - }
> -
> - kobject_put(hugepages_kobj);
> - kfree(hugetlb_fault_mutex_table);
> -}
> -module_exit(hugetlb_exit);
> -
>  static int __init hugetlb_init(void)
>  {
>   int i;
> @@ -2580,7 +2543,7 @@ static int __init hugetlb_init(void)
>   mutex_init(_fault_mutex_table[i]);
>   return 0;
>  }

I am all for removal of the module_exit and associated code.  It is
dead and is not used today.  It would be a good idea to remove this
in any case.

> -module_init(hugetlb_init);
> +device_initcall(hugetlb_init);

Other more experienced people have opinions on your staged approach
to changing these init calls.  If the consensus is to take this
approach, I would have no objections.

-- 
Mike Kravetz

>  
>  /* Should be called on processing a hugepagesz=... option */
>  void __init hugetlb_add_hstate(unsigned order)
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 03/10] mm: make hugetlb.c explicitly non-modular

2015-08-26 Thread Mike Kravetz
On 08/24/2015 03:14 PM, Paul Gortmaker wrote:
 The Kconfig currently controlling compilation of this code is:
 
 config HUGETLBFS
 bool HugeTLB file system support
 
 ...meaning that it currently is not being built as a module by anyone.
 
 Lets remove the modular code that is essentially orphaned, so that
 when reading the file there is no doubt it is builtin-only.
 
 Since module_init translates to device_initcall in the non-modular
 case, the init ordering remains unchanged with this commit.  However
 one could argue that fs_initcall() would make more sense here.

I would prefer that it NOT be changed to fs_initcall() as this is more
about generic mm code than fs code.  If this was in a hugetlbfs specific
file, fs_initcall() might make more sense.

More about changing initcall below.

 Cc: Andrew Morton a...@linux-foundation.org
 Cc: Naoya Horiguchi n-horigu...@ah.jp.nec.com
 Cc: Mike Kravetz mike.krav...@oracle.com
 Cc: David Rientjes rient...@google.com
 Cc: Hillf Danton hillf...@alibaba-inc.com
 Cc: Davidlohr Bueso d...@stgolabs.net
 Cc: linux...@kvack.org
 Signed-off-by: Paul Gortmaker paul.gortma...@windriver.com
 ---
  mm/hugetlb.c | 39 +--
  1 file changed, 1 insertion(+), 38 deletions(-)
 
 diff --git a/mm/hugetlb.c b/mm/hugetlb.c
 index 586aa69df900..1154152c8b99 100644
 --- a/mm/hugetlb.c
 +++ b/mm/hugetlb.c
 @@ -4,7 +4,6 @@
   */
  #include linux/list.h
  #include linux/init.h
 -#include linux/module.h
  #include linux/mm.h
  #include linux/seq_file.h
  #include linux/sysctl.h
 @@ -2439,25 +2438,6 @@ static void hugetlb_unregister_node(struct node *node)
   nhs-hugepages_kobj = NULL;
  }
  
 -/*
 - * hugetlb module exit:  unregister hstate attributes from node devices
 - * that have them.
 - */
 -static void hugetlb_unregister_all_nodes(void)
 -{
 - int nid;
 -
 - /*
 -  * disable node device registrations.
 -  */
 - register_hugetlbfs_with_node(NULL, NULL);
 -
 - /*
 -  * remove hstate attributes from any nodes that have them.
 -  */
 - for (nid = 0; nid  nr_node_ids; nid++)
 - hugetlb_unregister_node(node_devices[nid]);
 -}
  
  /*
   * Register hstate attributes for a single node device.
 @@ -2522,27 +2502,10 @@ static struct hstate *kobj_to_node_hstate(struct 
 kobject *kobj, int *nidp)
   return NULL;
  }
  
 -static void hugetlb_unregister_all_nodes(void) { }
 -
  static void hugetlb_register_all_nodes(void) { }
  
  #endif
  
 -static void __exit hugetlb_exit(void)
 -{
 - struct hstate *h;
 -
 - hugetlb_unregister_all_nodes();
 -
 - for_each_hstate(h) {
 - kobject_put(hstate_kobjs[hstate_index(h)]);
 - }
 -
 - kobject_put(hugepages_kobj);
 - kfree(hugetlb_fault_mutex_table);
 -}
 -module_exit(hugetlb_exit);
 -
  static int __init hugetlb_init(void)
  {
   int i;
 @@ -2580,7 +2543,7 @@ static int __init hugetlb_init(void)
   mutex_init(hugetlb_fault_mutex_table[i]);
   return 0;
  }

I am all for removal of the module_exit and associated code.  It is
dead and is not used today.  It would be a good idea to remove this
in any case.

 -module_init(hugetlb_init);
 +device_initcall(hugetlb_init);

Other more experienced people have opinions on your staged approach
to changing these init calls.  If the consensus is to take this
approach, I would have no objections.

-- 
Mike Kravetz

  
  /* Should be called on processing a hugepagesz=... option */
  void __init hugetlb_add_hstate(unsigned order)
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 03/10] mm: make hugetlb.c explicitly non-modular

2015-08-24 Thread Paul Gortmaker
The Kconfig currently controlling compilation of this code is:

config HUGETLBFS
bool "HugeTLB file system support"

...meaning that it currently is not being built as a module by anyone.

Lets remove the modular code that is essentially orphaned, so that
when reading the file there is no doubt it is builtin-only.

Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.  However
one could argue that fs_initcall() would make more sense here.

Cc: Andrew Morton 
Cc: Naoya Horiguchi 
Cc: Mike Kravetz 
Cc: David Rientjes 
Cc: Hillf Danton 
Cc: Davidlohr Bueso 
Cc: linux...@kvack.org
Signed-off-by: Paul Gortmaker 
---
 mm/hugetlb.c | 39 +--
 1 file changed, 1 insertion(+), 38 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 586aa69df900..1154152c8b99 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4,7 +4,6 @@
  */
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -2439,25 +2438,6 @@ static void hugetlb_unregister_node(struct node *node)
nhs->hugepages_kobj = NULL;
 }
 
-/*
- * hugetlb module exit:  unregister hstate attributes from node devices
- * that have them.
- */
-static void hugetlb_unregister_all_nodes(void)
-{
-   int nid;
-
-   /*
-* disable node device registrations.
-*/
-   register_hugetlbfs_with_node(NULL, NULL);
-
-   /*
-* remove hstate attributes from any nodes that have them.
-*/
-   for (nid = 0; nid < nr_node_ids; nid++)
-   hugetlb_unregister_node(node_devices[nid]);
-}
 
 /*
  * Register hstate attributes for a single node device.
@@ -2522,27 +2502,10 @@ static struct hstate *kobj_to_node_hstate(struct 
kobject *kobj, int *nidp)
return NULL;
 }
 
-static void hugetlb_unregister_all_nodes(void) { }
-
 static void hugetlb_register_all_nodes(void) { }
 
 #endif
 
-static void __exit hugetlb_exit(void)
-{
-   struct hstate *h;
-
-   hugetlb_unregister_all_nodes();
-
-   for_each_hstate(h) {
-   kobject_put(hstate_kobjs[hstate_index(h)]);
-   }
-
-   kobject_put(hugepages_kobj);
-   kfree(hugetlb_fault_mutex_table);
-}
-module_exit(hugetlb_exit);
-
 static int __init hugetlb_init(void)
 {
int i;
@@ -2580,7 +2543,7 @@ static int __init hugetlb_init(void)
mutex_init(_fault_mutex_table[i]);
return 0;
 }
-module_init(hugetlb_init);
+device_initcall(hugetlb_init);
 
 /* Should be called on processing a hugepagesz=... option */
 void __init hugetlb_add_hstate(unsigned order)
-- 
2.5.0

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


[PATCH 03/10] mm: make hugetlb.c explicitly non-modular

2015-08-24 Thread Paul Gortmaker
The Kconfig currently controlling compilation of this code is:

config HUGETLBFS
bool HugeTLB file system support

...meaning that it currently is not being built as a module by anyone.

Lets remove the modular code that is essentially orphaned, so that
when reading the file there is no doubt it is builtin-only.

Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.  However
one could argue that fs_initcall() would make more sense here.

Cc: Andrew Morton a...@linux-foundation.org
Cc: Naoya Horiguchi n-horigu...@ah.jp.nec.com
Cc: Mike Kravetz mike.krav...@oracle.com
Cc: David Rientjes rient...@google.com
Cc: Hillf Danton hillf...@alibaba-inc.com
Cc: Davidlohr Bueso d...@stgolabs.net
Cc: linux...@kvack.org
Signed-off-by: Paul Gortmaker paul.gortma...@windriver.com
---
 mm/hugetlb.c | 39 +--
 1 file changed, 1 insertion(+), 38 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 586aa69df900..1154152c8b99 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4,7 +4,6 @@
  */
 #include linux/list.h
 #include linux/init.h
-#include linux/module.h
 #include linux/mm.h
 #include linux/seq_file.h
 #include linux/sysctl.h
@@ -2439,25 +2438,6 @@ static void hugetlb_unregister_node(struct node *node)
nhs-hugepages_kobj = NULL;
 }
 
-/*
- * hugetlb module exit:  unregister hstate attributes from node devices
- * that have them.
- */
-static void hugetlb_unregister_all_nodes(void)
-{
-   int nid;
-
-   /*
-* disable node device registrations.
-*/
-   register_hugetlbfs_with_node(NULL, NULL);
-
-   /*
-* remove hstate attributes from any nodes that have them.
-*/
-   for (nid = 0; nid  nr_node_ids; nid++)
-   hugetlb_unregister_node(node_devices[nid]);
-}
 
 /*
  * Register hstate attributes for a single node device.
@@ -2522,27 +2502,10 @@ static struct hstate *kobj_to_node_hstate(struct 
kobject *kobj, int *nidp)
return NULL;
 }
 
-static void hugetlb_unregister_all_nodes(void) { }
-
 static void hugetlb_register_all_nodes(void) { }
 
 #endif
 
-static void __exit hugetlb_exit(void)
-{
-   struct hstate *h;
-
-   hugetlb_unregister_all_nodes();
-
-   for_each_hstate(h) {
-   kobject_put(hstate_kobjs[hstate_index(h)]);
-   }
-
-   kobject_put(hugepages_kobj);
-   kfree(hugetlb_fault_mutex_table);
-}
-module_exit(hugetlb_exit);
-
 static int __init hugetlb_init(void)
 {
int i;
@@ -2580,7 +2543,7 @@ static int __init hugetlb_init(void)
mutex_init(hugetlb_fault_mutex_table[i]);
return 0;
 }
-module_init(hugetlb_init);
+device_initcall(hugetlb_init);
 
 /* Should be called on processing a hugepagesz=... option */
 void __init hugetlb_add_hstate(unsigned order)
-- 
2.5.0

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