Clean W=1 build warnings for mm/

This is a janitorial only. During development of a tool to catch build warnings early to avoid tripping the Intel lkp-robot, I noticed that mm/ is not clean for W=1. This is generally harmless but there is no harm in cleaning it up. It disrupts git blame a little but on relatively obvious lines that are unlikely to be git blame targets.

 include/asm-generic/early_ioremap.h | 9 +++++++++
 include/linux/mmzone.h              | 5 ++++-
 include/linux/swap.h                | 6 +++++-
 mm/internal.h                       | 3 +--
 mm/mapping_dirty_helpers.c          | 2 +-
 mm/memcontrol.c                     | 2 +-
 mm/memory_hotplug.c                 | 6 +++---
 mm/mmap_lock.c                      | 2 ++
 mm/page_alloc.c                     | 2 +-
 mm/vmalloc.c                        | 3 +++
 mm/vmscan.c                         | 2 +-
 mm/z3fold.c                         | 2 ++
 mm/zbud.c                           | 2 ++
 13 files changed, 35 insertions(+), 11 deletions(-)

2.26.2

make W=1 generates the following warning for vmscan.c

mm/vmscan.c:1814: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst

It is not a kerneldoc comment and isolate_lru_pages() is a static function. While the detailed comment is nice, it does not need to be exposed via kernel-doc.

---
 mm/vmscan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 5199b9696bab..73682ba1f93c 100644
@@ -1810,7 +1810,7 @@ static __always_inline void update_lru_sizes(struct lruvec *lruvec,
 
 }
 
-/**
+/*
  * Isolating page from the lruvec to fill in @dst list by nr_to_scan times.
  *
  * lruvec->lru_lock is heavily contended.  Some of the functions that

2.26.2

make W=1 generates the following warning for mm/vmalloc.c

mm/vmalloc.c:1599:6: warning: no previous prototype for ‘set_iounmap_nonlazy’ [-Wmissing-prototypes]
void set_iounmap_nonlazy(void)

        ^~~~~~~~~~~~~~~~~~~

This is an arch-generic function only used by x86. On other arches, it's dead code. Include the header with the definition and make it x86-64 specific.

---
 mm/vmalloc.c | 3 +++
 1 file changed, 3 insertions(+)

index a13ac524f6ff..0522a418b1c5 100644
@@ -36,6 +36,7 @@
 #include <linux/overflow.h>
 #include <linux/pgtable.h>
 #include <linux/uaccess.h>
+#include <linux/io.h>
 #include <asm/tlbflush.h>
 #include <asm/shmparam.h>
 

@@ -1592,6 +1593,7 @@ static DEFINE_MUTEX(vmap_purge_lock);
 /* for per-CPU blocks */
 static void purge_fragmented_blocks_allcpus(void);
 
+#ifdef CONFIG_X86_64
 /*
  * called before a call to iounmap() if the caller wants vm_area_struct's
  * immediately freed.

@@ -1600,6 +1602,7 @@ void set_iounmap_nonlazy(void)
 {
 	atomic_long_set(&vmap_lazy_nr, lazy_max_pages()+1);
 }
+#endif /* CONFIG_X86_64 */
 
 /*
  * Purges all lazily-freed vmap areas.

2.26.2

make W=1 generates the following warning for mm/page_alloc.c

mm/page_alloc.c:3651:15: warning: no previous prototype for ‘should_fail_alloc_page’ [-Wmissing-prototypes]
noinline bool should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)

                 ^~~~~~~~~~~~~~~~~~~~~~

This function is deliberately split out for BPF to allow errors to be injected. The function is not used anywhere else so it is local to the file. Make it static which should still allow error injection to be used similar to how block/blk-core.c:should_fail_bio() works.

---
 mm/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index aaa1655cf682..26cc1a4e639b 100644
@@ -3648,7 +3648,7 @@ static inline bool __should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)
 
 #endif /* CONFIG_FAIL_PAGE_ALLOC */
 
-noinline bool should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)
+static noinline bool should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)
 {
 	return __should_fail_alloc_page(gfp_mask, order);
 }

2.26.2

The alternative fix of making the symbol global was rejected. eBPF needs to figure out a way of instrumenting code that is is unused by the kernel and not globally visible but I don't know how that might be achieved.

