2021-05-20 08:50:55

by Mel Gorman

[permalink] [raw]
Subject: [PATCH 00/14] 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


2021-05-20 08:50:55

by Mel Gorman

[permalink] [raw]
Subject: [PATCH 03/14] mm/page_alloc: Make should_fail_alloc_page a static function should_fail_alloc_page static

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.

Signed-off-by: Mel Gorman <[email protected]>
---
mm/page_alloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index aaa1655cf682..26cc1a4e639b 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -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

2021-05-20 08:51:49

by Mel Gorman

[permalink] [raw]
Subject: [PATCH 06/14] mm/memcontrol.c: Fix kerneldoc comment for mem_cgroup_calculate_protection

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.

Fixes: 45c7f7e1ef17 ("mm, memcg: decouple e{low,min} state mutations from protection checks")
Signed-off-by: Mel Gorman <[email protected]>
---
mm/memcontrol.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 64ada9e650a5..030c1dc131ce 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -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

2021-05-20 08:52:06

by Mel Gorman

[permalink] [raw]
Subject: [PATCH 10/14] mm/z3fold: Add kerneldoc fields for z3fold_pool

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.

Signed-off-by: Mel Gorman <[email protected]>
---
mm/z3fold.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/mm/z3fold.c b/mm/z3fold.c
index 7fe7adaaad01..56321bf37d56 100644
--- a/mm/z3fold.c
+++ b/mm/z3fold.c
@@ -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

2021-05-20 08:52:25

by Mel Gorman

[permalink] [raw]
Subject: [PATCH 11/14] mm/swap: Make swap_address_space an inline function

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.

Signed-off-by: Mel Gorman <[email protected]>
---
include/linux/swap.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index 144727041e78..216462c78a91 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -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

2021-05-20 08:53:23

by Mel Gorman

[permalink] [raw]
Subject: [PATCH 14/14] mm/swap: Make NODE_DATA an inline function on CONFIG_FLATMEM

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.

Signed-off-by: Mel Gorman <[email protected]>
---
include/linux/mmzone.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 0d53eba1c383..8af2f9ff9737 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -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

2021-05-20 08:53:42

by Mel Gorman

[permalink] [raw]
Subject: [PATCH 12/14] mm/mmap_lock: Remove dead code for !CONFIG_TRACING configurations

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.

Signed-off-by: Mel Gorman <[email protected]>
---
mm/mmap_lock.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/mm/mmap_lock.c b/mm/mmap_lock.c
index dcdde4f722a4..03ee85c696ef 100644
--- a/mm/mmap_lock.c
+++ b/mm/mmap_lock.c
@@ -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

2021-05-20 14:38:30

by Mel Gorman

[permalink] [raw]
Subject: [PATCH 02/14] mm/vmalloc: Include header for prototype of set_iounmap_nonlazy

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.

Signed-off-by: Mel Gorman <[email protected]>
---
mm/vmalloc.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index a13ac524f6ff..0522a418b1c5 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -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

2021-05-20 14:40:00

by Mel Gorman

[permalink] [raw]
Subject: [PATCH 04/14] mm/mapping_dirty_helpers: Remove double Note in kerneldoc

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.

Signed-off-by: Mel Gorman <[email protected]>
---
mm/mapping_dirty_helpers.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/mapping_dirty_helpers.c b/mm/mapping_dirty_helpers.c
index b890854ec761..ea734f248fce 100644
--- a/mm/mapping_dirty_helpers.c
+++ b/mm/mapping_dirty_helpers.c
@@ -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

2021-05-20 14:40:06

by Mel Gorman

[permalink] [raw]
Subject: [PATCH 05/14] mm/early_ioremap: Add prototype for early_memremap_pgprot_adjust

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.

Signed-off-by: Mel Gorman <[email protected]>
---
include/asm-generic/early_ioremap.h | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/include/asm-generic/early_ioremap.h b/include/asm-generic/early_ioremap.h
index 9def22e6e2b3..022f8f908b42 100644
--- a/include/asm-generic/early_ioremap.h
+++ b/include/asm-generic/early_ioremap.h
@@ -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

2021-05-20 14:40:08

by Mel Gorman

[permalink] [raw]
Subject: [PATCH 07/14] mm/memory_hotplug: Fix kerneldoc comment for __try_online_node

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.

Fixes: Commit b9ff036082cd ("mm/memory_hotplug.c: make add_memory_resource use __try_online_node")
Signed-off-by: Mel Gorman <[email protected]>
---
mm/memory_hotplug.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 70620d0dd923..e3266be1d020 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -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

2021-05-20 14:40:13

by Mel Gorman

[permalink] [raw]
Subject: [PATCH 08/14] mm/memory_hotplug: Fix kerneldoc comment for __remove_memory

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.

Fixes: eca499ab3749 ("mm/hotplug: make remove_memory() interface usable")
Signed-off-by: Mel Gorman <[email protected]>
---
mm/memory_hotplug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index e3266be1d020..4ea1b19e8c7a 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -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

2021-05-20 14:40:15

by Mel Gorman

[permalink] [raw]
Subject: [PATCH 09/14] mm/zbud: Add kerneldoc fields for zbud_pool

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.

Fixes: 479305fd7172 ("zpool: remove zpool_evict()")
Signed-off-by: Mel Gorman <[email protected]>
---
mm/zbud.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/mm/zbud.c b/mm/zbud.c
index 7ec5f27a68b0..a200121da400 100644
--- a/mm/zbud.c
+++ b/mm/zbud.c
@@ -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

