Mark function as static in compaction.c because it is not used outside
this file.
This eliminates the following warning from mm/compaction.c:
mm/compaction.c:1190:9: warning: no previous prototype for ‘sysfs_compact_node’ [-Wmissing-prototypes
Signed-off-by: Rashika Kheria <[email protected]>
Reviewed-by: Josh Triplett <[email protected]>
---
mm/compaction.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/compaction.c b/mm/compaction.c
index 805165b..a21f540 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1187,7 +1187,7 @@ int sysctl_extfrag_handler(struct ctl_table *table, int write,
}
#if defined(CONFIG_SYSFS) && defined(CONFIG_NUMA)
-ssize_t sysfs_compact_node(struct device *dev,
+static ssize_t sysfs_compact_node(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
--
1.7.9.5
Mark functions as static in memory.c because they are not used outside
this file.
This eliminates the following warnings in mm/memory.c:
mm/memory.c:3530:5: warning: no previous prototype for ‘numa_migrate_prep’ [-Wmissing-prototypes]
mm/memory.c:3545:5: warning: no previous prototype for ‘do_numa_page’ [-Wmissing-prototypes]
Signed-off-by: Rashika Kheria <[email protected]>
Reviewed-by: Josh Triplett <[email protected]>
---
mm/memory.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/memory.c b/mm/memory.c
index 5d9025f..982c1ad 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3527,7 +3527,7 @@ static int do_nonlinear_fault(struct mm_struct *mm, struct vm_area_struct *vma,
return __do_fault(mm, vma, address, pmd, pgoff, flags, orig_pte);
}
-int numa_migrate_prep(struct page *page, struct vm_area_struct *vma,
+static int numa_migrate_prep(struct page *page, struct vm_area_struct *vma,
unsigned long addr, int page_nid,
int *flags)
{
@@ -3542,7 +3542,7 @@ int numa_migrate_prep(struct page *page, struct vm_area_struct *vma,
return mpol_misplaced(page, vma, addr);
}
-int do_numa_page(struct mm_struct *mm, struct vm_area_struct *vma,
+static int do_numa_page(struct mm_struct *mm, struct vm_area_struct *vma,
unsigned long addr, pte_t pte, pte_t *ptep, pmd_t *pmd)
{
struct page *page = NULL;
--
1.7.9.5
Mark function as static in mmap.c because they are not used outside this
file.
This eliminates the following warning in mm/mmap.c:
mm/mmap.c:407:6: warning: no previous prototype for ‘validate_mm’ [-Wmissing-prototypes]
Signed-off-by: Rashika Kheria <[email protected]>
Reviewed-by: Josh Triplett <[email protected]>
---
mm/mmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/mmap.c b/mm/mmap.c
index 834b2d7..4a03790 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -404,7 +404,7 @@ static void validate_mm_rb(struct rb_root *root, struct vm_area_struct *ignore)
}
}
-void validate_mm(struct mm_struct *mm)
+static void validate_mm(struct mm_struct *mm)
{
int bug = 0;
int i = 0;
--
1.7.9.5
Mark function as static in process_vm_access.c because it is not used
outside this file.
This eliminates the following warning in mm/process_vm_access.c:
mm/process_vm_access.c:416:1: warning: no previous prototype for ‘compat_process_vm_rw’ [-Wmissing-prototypes]
Signed-off-by: Rashika Kheria <[email protected]>
Reviewed-by: Josh Triplett <[email protected]>
---
mm/process_vm_access.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/process_vm_access.c b/mm/process_vm_access.c
index fd26d04..f3aabbd 100644
--- a/mm/process_vm_access.c
+++ b/mm/process_vm_access.c
@@ -412,7 +412,7 @@ SYSCALL_DEFINE6(process_vm_writev, pid_t, pid,
#ifdef CONFIG_COMPAT
-asmlinkage ssize_t
+static asmlinkage ssize_t
compat_process_vm_rw(compat_pid_t pid,
const struct compat_iovec __user *lvec,
unsigned long liovcnt,
--
1.7.9.5
Mark functions as static in migrate.c because they are not used outside
this file.
This eliminates the following warnings in mm/migrate.c:
mm/migrate.c:1595:6: warning: no previous prototype for ‘numamigrate_update_ratelimit’ [-Wmissing-prototypes]
mm/migrate.c:1619:5: warning: no previous prototype for ‘numamigrate_isolate_page’ [-Wmissing-prototypes]
Signed-off-by: Rashika Kheria <[email protected]>
Reviewed-by: Josh Triplett <[email protected]>
---
mm/migrate.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/mm/migrate.c b/mm/migrate.c
index bb94004..c916e73 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1592,7 +1592,8 @@ bool migrate_ratelimited(int node)
}
/* Returns true if the node is migrate rate-limited after the update */
-bool numamigrate_update_ratelimit(pg_data_t *pgdat, unsigned long nr_pages)
+static bool numamigrate_update_ratelimit(pg_data_t *pgdat,
+ unsigned long nr_pages)
{
bool rate_limited = false;
@@ -1616,7 +1617,7 @@ bool numamigrate_update_ratelimit(pg_data_t *pgdat, unsigned long nr_pages)
return rate_limited;
}
-int numamigrate_isolate_page(pg_data_t *pgdat, struct page *page)
+static int numamigrate_isolate_page(pg_data_t *pgdat, struct page *page)
{
int page_lru;
--
1.7.9.5
Mark function as static in memcontrol.c because it is not used outside
this file.
This also eliminates the following warning in memcontrol.c:
mm/memcontrol.c:3089:5: warning: no previous prototype for ‘memcg_update_cache_sizes’ [-Wmissing-prototypes]
Signed-off-by: Rashika Kheria <[email protected]>
Reviewed-by: Josh Triplett <[email protected]>
---
mm/memcontrol.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index bf5e894..fbec2a5 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3086,7 +3086,7 @@ int memcg_cache_id(struct mem_cgroup *memcg)
* But when we create a new cache, we can call this as well if its parent
* is kmem-limited. That will have to hold set_limit_mutex as well.
*/
-int memcg_update_cache_sizes(struct mem_cgroup *memcg)
+static int memcg_update_cache_sizes(struct mem_cgroup *memcg)
{
int num, ret;
--
1.7.9.5
Mark functions as static in page_cgroup.c because they are not used
outside this file.
This eliminates the following warning in mm/page_cgroup.c:
mm/page_cgroup.c:177:6: warning: no previous prototype for ‘__free_page_cgroup’ [-Wmissing-prototypes]
mm/page_cgroup.c:190:15: warning: no previous prototype for ‘online_page_cgroup’ [-Wmissing-prototypes]
mm/page_cgroup.c:225:15: warning: no previous prototype for ‘offline_page_cgroup’ [-Wmissing-prototypes]
Signed-off-by: Rashika Kheria <[email protected]>
Reviewed-by: Josh Triplett <[email protected]>
---
mm/page_cgroup.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/mm/page_cgroup.c b/mm/page_cgroup.c
index 6d757e3a..6ec349c 100644
--- a/mm/page_cgroup.c
+++ b/mm/page_cgroup.c
@@ -174,7 +174,7 @@ static void free_page_cgroup(void *addr)
}
}
-void __free_page_cgroup(unsigned long pfn)
+static void __free_page_cgroup(unsigned long pfn)
{
struct mem_section *ms;
struct page_cgroup *base;
@@ -187,9 +187,9 @@ void __free_page_cgroup(unsigned long pfn)
ms->page_cgroup = NULL;
}
-int __meminit online_page_cgroup(unsigned long start_pfn,
- unsigned long nr_pages,
- int nid)
+static int __meminit online_page_cgroup(unsigned long start_pfn,
+ unsigned long nr_pages,
+ int nid)
{
unsigned long start, end, pfn;
int fail = 0;
@@ -222,8 +222,8 @@ int __meminit online_page_cgroup(unsigned long start_pfn,
return -ENOMEM;
}
-int __meminit offline_page_cgroup(unsigned long start_pfn,
- unsigned long nr_pages, int nid)
+static int __meminit offline_page_cgroup(unsigned long start_pfn,
+ unsigned long nr_pages, int nid)
{
unsigned long start, end, pfn;
--
1.7.9.5
Mark function as static in nobootmem.c because it is not used outside
this file.
This eliminates the following warning in mm/nobootmem.c:
mm/nobootmem.c:324:15: warning: no previous prototype for ‘___alloc_bootmem_node’ [-Wmissing-prototypes]
Signed-off-by: Rashika Kheria <[email protected]>
Reviewed-by: Josh Triplett <[email protected]>
---
The symbol '___alloc_bootmem_node' has also been defined in
mm/bootmem.c. Both the implementations are almost similar and hence
should be unified.
mm/nobootmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/nobootmem.c b/mm/nobootmem.c
index 2c254d3..a3724a1 100644
--- a/mm/nobootmem.c
+++ b/mm/nobootmem.c
@@ -321,7 +321,7 @@ void * __init __alloc_bootmem_node_nopanic(pg_data_t *pgdat, unsigned long size,
return ___alloc_bootmem_node_nopanic(pgdat, size, align, goal, 0);
}
-void * __init ___alloc_bootmem_node(pg_data_t *pgdat, unsigned long size,
+static void * __init ___alloc_bootmem_node(pg_data_t *pgdat, unsigned long size,
unsigned long align, unsigned long goal,
unsigned long limit)
{
--
1.7.9.5
The ifdef conditions in include/linux/mm.h presents three cases:
- !defined(CONFIG_HAVE_MEMBLOCK_NODE_MAP) && !defined(CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID)
There is no actual definition of function but include/linux/mm.h has a
static inline stub defined.
- defined(CONFIG_HAVE_MEMBLOCK_NODE_MAP) && !defined(CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID)
linux/mm.h does not define a prototype, but mm/page_alloc.c defines
the function.
Hence, compiler reports the following warning:
mm/page_alloc.c:4300:15: warning: no previous prototype for ‘__early_pfn_to_nid’ [-Wmissing-prototypes]
- defined(CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID)
The architecture defines the function, and linux/mm.h has a prototype.
Thus, join the conditions of Case 2 and 3 i.e. eliminate the ifdef
condition of CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID to eliminate the
missing prototype warning from file mm/page_alloc.c.
Signed-off-by: Rashika Kheria <[email protected]>
Reviewed-by: Josh Triplett <[email protected]>
---
include/linux/mm.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 1cedd00..5f8348f 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1589,10 +1589,8 @@ static inline int __early_pfn_to_nid(unsigned long pfn)
#else
/* please see mm/page_alloc.c */
extern int __meminit early_pfn_to_nid(unsigned long pfn);
-#ifdef CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID
/* there is a per-arch backend function. */
extern int __meminit __early_pfn_to_nid(unsigned long pfn);
-#endif /* CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID */
#endif
extern void set_dma_reserve(unsigned long new_dma_reserve);
--
1.7.9.5
On 02/07/2014 07:01 AM, Rashika Kheria wrote:
> Mark function as static in compaction.c because it is not used outside
> this file.
>
> This eliminates the following warning from mm/compaction.c:
> mm/compaction.c:1190:9: warning: no previous prototype for ‘sysfs_compact_node’ [-Wmissing-prototypes
>
> Signed-off-by: Rashika Kheria <[email protected]>
> Reviewed-by: Josh Triplett <[email protected]>
Reviewed-by: Rik van Riel <[email protected]>
--
All rights reversed
On 02/07/2014 07:03 AM, Rashika Kheria wrote:
> Mark functions as static in memory.c because they are not used outside
> this file.
>
> This eliminates the following warnings in mm/memory.c:
> mm/memory.c:3530:5: warning: no previous prototype for ‘numa_migrate_prep’ [-Wmissing-prototypes]
> mm/memory.c:3545:5: warning: no previous prototype for ‘do_numa_page’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria <[email protected]>
> Reviewed-by: Josh Triplett <[email protected]>
Reviewed-by: Rik van Riel <[email protected]>
--
All rights reversed
On 02/07/2014 07:04 AM, Rashika Kheria wrote:
> Mark function as static in mmap.c because they are not used outside this
> file.
>
> This eliminates the following warning in mm/mmap.c:
> mm/mmap.c:407:6: warning: no previous prototype for ‘validate_mm’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria <[email protected]>
> Reviewed-by: Josh Triplett <[email protected]>
Reviewed-by: Rik van Riel <[email protected]>
--
All rights reversed
On 02/07/2014 07:08 AM, Rashika Kheria wrote:
> Mark functions as static in migrate.c because they are not used outside
> this file.
>
> This eliminates the following warnings in mm/migrate.c:
> mm/migrate.c:1595:6: warning: no previous prototype for ‘numamigrate_update_ratelimit’ [-Wmissing-prototypes]
> mm/migrate.c:1619:5: warning: no previous prototype for ‘numamigrate_isolate_page’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria <[email protected]>
> Reviewed-by: Josh Triplett <[email protected]>
Reviewed-by: Rik van Riel <[email protected]>
--
All rights reversed
On 02/07/2014 07:15 AM, Rashika Kheria wrote:
> The ifdef conditions in include/linux/mm.h presents three cases:
>
> - !defined(CONFIG_HAVE_MEMBLOCK_NODE_MAP) && !defined(CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID)
> There is no actual definition of function but include/linux/mm.h has a
> static inline stub defined.
>
> - defined(CONFIG_HAVE_MEMBLOCK_NODE_MAP) && !defined(CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID)
> linux/mm.h does not define a prototype, but mm/page_alloc.c defines
> the function.
> Hence, compiler reports the following warning:
> mm/page_alloc.c:4300:15: warning: no previous prototype for ‘__early_pfn_to_nid’ [-Wmissing-prototypes]
>
> - defined(CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID)
> The architecture defines the function, and linux/mm.h has a prototype.
>
> Thus, join the conditions of Case 2 and 3 i.e. eliminate the ifdef
> condition of CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID to eliminate the
> missing prototype warning from file mm/page_alloc.c.
>
> Signed-off-by: Rashika Kheria <[email protected]>
> Reviewed-by: Josh Triplett <[email protected]>
Reviewed-by: Rik van Riel <[email protected]>
--
All rights reversed
On Fri, 7 Feb 2014, Rashika Kheria wrote:
> Mark function as static in compaction.c because it is not used outside
> this file.
>
> This eliminates the following warning from mm/compaction.c:
> mm/compaction.c:1190:9: warning: no previous prototype for ‘sysfs_compact_node’ [-Wmissing-prototypes
>
> Signed-off-by: Rashika Kheria <[email protected]>
> Reviewed-by: Josh Triplett <[email protected]>
Acked-by: David Rientjes <[email protected]>
On Fri, 7 Feb 2014, Rashika Kheria wrote:
> Mark functions as static in memory.c because they are not used outside
> this file.
>
> This eliminates the following warnings in mm/memory.c:
> mm/memory.c:3530:5: warning: no previous prototype for ‘numa_migrate_prep’ [-Wmissing-prototypes]
> mm/memory.c:3545:5: warning: no previous prototype for ‘do_numa_page’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria <[email protected]>
> Reviewed-by: Josh Triplett <[email protected]>
Acked-by: David Rientjes <[email protected]>
On Fri, 7 Feb 2014, Rashika Kheria wrote:
> Mark function as static in mmap.c because they are not used outside this
> file.
>
> This eliminates the following warning in mm/mmap.c:
> mm/mmap.c:407:6: warning: no previous prototype for ‘validate_mm’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria <[email protected]>
> Reviewed-by: Josh Triplett <[email protected]>
Acked-by: David Rientjes <[email protected]>
On Fri, 7 Feb 2014, Rashika Kheria wrote:
> Mark function as static in process_vm_access.c because it is not used
> outside this file.
>
> This eliminates the following warning in mm/process_vm_access.c:
> mm/process_vm_access.c:416:1: warning: no previous prototype for ‘compat_process_vm_rw’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria <[email protected]>
> Reviewed-by: Josh Triplett <[email protected]>
Acked-by: David Rientjes <[email protected]>
On Fri, 7 Feb 2014, Rashika Kheria wrote:
> Mark functions as static in migrate.c because they are not used outside
> this file.
>
> This eliminates the following warnings in mm/migrate.c:
> mm/migrate.c:1595:6: warning: no previous prototype for ‘numamigrate_update_ratelimit’ [-Wmissing-prototypes]
> mm/migrate.c:1619:5: warning: no previous prototype for ‘numamigrate_isolate_page’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria <[email protected]>
> Reviewed-by: Josh Triplett <[email protected]>
Acked-by: David Rientjes <[email protected]>
On Fri, 7 Feb 2014, Rashika Kheria wrote:
> Mark function as static in memcontrol.c because it is not used outside
> this file.
>
> This also eliminates the following warning in memcontrol.c:
> mm/memcontrol.c:3089:5: warning: no previous prototype for ‘memcg_update_cache_sizes’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria <[email protected]>
> Reviewed-by: Josh Triplett <[email protected]>
memcg_update_cache_sizes() was removed in commit d6441637709b ("memcg:
rework memcg_update_kmem_limit synchronization") for 3.14-rc1.
On Fri, 7 Feb 2014, Rashika Kheria wrote:
> Mark functions as static in page_cgroup.c because they are not used
> outside this file.
>
> This eliminates the following warning in mm/page_cgroup.c:
> mm/page_cgroup.c:177:6: warning: no previous prototype for ‘__free_page_cgroup’ [-Wmissing-prototypes]
> mm/page_cgroup.c:190:15: warning: no previous prototype for ‘online_page_cgroup’ [-Wmissing-prototypes]
> mm/page_cgroup.c:225:15: warning: no previous prototype for ‘offline_page_cgroup’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria <[email protected]>
> Reviewed-by: Josh Triplett <[email protected]>
Acked-by: David Rientjes <[email protected]>
On Fri, 7 Feb 2014, Rashika Kheria wrote:
> Mark function as static in nobootmem.c because it is not used outside
> this file.
>
> This eliminates the following warning in mm/nobootmem.c:
> mm/nobootmem.c:324:15: warning: no previous prototype for ‘___alloc_bootmem_node’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria <[email protected]>
> Reviewed-by: Josh Triplett <[email protected]>
Acked-by: David Rientjes <[email protected]>
On Fri, 7 Feb 2014, Rashika Kheria wrote:
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 1cedd00..5f8348f 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1589,10 +1589,8 @@ static inline int __early_pfn_to_nid(unsigned long pfn)
> #else
> /* please see mm/page_alloc.c */
> extern int __meminit early_pfn_to_nid(unsigned long pfn);
> -#ifdef CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID
> /* there is a per-arch backend function. */
> extern int __meminit __early_pfn_to_nid(unsigned long pfn);
> -#endif /* CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID */
> #endif
>
> extern void set_dma_reserve(unsigned long new_dma_reserve);
Wouldn't it be better to just declare the __early_pfn_to_nid() in
mm/page_alloc.c to be static?
On Fri, Feb 07, 2014 at 01:04:47PM -0800, David Rientjes wrote:
> On Fri, 7 Feb 2014, Rashika Kheria wrote:
>
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 1cedd00..5f8348f 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -1589,10 +1589,8 @@ static inline int __early_pfn_to_nid(unsigned long pfn)
> > #else
> > /* please see mm/page_alloc.c */
> > extern int __meminit early_pfn_to_nid(unsigned long pfn);
> > -#ifdef CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID
> > /* there is a per-arch backend function. */
> > extern int __meminit __early_pfn_to_nid(unsigned long pfn);
> > -#endif /* CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID */
> > #endif
> >
> > extern void set_dma_reserve(unsigned long new_dma_reserve);
>
> Wouldn't it be better to just declare the __early_pfn_to_nid() in
> mm/page_alloc.c to be static?
Won't that break the ability to override that function in
architecture-specific code (as arch/ia64/mm/numa.c does)?
- Josh Triplett
On Fri, 7 Feb 2014, Josh Triplett wrote:
> > > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > > index 1cedd00..5f8348f 100644
> > > --- a/include/linux/mm.h
> > > +++ b/include/linux/mm.h
> > > @@ -1589,10 +1589,8 @@ static inline int __early_pfn_to_nid(unsigned long pfn)
> > > #else
> > > /* please see mm/page_alloc.c */
> > > extern int __meminit early_pfn_to_nid(unsigned long pfn);
> > > -#ifdef CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID
> > > /* there is a per-arch backend function. */
> > > extern int __meminit __early_pfn_to_nid(unsigned long pfn);
> > > -#endif /* CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID */
> > > #endif
> > >
> > > extern void set_dma_reserve(unsigned long new_dma_reserve);
> >
> > Wouldn't it be better to just declare the __early_pfn_to_nid() in
> > mm/page_alloc.c to be static?
>
> Won't that break the ability to override that function in
> architecture-specific code (as arch/ia64/mm/numa.c does)?
>
Why? CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID should define where this function
is defined so ia64 should have it set and the definition which I'm
suggesting be static is only compiled when this is undefined in
mm/page_alloc.c. I'm not sure why we'd want to be messing with the
declaration?
On Fri, 7 Feb 2014 13:15:29 -0800 (PST) David Rientjes <[email protected]> wrote:
> On Fri, 7 Feb 2014, Josh Triplett wrote:
>
> > > > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > > > index 1cedd00..5f8348f 100644
> > > > --- a/include/linux/mm.h
> > > > +++ b/include/linux/mm.h
> > > > @@ -1589,10 +1589,8 @@ static inline int __early_pfn_to_nid(unsigned long pfn)
> > > > #else
> > > > /* please see mm/page_alloc.c */
> > > > extern int __meminit early_pfn_to_nid(unsigned long pfn);
> > > > -#ifdef CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID
> > > > /* there is a per-arch backend function. */
> > > > extern int __meminit __early_pfn_to_nid(unsigned long pfn);
> > > > -#endif /* CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID */
> > > > #endif
> > > >
> > > > extern void set_dma_reserve(unsigned long new_dma_reserve);
> > >
> > > Wouldn't it be better to just declare the __early_pfn_to_nid() in
> > > mm/page_alloc.c to be static?
> >
> > Won't that break the ability to override that function in
> > architecture-specific code (as arch/ia64/mm/numa.c does)?
> >
>
> Why? CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID should define where this function
> is defined so ia64 should have it set and the definition which I'm
> suggesting be static is only compiled when this is undefined in
> mm/page_alloc.c. I'm not sure why we'd want to be messing with the
> declaration?
__early_pfn_to_nid() must be global if it is implemented in arch/.
Making it static when it is implemented in core mm makes a bit of
sense, in that it cleans up the non-ia64 namespace and discourages
usage from other compilation units. But it's is a bit odd and
unexpected to do such a thing. I'm inclined to happily nuke the ifdef
then go think about something else ;)
On Fri, 7 Feb 2014, Andrew Morton wrote:
> > > > > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > > > > index 1cedd00..5f8348f 100644
> > > > > --- a/include/linux/mm.h
> > > > > +++ b/include/linux/mm.h
> > > > > @@ -1589,10 +1589,8 @@ static inline int __early_pfn_to_nid(unsigned long pfn)
> > > > > #else
> > > > > /* please see mm/page_alloc.c */
> > > > > extern int __meminit early_pfn_to_nid(unsigned long pfn);
> > > > > -#ifdef CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID
> > > > > /* there is a per-arch backend function. */
> > > > > extern int __meminit __early_pfn_to_nid(unsigned long pfn);
> > > > > -#endif /* CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID */
> > > > > #endif
> > > > >
> > > > > extern void set_dma_reserve(unsigned long new_dma_reserve);
> > > >
> > > > Wouldn't it be better to just declare the __early_pfn_to_nid() in
> > > > mm/page_alloc.c to be static?
> > >
> > > Won't that break the ability to override that function in
> > > architecture-specific code (as arch/ia64/mm/numa.c does)?
> > >
> >
> > Why? CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID should define where this function
> > is defined so ia64 should have it set and the definition which I'm
> > suggesting be static is only compiled when this is undefined in
> > mm/page_alloc.c. I'm not sure why we'd want to be messing with the
> > declaration?
>
> __early_pfn_to_nid() must be global if it is implemented in arch/.
>
Why?? If CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID then, yes, we need it to be
global. Otherwise it's perfectly fine just being static in file scope.
This causes the compilation unit to break when you compile it, not wait
until vmlinux and find undefined references.
I see no reason it can't be done like this in mm/page_alloc.c:
#ifdef CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID
extern int __meminit __early_pfn_to_nid(unsigned long pfn);
#else
static int __meminit __early_pfn_to_nid(unsigned long pfn)
{
...
}
or delcare __early_pfn_to_nid() to have __attribute__((weak)) and override
it when CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID (and get rid of the pointless
CONFIG option entirely at that point).
Both of these options look much better than
include/linux/mm.h:
#if !defined(CONFIG_HAVE_MEMBLOCK_NODE_MAP) && \
!defined(CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID)
static inline int __early_pfn_to_nid(unsigned long pfn)
{
return 0;
}
#else
/* please see mm/page_alloc.c */
extern int __meminit early_pfn_to_nid(unsigned long pfn);
#ifdef CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID
/* there is a per-arch backend function. */
extern int __meminit __early_pfn_to_nid(unsigned long pfn);
#endif /* CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID */
#endif
where all this confusion is originating from.
It's obviously up to your taste in how to proceed, but the latter looks
sloppy to me and is the reason we have so many unreferenced prototypes in
header files.
On Fri, Feb 07, 2014 at 03:09:09PM -0800, David Rientjes wrote:
> On Fri, 7 Feb 2014, Andrew Morton wrote:
>
> > > > > > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > > > > > index 1cedd00..5f8348f 100644
> > > > > > --- a/include/linux/mm.h
> > > > > > +++ b/include/linux/mm.h
> > > > > > @@ -1589,10 +1589,8 @@ static inline int __early_pfn_to_nid(unsigned long pfn)
> > > > > > #else
> > > > > > /* please see mm/page_alloc.c */
> > > > > > extern int __meminit early_pfn_to_nid(unsigned long pfn);
> > > > > > -#ifdef CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID
> > > > > > /* there is a per-arch backend function. */
> > > > > > extern int __meminit __early_pfn_to_nid(unsigned long pfn);
> > > > > > -#endif /* CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID */
> > > > > > #endif
> > > > > >
> > > > > > extern void set_dma_reserve(unsigned long new_dma_reserve);
> > > > >
> > > > > Wouldn't it be better to just declare the __early_pfn_to_nid() in
> > > > > mm/page_alloc.c to be static?
> > > >
> > > > Won't that break the ability to override that function in
> > > > architecture-specific code (as arch/ia64/mm/numa.c does)?
> > > >
> > >
> > > Why? CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID should define where this function
> > > is defined so ia64 should have it set and the definition which I'm
> > > suggesting be static is only compiled when this is undefined in
> > > mm/page_alloc.c. I'm not sure why we'd want to be messing with the
> > > declaration?
> >
> > __early_pfn_to_nid() must be global if it is implemented in arch/.
> >
>
> Why?? If CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID then, yes, we need it to be
> global. Otherwise it's perfectly fine just being static in file scope.
> This causes the compilation unit to break when you compile it, not wait
> until vmlinux and find undefined references.
>
> I see no reason it can't be done like this in mm/page_alloc.c:
>
> #ifdef CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID
> extern int __meminit __early_pfn_to_nid(unsigned long pfn);
No, a .c file should not have an extern declaration in it. This should
live in an appropriate header file, to be included in both page_alloc.c
and any arch file that defines an overriding function.
> Both of these options look much better than
>
> include/linux/mm.h:
>
> #if !defined(CONFIG_HAVE_MEMBLOCK_NODE_MAP) && \
> !defined(CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID)
> static inline int __early_pfn_to_nid(unsigned long pfn)
> {
> return 0;
> }
> #else
> /* please see mm/page_alloc.c */
> extern int __meminit early_pfn_to_nid(unsigned long pfn);
> #ifdef CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID
> /* there is a per-arch backend function. */
> extern int __meminit __early_pfn_to_nid(unsigned long pfn);
> #endif /* CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID */
> #endif
>
> where all this confusion is originating from.
The proposal is to first simplify those ifdefs by eliminating the inner
one in the #else; I agree with Andrew that we ought to go ahead and take
that step given the patch at hand, and then figure out if there's an
additional simplification possible.
- Josh Triplett
On Fri, 7 Feb 2014, Josh Triplett wrote:
> > Why?? If CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID then, yes, we need it to be
> > global. Otherwise it's perfectly fine just being static in file scope.
> > This causes the compilation unit to break when you compile it, not wait
> > until vmlinux and find undefined references.
> >
> > I see no reason it can't be done like this in mm/page_alloc.c:
> >
> > #ifdef CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID
> > extern int __meminit __early_pfn_to_nid(unsigned long pfn);
>
> No, a .c file should not have an extern declaration in it. This should
> live in an appropriate header file, to be included in both page_alloc.c
> and any arch file that defines an overriding function.
>
Ok, so you have religious beliefs about extern being used in files ending
in .c and don't mind the 2900 occurrences of it in the kernel tree and
desire 14 line obfuscation in header files with comments to what is being
defined in .c files such as "please see mm/page_alloc.c" as mm.h has.
Good point.
> > Both of these options look much better than
> >
> > include/linux/mm.h:
> >
> > #if !defined(CONFIG_HAVE_MEMBLOCK_NODE_MAP) && \
> > !defined(CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID)
> > static inline int __early_pfn_to_nid(unsigned long pfn)
> > {
> > return 0;
> > }
> > #else
> > /* please see mm/page_alloc.c */
> > extern int __meminit early_pfn_to_nid(unsigned long pfn);
> > #ifdef CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID
> > /* there is a per-arch backend function. */
> > extern int __meminit __early_pfn_to_nid(unsigned long pfn);
> > #endif /* CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID */
> > #endif
> >
> > where all this confusion is originating from.
>
> The proposal is to first simplify those ifdefs by eliminating the inner
> one in the #else; I agree with Andrew that we ought to go ahead and take
> that step given the patch at hand, and then figure out if there's an
> additional simplification possible.
>
If additional simplification is possible? Yeah, it's __weak which is
designed for this purpose.
On Fri, Feb 07, 2014 at 05:02:02PM -0800, David Rientjes wrote:
> On Fri, 7 Feb 2014, Josh Triplett wrote:
>
> > > Why?? If CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID then, yes, we need it to be
> > > global. Otherwise it's perfectly fine just being static in file scope.
> > > This causes the compilation unit to break when you compile it, not wait
> > > until vmlinux and find undefined references.
> > >
> > > I see no reason it can't be done like this in mm/page_alloc.c:
> > >
> > > #ifdef CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID
> > > extern int __meminit __early_pfn_to_nid(unsigned long pfn);
> >
> > No, a .c file should not have an extern declaration in it. This should
> > live in an appropriate header file, to be included in both page_alloc.c
> > and any arch file that defines an overriding function.
> >
>
> Ok, so you have religious beliefs about extern being used in files ending
> in .c and don't mind the 2900 occurrences of it in the kernel tree and
> desire 14 line obfuscation in header files with comments to what is being
> defined in .c files such as "please see mm/page_alloc.c" as mm.h has.
I (and many others) have very specific technical objections to not
having the same prototype seen by both the definition and use of a
function: that helps keep the prototype in sync with the definition(s),
helps keep all definitions in sync if there are multiple such
definitions, and eliminates -Wmissing-prototype warnings (which in
addition to those benefits also help detect functions that should be
made static or eliminated).
And as mentioned before, those 14 lines can be significantly simplified;
Rashika's patch already does one such simplification.
Those 2900 occurrences should go away as well, and Rashika's previous
patches have already fixed many of them.
> > > Both of these options look much better than
> > >
> > > include/linux/mm.h:
> > >
> > > #if !defined(CONFIG_HAVE_MEMBLOCK_NODE_MAP) && \
> > > !defined(CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID)
> > > static inline int __early_pfn_to_nid(unsigned long pfn)
> > > {
> > > return 0;
> > > }
> > > #else
> > > /* please see mm/page_alloc.c */
> > > extern int __meminit early_pfn_to_nid(unsigned long pfn);
> > > #ifdef CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID
> > > /* there is a per-arch backend function. */
> > > extern int __meminit __early_pfn_to_nid(unsigned long pfn);
> > > #endif /* CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID */
> > > #endif
> > >
> > > where all this confusion is originating from.
> >
> > The proposal is to first simplify those ifdefs by eliminating the inner
> > one in the #else; I agree with Andrew that we ought to go ahead and take
> > that step given the patch at hand, and then figure out if there's an
> > additional simplification possible.
> >
>
> If additional simplification is possible? Yeah, it's __weak which is
> designed for this purpose.
No objections here if someone wants to write that patch.
- Josh Triplett
On Fri 07-02-14 17:41:34, Rashika Kheria wrote:
> Mark functions as static in page_cgroup.c because they are not used
> outside this file.
>
> This eliminates the following warning in mm/page_cgroup.c:
> mm/page_cgroup.c:177:6: warning: no previous prototype for ‘__free_page_cgroup’ [-Wmissing-prototypes]
> mm/page_cgroup.c:190:15: warning: no previous prototype for ‘online_page_cgroup’ [-Wmissing-prototypes]
> mm/page_cgroup.c:225:15: warning: no previous prototype for ‘offline_page_cgroup’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria <[email protected]>
> Reviewed-by: Josh Triplett <[email protected]>
Acked-by: Michal Hocko <[email protected]>
> ---
> mm/page_cgroup.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/mm/page_cgroup.c b/mm/page_cgroup.c
> index 6d757e3a..6ec349c 100644
> --- a/mm/page_cgroup.c
> +++ b/mm/page_cgroup.c
> @@ -174,7 +174,7 @@ static void free_page_cgroup(void *addr)
> }
> }
>
> -void __free_page_cgroup(unsigned long pfn)
> +static void __free_page_cgroup(unsigned long pfn)
> {
> struct mem_section *ms;
> struct page_cgroup *base;
> @@ -187,9 +187,9 @@ void __free_page_cgroup(unsigned long pfn)
> ms->page_cgroup = NULL;
> }
>
> -int __meminit online_page_cgroup(unsigned long start_pfn,
> - unsigned long nr_pages,
> - int nid)
> +static int __meminit online_page_cgroup(unsigned long start_pfn,
> + unsigned long nr_pages,
> + int nid)
> {
> unsigned long start, end, pfn;
> int fail = 0;
> @@ -222,8 +222,8 @@ int __meminit online_page_cgroup(unsigned long start_pfn,
> return -ENOMEM;
> }
>
> -int __meminit offline_page_cgroup(unsigned long start_pfn,
> - unsigned long nr_pages, int nid)
> +static int __meminit offline_page_cgroup(unsigned long start_pfn,
> + unsigned long nr_pages, int nid)
> {
> unsigned long start, end, pfn;
>
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
>
--
Michal Hocko
SUSE Labs