Mel Gorman
SUSE Labs

make W=1 generates the following warning for mm/mapping_dirty_helpers.c

mm/mapping_dirty_helpers.c:325: warning: duplicate section name 'Note'

The helper function is very specific to one driver -- vmwgfx. While the two notes are separate, all of it needs to be taken into account when using the helper so make it one note.

---
 mm/mapping_dirty_helpers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index b890854ec761..ea734f248fce 100644
@@ -317,7 +317,7 @@ EXPORT_SYMBOL_GPL(wp_shared_mapping_range);
  * pfn_mkwrite(). And then after a TLB flush following the write-protection
  * pick up all dirty bits.
  *
- * Note: This function currently skips transhuge page-table entries, since
+ * This function currently skips transhuge page-table entries, since
  * it's intended for dirty-tracking on the PTE level. It will warn on
  * encountering transhuge dirty entries, though, and can easily be extended
  * to handle them as well.

2.26.2

make W=1 generates the following warning for mm/early_ioremap.c

mm/early_ioremap.c:34:24: warning: no previous prototype for ‘early_memremap_pgprot_adjust’ [-Wmissing-prototypes]
pgprot_t __init __weak early_memremap_pgprot_adjust(resource_size_t phys_addr,

                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

The weak functions that can be overridden by architectures are declared in include/asm-generic/early_ioremap.h so add the prototype there. The asm/fixmap.h header is need for pgprot_t.

---
 include/asm-generic/early_ioremap.h | 9 +++++++++
 1 file changed, 9 insertions(+)

index 9def22e6e2b3..022f8f908b42 100644
@@ -3,6 +3,7 @@
 #define _ASM_EARLY_IOREMAP_H_
 
 #include <linux/types.h>
+#include <asm/fixmap.h>
 
 /*
  * early_ioremap() and early_iounmap() are for temporary early boot-time

@@ -19,6 +20,14 @@ extern void *early_memremap_prot(resource_size_t phys_addr,
 extern void early_iounmap(void __iomem *addr, unsigned long size);
 extern void early_memunmap(void *addr, unsigned long size);
 
+/*
+ * Weak function called by early_memremap and early_memremap_ro. It does
+ * nothing, but architectures may provide their own version to handle
+ * memory encryption.
+ */
+extern pgprot_t early_memremap_pgprot_adjust(resource_size_t phys_addr,
+					    unsigned long size, pgprot_t prot);
+
 /*
  * Weak function called by early_ioremap_reset(). It does nothing, but
  * architectures may provide their own version to do any needed cleanups.

2.26.2

make W=1 generates the following warning for mem_cgroup_calculate_protection

mm/memcontrol.c:6468: warning: expecting prototype for mem_cgroup_protected(). Prototype was for mem_cgroup_calculate_protection() instead

Commit 45c7f7e1ef17 ("mm, memcg: decouple e{low,min} state mutations from protection checks") changed the function definition but not the associated kerneldoc comment.

---
 mm/memcontrol.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 64ada9e650a5..030c1dc131ce 100644
@@ -6456,7 +6456,7 @@ static unsigned long effective_protection(unsigned long usage,
 }
 
 /**
- * mem_cgroup_protected - check if memory consumption is in the normal range
+ * mem_cgroup_calculate_protection - check if memory consumption is in the normal range
  * @root: the top ancestor of the sub-tree being checked
  * @memcg: the memory cgroup to check
  *

2.26.2

Whoops, thanks.

make W=1 generates the following warning for try_online_node

mm/memory_hotplug.c:1087: warning: expecting prototype for try_online_node(). Prototype was for __try_online_node() instead

Commit b9ff036082cd ("mm/memory_hotplug.c: make add_memory_resource use __try_online_node") renamed the function but did not update the associated kerneldoc. The function is static and somewhat specialised in nature so it's not clear it warrants being a kerneldoc by moving the comment to try_online_node. Hence, leave the comment of the internal helper in place but leave it out of kerneldoc and correct the function name in the comment.

---
 mm/memory_hotplug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 70620d0dd923..e3266be1d020 100644
@@ -1072,8 +1072,8 @@ static void rollback_node_hotadd(int nid)
 }
 
 
-/**
- * try_online_node - online a node if offlined
+/*
+ * __try_online_node - online a node if offlined
  * @nid: the node ID
  * @set_node_online: Whether we want to online the node
  * called by cpu_up() to online a node without onlined memory.

2.26.2

make W=1 generates the following warning for __remove_memory

mm/memory_hotplug.c:2044: warning: expecting prototype for remove_memory(). Prototype was for __remove_memory() instead

Commit eca499ab3749 ("mm/hotplug: make remove_memory() interface usable") introduced the kerneldoc comment and function but the kerneldoc name and function name did not match.

---
 mm/memory_hotplug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index e3266be1d020..4ea1b19e8c7a 100644
@@ -2031,7 +2031,7 @@ static int __ref try_remove_memory(int nid, u64 start, u64 size)
 }
 
 /**
- * remove_memory
+ * __remove_memory - Remove memory if every memory block is offline
  * @nid: the node ID
  * @start: physical address of the region to remove
  * @size: size of the region to remove

2.26.2

make W=1 generates the following warning for zbud_pool

mm/zbud.c:105: warning: Function parameter or member 'zpool' not described in 'zbud_pool' mm/zbud.c:105: warning: Function parameter or member 'zpool_ops' not described in 'zbud_pool'

Commit 479305fd7172 ("zpool: remove zpool_evict()") removed the zpool_evict helper and added the associated zpool and operations structure in struct zbud_pool but did not add documentation for the fields. Add rudimentary documentation.

---
 mm/zbud.c | 2 ++
 1 file changed, 2 insertions(+)

index 7ec5f27a68b0..a200121da400 100644
@@ -87,6 +87,8 @@
  * @pages_nr:	number of zbud pages in the pool.
  * @ops:	pointer to a structure of user defined operations specified at
  *		pool creation time.
+ * @zpool:	zpool driver
+ * @zpool_ops:	zpool operations structure with an evict callback
  *
  * This structure is allocated at pool creation time and maintains metadata
  * pertaining to a particular zbud pool.

2.26.2

make W=1 generates the following warning for z3fold_pool

mm/z3fold.c:171: warning: Function parameter or member 'zpool' not described in 'z3fold_pool' mm/z3fold.c:171: warning: Function parameter or member 'zpool_ops' not described in 'z3fold_pool'

Commit 9a001fc19ccc ("z3fold: the 3-fold allocator for compressed pages") simply did not document the fields at the time. Add rudimentary documentation.

---
 mm/z3fold.c | 2 ++
 1 file changed, 2 insertions(+)

index 7fe7adaaad01..56321bf37d56 100644
@@ -144,6 +144,8 @@ struct z3fold_header {
  * @c_handle:	cache for z3fold_buddy_slots allocation
  * @ops:	pointer to a structure of user defined operations specified at
  *		pool creation time.
+ * @zpool:	zpool driver
+ * @zpool_ops:	zpool operations structure with an evict callback
  * @compact_wq:	workqueue for page layout background optimization
  * @release_wq:	workqueue for safe page release
  * @work:	work_struct for safe page release

2.26.2

make W=1 generates the following warning in page_mapping() for allnoconfig

mm/util.c:700:15: warning: variable ‘entry’ set but not used [-Wunused-but-set-variable]

     swp_entry_t entry;
                 ^~~~~

swap_address is a #define on !CONFIG_SWAP configurations. Make the helper an inline function to suppress the warning, add type checking and to apply any side-effects in the parameter list.

---
 include/linux/swap.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

index 144727041e78..216462c78a91 100644
@@ -526,7 +526,11 @@ static inline struct swap_info_struct *swp_swap_info(swp_entry_t entry)
 	return NULL;
 }
 
-#define swap_address_space(entry)		(NULL)
+static inline struct address_space *swap_address_space(swp_entry_t entry)
+{
+	return NULL;
+}
+
 #define get_nr_swap_pages()			0L
 #define total_swap_pages			0L
 #define total_swapcache_pages()			0UL

2.26.2

make W=1 generates the following warning in mmap_lock.c for allnoconfig

mm/mmap_lock.c:213:6: warning: no previous prototype for ‘__mmap_lock_do_trace_start_locking’ [-Wmissing-prototypes]
void __mmap_lock_do_trace_start_locking(struct mm_struct *mm, bool write)

        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  mm/mmap_lock.c:219:6: warning: no previous prototype for ‘__mmap_lock_do_trace_acquire_returned’ [-Wmissing-prototypes]
   void __mmap_lock_do_trace_acquire_returned(struct mm_struct *mm, bool write,
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

mm/mmap_lock.c:226:6: warning: no previous prototype for ‘__mmap_lock_do_trace_released’ [-Wmissing-prototypes]
void __mmap_lock_do_trace_released(struct mm_struct *mm, bool write)

On !CONFIG_TRACING configurations, the code is dead so put it behind an #ifdef.

---
 mm/mmap_lock.c | 2 ++
 1 file changed, 2 insertions(+)

index dcdde4f722a4..03ee85c696ef 100644
@@ -205,6 +205,7 @@ void trace_mmap_lock_unreg(void)
 
 #endif /* CONFIG_MEMCG */
 
+#ifdef CONFIG_TRACING
 /*
  * Trace calls must be in a separate file, as otherwise there's a circular
  * dependency between linux/mmap_lock.h and trace/events/mmap_lock.h.

@@ -228,3 +229,4 @@ void __mmap_lock_do_trace_released(struct mm_struct *mm, bool write)
 	TRACE_MMAP_LOCK_EVENT(released, mm, write);
 }
 EXPORT_SYMBOL(__mmap_lock_do_trace_released);
+#endif /* CONFIG_TRACING */

2.26.2

make W=1 generates the following warning in mmap_lock.c for allnoconfig

mm/page_alloc.c:2670:5: warning: no previous prototype for ‘find_suitable_fallback’ [-Wmissing-prototypes]
int find_suitable_fallback(struct free_area *area, unsigned int order,

       ^~~~~~~~~~~~~~~~~~~~~~

find_suitable_fallback is only shared outside of page_alloc.c for CONFIG_COMPACTION but to suppress the warning, move the protype outside of CONFIG_COMPACTION. It is not worth the effort at this time to find a clever way of allowing compaction.c to share the code or avoid the use entirely as the function is called on relatively slow paths.

---
 mm/internal.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

index 54bd0dc2c23c..bc9890948712 100644
@@ -289,11 +289,10 @@ isolate_freepages_range(struct compact_control *cc,
 int
 isolate_migratepages_range(struct compact_control *cc,
 			   unsigned long low_pfn, unsigned long end_pfn);
+#endif
 int find_suitable_fallback(struct free_area *area, unsigned int order,
 			int migratetype, bool only_stealable, bool *can_steal);
 
-#endif
-
 /*
  * This function returns the order of a free page in the buddy system. In
  * general, page_zone(page)->lock must be held by the caller to prevent the

2.26.2

make W=1 generates the following warning in mm/workingset.c for allnoconfig

mm/workingset.c: In function ‘unpack_shadow’:
mm/workingset.c:201:15: warning: variable ‘nid’ set but not used [-Wunused-but-set-variable]

    int memcgid, nid;
                 ^~~

On FLATMEM, NODE_DATA returns a global pglist_data without dereferencing nid. Make the helper an inline function to suppress the warning, add type checking and to apply any side-effects in the parameter list.

---
 include/linux/mmzone.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

index 0d53eba1c383..8af2f9ff9737 100644
@@ -1044,7 +1044,10 @@ extern char numa_zonelist_order[];
 #ifndef CONFIG_NEED_MULTIPLE_NODES
 
 extern struct pglist_data contig_page_data;
-#define NODE_DATA(nid)		(&contig_page_data)
+static inline struct pglist_data *NODE_DATA(int nid)
+{
+	return &contig_page_data;
+}
 #define NODE_MEM_MAP(nid)	mem_map
 
 #else /* CONFIG_NEED_MULTIPLE_NODES */

2.26.2

The whole series looks good to me. Reviewed-by: Yang Shi <[email protected]>

Thanks, looks good.

patch 3/14 subject looks like it should read just "mm/page_alloc: Make should_fail_alloc_page a static function"

Yes it should. Andrew, do you mind fixing it directly instead of resending the entire series?

Thanks!

Mel Gorman
SUSE Labs