2021-05-20 14:50:59

by Mel Gorman

[permalink] [raw]
Subject: [PATCH 01/14] mm/vmscan: Remove kerneldoc-like comment from isolate_lru_pages

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.

Signed-off-by: Mel Gorman <[email protected]>
---
mm/vmscan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 5199b9696bab..73682ba1f93c 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -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

2021-05-20 14:51:53

by Mel Gorman

[permalink] [raw]
Subject: [PATCH 13/14] mm/page_alloc: Move prototype for find_suitable_fallback

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.

Signed-off-by: Mel Gorman <[email protected]>
---
mm/internal.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/internal.h b/mm/internal.h
index 54bd0dc2c23c..bc9890948712 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -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

2021-05-20 21:11:41

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH 07/14] mm/memory_hotplug: Fix kerneldoc comment for __try_online_node

On 20.05.21 10:48, Mel Gorman wrote:
> 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.
>
> Fixes: Commit b9ff036082cd ("mm/memory_hotplug.c: make add_memory_resource use __try_online_node")
> Signed-off-by: Mel Gorman <[email protected]>
> ---
> mm/memory_hotplug.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 70620d0dd923..e3266be1d020 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -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.
>

Reviewed-by: David Hildenbrand <[email protected]>

--
Thanks,

David / dhildenb

2021-05-20 21:11:53

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH 08/14] mm/memory_hotplug: Fix kerneldoc comment for __remove_memory

On 20.05.21 10:48, Mel Gorman wrote:
> 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.
>
> Fixes: eca499ab3749 ("mm/hotplug: make remove_memory() interface usable")
> Signed-off-by: Mel Gorman <[email protected]>
> ---
> mm/memory_hotplug.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index e3266be1d020..4ea1b19e8c7a 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -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
>

Reviewed-by: David Hildenbrand <[email protected]>

--
Thanks,

David / dhildenb

2021-05-21 08:26:16

by Yang Shi

[permalink] [raw]
Subject: Re: [PATCH 00/14] Clean W=1 build warnings for mm/

On Thu, May 20, 2021 at 1:48 AM Mel Gorman <[email protected]> wrote:
>
> 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(-)

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

>
> --
> 2.26.2
>

2021-05-21 08:40:04

by Chris Down

[permalink] [raw]
Subject: Re: [PATCH 06/14] mm/memcontrol.c: Fix kerneldoc comment for mem_cgroup_calculate_protection

Mel Gorman writes:
>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.
>
>Fixes: 45c7f7e1ef17 ("mm, memcg: decouple e{low,min} state mutations from protection checks")
>Signed-off-by: Mel Gorman <[email protected]>

Whoops, thanks.

Acked-by: Chris Down <[email protected]>

>---
> mm/memcontrol.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>index 64ada9e650a5..030c1dc131ce 100644
>--- a/mm/memcontrol.c
>+++ b/mm/memcontrol.c
>@@ -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
>
>

2021-05-25 15:41:55

by Vlastimil Babka

[permalink] [raw]
Subject: Re: [PATCH 00/14] Clean W=1 build warnings for mm/

On 5/20/21 10:47 AM, Mel Gorman wrote:
> 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(-)

Thanks, looks good.

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

Acked-by: Vlastimil Babka <[email protected]>


2021-05-25 17:06:17

by Mel Gorman

[permalink] [raw]
Subject: Re: [PATCH 00/14] Clean W=1 build warnings for mm/

On Tue, May 25, 2021 at 01:48:26PM +0200, Vlastimil Babka wrote:
> On 5/20/21 10:47 AM, Mel Gorman wrote:
> > 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(-)
>
> 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?

> Acked-by: Vlastimil Babka <[email protected]>
>

Thanks!

--
Mel Gorman
SUSE Labs

2021-07-08 19:22:31

by Matteo Croce

[permalink] [raw]
Subject: Re: [PATCH 03/14] mm/page_alloc: Make should_fail_alloc_page a static function should_fail_alloc_page static

On Thu, 20 May 2021 09:47:58 +0100
Mel Gorman <[email protected]> wrote:

> 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.
>
> Signed-off-by: Mel Gorman <[email protected]>
> ---
> mm/page_alloc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index aaa1655cf682..26cc1a4e639b 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -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);
> }


Hi Mel,

It seems that this breaks builds with CONFIG_DEBUG_INFO_BTF=y.
Maybe that warning was a false positive because
should_fail_alloc_page() is referenced via a macro?

I proposed to revert it, feel free to propose another fix.

Regards,
--
per aspera ad upstream

2021-07-09 09:32:13

by Mel Gorman

[permalink] [raw]
Subject: Re: [PATCH 03/14] mm/page_alloc: Make should_fail_alloc_page a static function should_fail_alloc_page static

On Thu, Jul 08, 2021 at 09:18:44PM +0200, Matteo Croce wrote:
> On Thu, 20 May 2021 09:47:58 +0100
> Mel Gorman <[email protected]> wrote:
>
> > 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.
> >
> > Signed-off-by: Mel Gorman <[email protected]>
> > ---
> > mm/page_alloc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index aaa1655cf682..26cc1a4e639b 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -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);
> > }
>
>
> Hi Mel,
>
> It seems that this breaks builds with CONFIG_DEBUG_INFO_BTF=y.
> Maybe that warning was a false positive because
> should_fail_alloc_page() is referenced via a macro?
>
> I proposed to revert it, feel free to propose another fix.
>

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