[PATCH 0/3] mm/slub: Fix unused function warnings

[PATCH 0/3] mm/slub: Fix unused function warnings

[Matthias Kaehlcke]

This series fixes a bunch of warnings about unused functions in SLUB

Matthias Kaehlcke (3):
  mm/slub: Only define kmalloc_large_node_hook() for NUMA systems
  mm/slub: Mark slab_free_hook() as __maybe_unused
  mm/slub: Put tid_to_cpu() and tid_to_event() inside #ifdef block

 mm/slub.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

-- 
2.13.0.303.g4ebf302169-goog

[PATCH 0/3] mm/slub: Fix unused function warnings

[Matthias Kaehlcke]

This series fixes a bunch of warnings about unused functions in SLUB

Matthias Kaehlcke (3):
  mm/slub: Only define kmalloc_large_node_hook() for NUMA systems
  mm/slub: Mark slab_free_hook() as __maybe_unused
  mm/slub: Put tid_to_cpu() and tid_to_event() inside #ifdef block

 mm/slub.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

-- 
2.13.0.303.g4ebf302169-goog

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[Matthias Kaehlcke]

The function is only used when CONFIG_NUMA=y. Placing it in an #ifdef
block fixes the following warning when building with clang:

mm/slub.c:1246:20: error: unused function 'kmalloc_large_node_hook'
    [-Werror,-Wunused-function]

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 mm/slub.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/slub.c b/mm/slub.c
index 57e5156f02be..66e1046435b7 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1313,11 +1313,14 @@ static inline void dec_slabs_node(struct kmem_cache *s, int node,
  * Hooks for other subsystems that check memory allocations. In a typical
  * production configuration these hooks all should produce no code at all.
  */
+
+#ifdef CONFIG_NUMA
 static inline void kmalloc_large_node_hook(void *ptr, size_t size, gfp_t flags)
 {
 	kmemleak_alloc(ptr, size, 1, flags);
 	kasan_kmalloc_large(ptr, size, flags);
 }
+#endif
 
 static inline void kfree_hook(const void *x)
 {
-- 
2.13.0.303.g4ebf302169-goog

[PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[Matthias Kaehlcke]

The function is only used when CONFIG_NUMA=y. Placing it in an #ifdef
block fixes the following warning when building with clang:

mm/slub.c:1246:20: error: unused function 'kmalloc_large_node_hook'
    [-Werror,-Wunused-function]

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 mm/slub.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/slub.c b/mm/slub.c
index 57e5156f02be..66e1046435b7 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1313,11 +1313,14 @@ static inline void dec_slabs_node(struct kmem_cache *s, int node,
  * Hooks for other subsystems that check memory allocations. In a typical
  * production configuration these hooks all should produce no code at all.
  */
+
+#ifdef CONFIG_NUMA
 static inline void kmalloc_large_node_hook(void *ptr, size_t size, gfp_t flags)
 {
 	kmemleak_alloc(ptr, size, 1, flags);
 	kasan_kmalloc_large(ptr, size, flags);
 }
+#endif
 
 static inline void kfree_hook(const void *x)
 {
-- 
2.13.0.303.g4ebf302169-goog

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/3] mm/slub: Mark slab_free_hook() as __maybe_unused

[Matthias Kaehlcke]

The function is only used when certain configuration option are enabled.
Adding the attribute fixes the following warning when building with
clang:

mm/slub.c:1258:20: error: unused function 'slab_free_hook'
    [-Werror,-Wunused-function]

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 mm/slub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/slub.c b/mm/slub.c
index 66e1046435b7..23a8eb83efff 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1328,7 +1328,7 @@ static inline void kfree_hook(const void *x)
 	kasan_kfree_large(x);
 }
 
-static inline void *slab_free_hook(struct kmem_cache *s, void *x)
+static inline void *__maybe_unused slab_free_hook(struct kmem_cache *s, void *x)
 {
 	void *freeptr;
 
-- 
2.13.0.303.g4ebf302169-goog

[PATCH 2/3] mm/slub: Mark slab_free_hook() as __maybe_unused

[Matthias Kaehlcke]

The function is only used when certain configuration option are enabled.
Adding the attribute fixes the following warning when building with
clang:

mm/slub.c:1258:20: error: unused function 'slab_free_hook'
    [-Werror,-Wunused-function]

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 mm/slub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/slub.c b/mm/slub.c
index 66e1046435b7..23a8eb83efff 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1328,7 +1328,7 @@ static inline void kfree_hook(const void *x)
 	kasan_kfree_large(x);
 }
 
-static inline void *slab_free_hook(struct kmem_cache *s, void *x)
+static inline void *__maybe_unused slab_free_hook(struct kmem_cache *s, void *x)
 {
 	void *freeptr;
 
-- 
2.13.0.303.g4ebf302169-goog

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 3/3] mm/slub: Put tid_to_cpu() and tid_to_event() inside #ifdef block

[Matthias Kaehlcke]

The functions are only used when certain config options are set. Putting
them inside #ifdef fixes the following warnings when building with clang:

mm/slub.c:1759:28: error: unused function 'tid_to_cpu'
    [-Werror,-Wunused-function]
                           ^
mm/slub.c:1764:29: error: unused function 'tid_to_event'
    [-Werror,-Wunused-function]

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 mm/slub.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/slub.c b/mm/slub.c
index 23a8eb83efff..6df95738420d 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1945,15 +1945,19 @@ static inline unsigned long next_tid(unsigned long tid)
 	return tid + TID_STEP;
 }
 
+#ifdef SLUB_DEBUG_CMPXCHG
+#ifdef CONFIG_PREEMPT
 static inline unsigned int tid_to_cpu(unsigned long tid)
 {
 	return tid % TID_STEP;
 }
+#endif
 
 static inline unsigned long tid_to_event(unsigned long tid)
 {
 	return tid / TID_STEP;
 }
+#endif
 
 static inline unsigned int init_tid(int cpu)
 {
-- 
2.13.0.303.g4ebf302169-goog

[PATCH 3/3] mm/slub: Put tid_to_cpu() and tid_to_event() inside #ifdef block

[Matthias Kaehlcke]

The functions are only used when certain config options are set. Putting
them inside #ifdef fixes the following warnings when building with clang:

mm/slub.c:1759:28: error: unused function 'tid_to_cpu'
    [-Werror,-Wunused-function]
                           ^
mm/slub.c:1764:29: error: unused function 'tid_to_event'
    [-Werror,-Wunused-function]

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 mm/slub.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/slub.c b/mm/slub.c
index 23a8eb83efff..6df95738420d 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1945,15 +1945,19 @@ static inline unsigned long next_tid(unsigned long tid)
 	return tid + TID_STEP;
 }
 
+#ifdef SLUB_DEBUG_CMPXCHG
+#ifdef CONFIG_PREEMPT
 static inline unsigned int tid_to_cpu(unsigned long tid)
 {
 	return tid % TID_STEP;
 }
+#endif
 
 static inline unsigned long tid_to_event(unsigned long tid)
 {
 	return tid / TID_STEP;
 }
+#endif
 
 static inline unsigned int init_tid(int cpu)
 {
-- 
2.13.0.303.g4ebf302169-goog

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/3] mm/slub: Fix unused function warnings

[Christoph Lameter]

On Fri, 19 May 2017, Matthias Kaehlcke wrote:

> This series fixes a bunch of warnings about unused functions in SLUB

Looks ok to me.

Acked-by: Christoph Lameter <cl@linux.com>

Re: [PATCH 0/3] mm/slub: Fix unused function warnings

[Christoph Lameter]

On Fri, 19 May 2017, Matthias Kaehlcke wrote:

> This series fixes a bunch of warnings about unused functions in SLUB

Looks ok to me.

Acked-by: Christoph Lameter <cl@linux.com>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[David Rientjes]

On Fri, 19 May 2017, Matthias Kaehlcke wrote:

> The function is only used when CONFIG_NUMA=y. Placing it in an #ifdef
> block fixes the following warning when building with clang:
> 
> mm/slub.c:1246:20: error: unused function 'kmalloc_large_node_hook'
>     [-Werror,-Wunused-function]
> 

Is clang not inlining kmalloc_large_node_hook() for some reason?  I don't 
think this should ever warn on gcc.

> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>

Acked-by: David Rientjes <rientjes@google.com>

> ---
>  mm/slub.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 57e5156f02be..66e1046435b7 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1313,11 +1313,14 @@ static inline void dec_slabs_node(struct kmem_cache *s, int node,
>   * Hooks for other subsystems that check memory allocations. In a typical
>   * production configuration these hooks all should produce no code at all.
>   */
> +
> +#ifdef CONFIG_NUMA
>  static inline void kmalloc_large_node_hook(void *ptr, size_t size, gfp_t flags)
>  {
>  	kmemleak_alloc(ptr, size, 1, flags);
>  	kasan_kmalloc_large(ptr, size, flags);
>  }
> +#endif
>  
>  static inline void kfree_hook(const void *x)
>  {

Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[David Rientjes]

On Fri, 19 May 2017, Matthias Kaehlcke wrote:

> The function is only used when CONFIG_NUMA=y. Placing it in an #ifdef
> block fixes the following warning when building with clang:
> 
> mm/slub.c:1246:20: error: unused function 'kmalloc_large_node_hook'
>     [-Werror,-Wunused-function]
> 

Is clang not inlining kmalloc_large_node_hook() for some reason?  I don't 
think this should ever warn on gcc.

> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>

Acked-by: David Rientjes <rientjes@google.com>

> ---
>  mm/slub.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 57e5156f02be..66e1046435b7 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1313,11 +1313,14 @@ static inline void dec_slabs_node(struct kmem_cache *s, int node,
>   * Hooks for other subsystems that check memory allocations. In a typical
>   * production configuration these hooks all should produce no code at all.
>   */
> +
> +#ifdef CONFIG_NUMA
>  static inline void kmalloc_large_node_hook(void *ptr, size_t size, gfp_t flags)
>  {
>  	kmemleak_alloc(ptr, size, 1, flags);
>  	kasan_kmalloc_large(ptr, size, flags);
>  }
> +#endif
>  
>  static inline void kfree_hook(const void *x)
>  {

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[Matthias Kaehlcke]

El Mon, May 22, 2017 at 01:39:26PM -0700 David Rientjes ha dit:

> On Fri, 19 May 2017, Matthias Kaehlcke wrote:
> 
> > The function is only used when CONFIG_NUMA=y. Placing it in an #ifdef
> > block fixes the following warning when building with clang:
> > 
> > mm/slub.c:1246:20: error: unused function 'kmalloc_large_node_hook'
> >     [-Werror,-Wunused-function]
> > 
> 
> Is clang not inlining kmalloc_large_node_hook() for some reason?  I don't 
> think this should ever warn on gcc.

clang warns about unused static inline functions outside of header
files, in difference to gcc.

> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> 
> Acked-by: David Rientjes <rientjes@google.com>
> 
> > ---
> >  mm/slub.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 57e5156f02be..66e1046435b7 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -1313,11 +1313,14 @@ static inline void dec_slabs_node(struct kmem_cache *s, int node,
> >   * Hooks for other subsystems that check memory allocations. In a typical
> >   * production configuration these hooks all should produce no code at all.
> >   */
> > +
> > +#ifdef CONFIG_NUMA
> >  static inline void kmalloc_large_node_hook(void *ptr, size_t size, gfp_t flags)
> >  {
> >  	kmemleak_alloc(ptr, size, 1, flags);
> >  	kasan_kmalloc_large(ptr, size, flags);
> >  }
> > +#endif
> >  
> >  static inline void kfree_hook(const void *x)
> >  {

Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[Matthias Kaehlcke]

El Mon, May 22, 2017 at 01:39:26PM -0700 David Rientjes ha dit:

> On Fri, 19 May 2017, Matthias Kaehlcke wrote:
> 
> > The function is only used when CONFIG_NUMA=y. Placing it in an #ifdef
> > block fixes the following warning when building with clang:
> > 
> > mm/slub.c:1246:20: error: unused function 'kmalloc_large_node_hook'
> >     [-Werror,-Wunused-function]
> > 
> 
> Is clang not inlining kmalloc_large_node_hook() for some reason?  I don't 
> think this should ever warn on gcc.

clang warns about unused static inline functions outside of header
files, in difference to gcc.

> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> 
> Acked-by: David Rientjes <rientjes@google.com>
> 
> > ---
> >  mm/slub.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 57e5156f02be..66e1046435b7 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -1313,11 +1313,14 @@ static inline void dec_slabs_node(struct kmem_cache *s, int node,
> >   * Hooks for other subsystems that check memory allocations. In a typical
> >   * production configuration these hooks all should produce no code at all.
> >   */
> > +
> > +#ifdef CONFIG_NUMA
> >  static inline void kmalloc_large_node_hook(void *ptr, size_t size, gfp_t flags)
> >  {
> >  	kmemleak_alloc(ptr, size, 1, flags);
> >  	kasan_kmalloc_large(ptr, size, flags);
> >  }
> > +#endif
> >  
> >  static inline void kfree_hook(const void *x)
> >  {

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[Andrew Morton]

On Mon, 22 May 2017 13:56:21 -0700 Matthias Kaehlcke <mka@chromium.org> wrote:

> El Mon, May 22, 2017 at 01:39:26PM -0700 David Rientjes ha dit:
> 
> > On Fri, 19 May 2017, Matthias Kaehlcke wrote:
> > 
> > > The function is only used when CONFIG_NUMA=y. Placing it in an #ifdef
> > > block fixes the following warning when building with clang:
> > > 
> > > mm/slub.c:1246:20: error: unused function 'kmalloc_large_node_hook'
> > >     [-Werror,-Wunused-function]
> > > 
> > 
> > Is clang not inlining kmalloc_large_node_hook() for some reason?  I don't 
> > think this should ever warn on gcc.
> 
> clang warns about unused static inline functions outside of header
> files, in difference to gcc.

I wish it wouldn't.  These patches just add clutter.

Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[Andrew Morton]

On Mon, 22 May 2017 13:56:21 -0700 Matthias Kaehlcke <mka@chromium.org> wrote:

> El Mon, May 22, 2017 at 01:39:26PM -0700 David Rientjes ha dit:
> 
> > On Fri, 19 May 2017, Matthias Kaehlcke wrote:
> > 
> > > The function is only used when CONFIG_NUMA=y. Placing it in an #ifdef
> > > block fixes the following warning when building with clang:
> > > 
> > > mm/slub.c:1246:20: error: unused function 'kmalloc_large_node_hook'
> > >     [-Werror,-Wunused-function]
> > > 
> > 
> > Is clang not inlining kmalloc_large_node_hook() for some reason?  I don't 
> > think this should ever warn on gcc.
> 
> clang warns about unused static inline functions outside of header
> files, in difference to gcc.

I wish it wouldn't.  These patches just add clutter.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[David Rientjes]

On Mon, 22 May 2017, Andrew Morton wrote:

> > > Is clang not inlining kmalloc_large_node_hook() for some reason?  I don't 
> > > think this should ever warn on gcc.
> > 
> > clang warns about unused static inline functions outside of header
> > files, in difference to gcc.
> 
> I wish it wouldn't.  These patches just add clutter.
> 

Matthias, what breaks if you do this?

diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index de179993e039..e1895ce6fa1b 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -15,3 +15,8 @@
  * with any version that can compile the kernel
  */
 #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
+
+#ifdef inline
+#undef inline
+#define inline __attribute__((unused))
+#endif

Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[David Rientjes]

On Mon, 22 May 2017, Andrew Morton wrote:

> > > Is clang not inlining kmalloc_large_node_hook() for some reason?  I don't 
> > > think this should ever warn on gcc.
> > 
> > clang warns about unused static inline functions outside of header
> > files, in difference to gcc.
> 
> I wish it wouldn't.  These patches just add clutter.
> 

Matthias, what breaks if you do this?

diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index de179993e039..e1895ce6fa1b 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -15,3 +15,8 @@
  * with any version that can compile the kernel
  */
 #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
+
+#ifdef inline
+#undef inline
+#define inline __attribute__((unused))
+#endif

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[Matthias Kaehlcke]

Hi David,

El Mon, May 22, 2017 at 06:35:23PM -0700 David Rientjes ha dit:

> On Mon, 22 May 2017, Andrew Morton wrote:
> 
> > > > Is clang not inlining kmalloc_large_node_hook() for some reason?  I don't 
> > > > think this should ever warn on gcc.
> > > 
> > > clang warns about unused static inline functions outside of header
> > > files, in difference to gcc.
> > 
> > I wish it wouldn't.  These patches just add clutter.
> > 
> 
> Matthias, what breaks if you do this?
> 
> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> index de179993e039..e1895ce6fa1b 100644
> --- a/include/linux/compiler-clang.h
> +++ b/include/linux/compiler-clang.h
> @@ -15,3 +15,8 @@
>   * with any version that can compile the kernel
>   */
>  #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
> +
> +#ifdef inline
> +#undef inline
> +#define inline __attribute__((unused))
> +#endif

Thanks for the suggestion!

Nothing breaks and the warnings are silenced. It seems we could use
this if there is a stong opposition against having warnings on unused
static inline functions in .c files.

Still I am not convinced that gcc's behavior is preferable in this
case. True, it saves us from adding a bunch of __maybe_unused or
#ifdefs, on the other hand the warning is a useful tool to spot truly
unused code. So far about 50% of the warnings I looked into fall into
this category.

Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[Matthias Kaehlcke]

Hi David,

El Mon, May 22, 2017 at 06:35:23PM -0700 David Rientjes ha dit:

> On Mon, 22 May 2017, Andrew Morton wrote:
> 
> > > > Is clang not inlining kmalloc_large_node_hook() for some reason?  I don't 
> > > > think this should ever warn on gcc.
> > > 
> > > clang warns about unused static inline functions outside of header
> > > files, in difference to gcc.
> > 
> > I wish it wouldn't.  These patches just add clutter.
> > 
> 
> Matthias, what breaks if you do this?
> 
> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> index de179993e039..e1895ce6fa1b 100644
> --- a/include/linux/compiler-clang.h
> +++ b/include/linux/compiler-clang.h
> @@ -15,3 +15,8 @@
>   * with any version that can compile the kernel
>   */
>  #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
> +
> +#ifdef inline
> +#undef inline
> +#define inline __attribute__((unused))
> +#endif

Thanks for the suggestion!

Nothing breaks and the warnings are silenced. It seems we could use
this if there is a stong opposition against having warnings on unused
static inline functions in .c files.

Still I am not convinced that gcc's behavior is preferable in this
case. True, it saves us from adding a bunch of __maybe_unused or
#ifdefs, on the other hand the warning is a useful tool to spot truly
unused code. So far about 50% of the warnings I looked into fall into
this category.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[Doug Anderson]

Hi,

On Tue, May 23, 2017 at 9:56 AM, Matthias Kaehlcke <mka@chromium.org> wrote:
> Hi David,
>
> El Mon, May 22, 2017 at 06:35:23PM -0700 David Rientjes ha dit:
>
>> On Mon, 22 May 2017, Andrew Morton wrote:
>>
>> > > > Is clang not inlining kmalloc_large_node_hook() for some reason?  I don't
>> > > > think this should ever warn on gcc.
>> > >
>> > > clang warns about unused static inline functions outside of header
>> > > files, in difference to gcc.
>> >
>> > I wish it wouldn't.  These patches just add clutter.
>> >
>>
>> Matthias, what breaks if you do this?
>>
>> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
>> index de179993e039..e1895ce6fa1b 100644
>> --- a/include/linux/compiler-clang.h
>> +++ b/include/linux/compiler-clang.h
>> @@ -15,3 +15,8 @@
>>   * with any version that can compile the kernel
>>   */
>>  #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
>> +
>> +#ifdef inline
>> +#undef inline
>> +#define inline __attribute__((unused))
>> +#endif
>
> Thanks for the suggestion!

Wow, I totally didn't think of this either when talking with Matthias
about this.  I guess it makes the assumption that nobody else is
hacking "inline" like we are, but "what are the chances of someone
doing that (TM)"  ;-)


> Nothing breaks and the warnings are silenced. It seems we could use
> this if there is a stong opposition against having warnings on unused
> static inline functions in .c files.
>
> Still I am not convinced that gcc's behavior is preferable in this
> case. True, it saves us from adding a bunch of __maybe_unused or
> #ifdefs, on the other hand the warning is a useful tool to spot truly
> unused code. So far about 50% of the warnings I looked into fall into
> this category.

Personally I actually prefer clang's behavior to gcc's here and I
think Matthias's patches are actually doing a service to the
community.  As he points out, many of the patches he's posted have
removed totally dead code from the kernel.  This code has often
existed for many years but never been noticed.  While the code wasn't
hurting anyone (it was dead and the compiler wasn't generating any
code for it), it would certainly add confusion to anyone reading the
source code.

Obviously my opinion isn't as important as the opinion of upstream
maintainers, but hopefully you'll consider it anyway.  :)  If you
still want to just make clang behave like gcc then I think David's
suggestion is a great one.


-Doug

Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[Doug Anderson]

Hi,

On Tue, May 23, 2017 at 9:56 AM, Matthias Kaehlcke <mka@chromium.org> wrote:
> Hi David,
>
> El Mon, May 22, 2017 at 06:35:23PM -0700 David Rientjes ha dit:
>
>> On Mon, 22 May 2017, Andrew Morton wrote:
>>
>> > > > Is clang not inlining kmalloc_large_node_hook() for some reason?  I don't
>> > > > think this should ever warn on gcc.
>> > >
>> > > clang warns about unused static inline functions outside of header
>> > > files, in difference to gcc.
>> >
>> > I wish it wouldn't.  These patches just add clutter.
>> >
>>
>> Matthias, what breaks if you do this?
>>
>> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
>> index de179993e039..e1895ce6fa1b 100644
>> --- a/include/linux/compiler-clang.h
>> +++ b/include/linux/compiler-clang.h
>> @@ -15,3 +15,8 @@
>>   * with any version that can compile the kernel
>>   */
>>  #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
>> +
>> +#ifdef inline
>> +#undef inline
>> +#define inline __attribute__((unused))
>> +#endif
>
> Thanks for the suggestion!

Wow, I totally didn't think of this either when talking with Matthias
about this.  I guess it makes the assumption that nobody else is
hacking "inline" like we are, but "what are the chances of someone
doing that (TM)"  ;-)


> Nothing breaks and the warnings are silenced. It seems we could use
> this if there is a stong opposition against having warnings on unused
> static inline functions in .c files.
>
> Still I am not convinced that gcc's behavior is preferable in this
> case. True, it saves us from adding a bunch of __maybe_unused or
> #ifdefs, on the other hand the warning is a useful tool to spot truly
> unused code. So far about 50% of the warnings I looked into fall into
> this category.

Personally I actually prefer clang's behavior to gcc's here and I
think Matthias's patches are actually doing a service to the
community.  As he points out, many of the patches he's posted have
removed totally dead code from the kernel.  This code has often
existed for many years but never been noticed.  While the code wasn't
hurting anyone (it was dead and the compiler wasn't generating any
code for it), it would certainly add confusion to anyone reading the
source code.

Obviously my opinion isn't as important as the opinion of upstream
maintainers, but hopefully you'll consider it anyway.  :)  If you
still want to just make clang behave like gcc then I think David's
suggestion is a great one.


-Doug

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[David Rientjes]

On Tue, 23 May 2017, Matthias Kaehlcke wrote:

> > diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> > index de179993e039..e1895ce6fa1b 100644
> > --- a/include/linux/compiler-clang.h
> > +++ b/include/linux/compiler-clang.h
> > @@ -15,3 +15,8 @@
> >   * with any version that can compile the kernel
> >   */
> >  #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
> > +
> > +#ifdef inline
> > +#undef inline
> > +#define inline __attribute__((unused))
> > +#endif
> 
> Thanks for the suggestion!
> 
> Nothing breaks and the warnings are silenced. It seems we could use
> this if there is a stong opposition against having warnings on unused
> static inline functions in .c files.
> 

It would be slightly different, it would be:

#define inline inline __attribute__((unused))

to still inline the functions, I was just seeing if there was anything 
else that clang was warning about that was unrelated to a function's 
inlining.

> Still I am not convinced that gcc's behavior is preferable in this
> case. True, it saves us from adding a bunch of __maybe_unused or
> #ifdefs, on the other hand the warning is a useful tool to spot truly
> unused code. So far about 50% of the warnings I looked into fall into
> this category.
> 

I think gcc's behavior is a result of how it does preprocessing and is a 
clearly defined and long-standing semantic given in the gcc manual 
regarding -Wunused-function.

#define IS_PAGE_ALIGNED(__size)	(!(__size & ((size_t)PAGE_SIZE - 1)))
static inline int is_page_aligned(size_t size)
{
	return !(size & ((size_t)PAGE_SIZE - 1));
}

Gcc will not warn about either of these being unused, regardless of -Wall, 
-Wunused-function, or -pedantic.  Clang, correct me if I'm wrong, will 
only warn about is_page_aligned().

So the argument could be made that one of the additional benefits of 
static inline functions is that a subset of compilers, heavily in the 
minority, will detect whether it's unused and we'll get patches that 
remove them.  Functionally, it would only result in LOC reduction.  But, 
isn't adding #ifdef's to silence the warning just adding more LOC?

I have no preference either way, I think it would be up to the person who 
is maintaining the code and has to deal with the patches.

Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[David Rientjes]

On Tue, 23 May 2017, Matthias Kaehlcke wrote:

> > diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> > index de179993e039..e1895ce6fa1b 100644
> > --- a/include/linux/compiler-clang.h
> > +++ b/include/linux/compiler-clang.h
> > @@ -15,3 +15,8 @@
> >   * with any version that can compile the kernel
> >   */
> >  #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
> > +
> > +#ifdef inline
> > +#undef inline
> > +#define inline __attribute__((unused))
> > +#endif
> 
> Thanks for the suggestion!
> 
> Nothing breaks and the warnings are silenced. It seems we could use
> this if there is a stong opposition against having warnings on unused
> static inline functions in .c files.
> 

It would be slightly different, it would be:

#define inline inline __attribute__((unused))

to still inline the functions, I was just seeing if there was anything 
else that clang was warning about that was unrelated to a function's 
inlining.

> Still I am not convinced that gcc's behavior is preferable in this
> case. True, it saves us from adding a bunch of __maybe_unused or
> #ifdefs, on the other hand the warning is a useful tool to spot truly
> unused code. So far about 50% of the warnings I looked into fall into
> this category.
> 

I think gcc's behavior is a result of how it does preprocessing and is a 
clearly defined and long-standing semantic given in the gcc manual 
regarding -Wunused-function.

#define IS_PAGE_ALIGNED(__size)	(!(__size & ((size_t)PAGE_SIZE - 1)))
static inline int is_page_aligned(size_t size)
{
	return !(size & ((size_t)PAGE_SIZE - 1));
}

Gcc will not warn about either of these being unused, regardless of -Wall, 
-Wunused-function, or -pedantic.  Clang, correct me if I'm wrong, will 
only warn about is_page_aligned().

So the argument could be made that one of the additional benefits of 
static inline functions is that a subset of compilers, heavily in the 
minority, will detect whether it's unused and we'll get patches that 
remove them.  Functionally, it would only result in LOC reduction.  But, 
isn't adding #ifdef's to silence the warning just adding more LOC?

I have no preference either way, I think it would be up to the person who 
is maintaining the code and has to deal with the patches.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[Matthias Kaehlcke]

Hi David,

El Wed, May 24, 2017 at 01:36:21PM -0700 David Rientjes ha dit:

> On Tue, 23 May 2017, Matthias Kaehlcke wrote:
> 
> > > diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> > > index de179993e039..e1895ce6fa1b 100644
> > > --- a/include/linux/compiler-clang.h
> > > +++ b/include/linux/compiler-clang.h
> > > @@ -15,3 +15,8 @@
> > >   * with any version that can compile the kernel
> > >   */
> > >  #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
> > > +
> > > +#ifdef inline
> > > +#undef inline
> > > +#define inline __attribute__((unused))
> > > +#endif
> > 
> > Thanks for the suggestion!
> > 
> > Nothing breaks and the warnings are silenced. It seems we could use
> > this if there is a stong opposition against having warnings on unused
> > static inline functions in .c files.
> > 
> 
> It would be slightly different, it would be:
> 
> #define inline inline __attribute__((unused))
> 
> to still inline the functions, I was just seeing if there was anything 
> else that clang was warning about that was unrelated to a function's 
> inlining.
> 
> > Still I am not convinced that gcc's behavior is preferable in this
> > case. True, it saves us from adding a bunch of __maybe_unused or
> > #ifdefs, on the other hand the warning is a useful tool to spot truly
> > unused code. So far about 50% of the warnings I looked into fall into
> > this category.
> > 
> 
> I think gcc's behavior is a result of how it does preprocessing and is a 
> clearly defined and long-standing semantic given in the gcc manual 
> regarding -Wunused-function.
> 
> #define IS_PAGE_ALIGNED(__size)	(!(__size & ((size_t)PAGE_SIZE - 1)))
> static inline int is_page_aligned(size_t size)
> {
> 	return !(size & ((size_t)PAGE_SIZE - 1));
> }
> 
> Gcc will not warn about either of these being unused, regardless of -Wall, 
> -Wunused-function, or -pedantic.  Clang, correct me if I'm wrong, will 
> only warn about is_page_aligned().

Indeed, clang does not warn about unused defines.

> So the argument could be made that one of the additional benefits of 
> static inline functions is that a subset of compilers, heavily in the 
> minority, will detect whether it's unused and we'll get patches that 
> remove them.  Functionally, it would only result in LOC reduction.  But, 
> isn't adding #ifdef's to silence the warning just adding more LOC?

The LOC reduction comes from the removal of the actual dead code that
is spotted because the warning was enabled and pointed it out :)

Using #ifdef is one option, in most cases the function can be marked as
__maybe_unused, which technically doesn't (necessarily) increase
LOC. However some maintainers prefer the use of #ifdef over
__maybe_unused in certain cases.

> I have no preference either way, I think it would be up to the person who 
> is maintaining the code and has to deal with the patches.

I think it would be good to have a general policy/agreement, to either
disable the warning completely (not my preference) or 'allow' the use
of one of the available mechanism to suppress the warning for
functions that are not used in some configurations or only kept around
for reference/debugging/symmetry.

Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[Matthias Kaehlcke]

Hi David,

El Wed, May 24, 2017 at 01:36:21PM -0700 David Rientjes ha dit:

> On Tue, 23 May 2017, Matthias Kaehlcke wrote:
> 
> > > diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> > > index de179993e039..e1895ce6fa1b 100644
> > > --- a/include/linux/compiler-clang.h
> > > +++ b/include/linux/compiler-clang.h
> > > @@ -15,3 +15,8 @@
> > >   * with any version that can compile the kernel
> > >   */
> > >  #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
> > > +
> > > +#ifdef inline
> > > +#undef inline
> > > +#define inline __attribute__((unused))
> > > +#endif
> > 
> > Thanks for the suggestion!
> > 
> > Nothing breaks and the warnings are silenced. It seems we could use
> > this if there is a stong opposition against having warnings on unused
> > static inline functions in .c files.
> > 
> 
> It would be slightly different, it would be:
> 
> #define inline inline __attribute__((unused))
> 
> to still inline the functions, I was just seeing if there was anything 
> else that clang was warning about that was unrelated to a function's 
> inlining.
> 
> > Still I am not convinced that gcc's behavior is preferable in this
> > case. True, it saves us from adding a bunch of __maybe_unused or
> > #ifdefs, on the other hand the warning is a useful tool to spot truly
> > unused code. So far about 50% of the warnings I looked into fall into
> > this category.
> > 
> 
> I think gcc's behavior is a result of how it does preprocessing and is a 
> clearly defined and long-standing semantic given in the gcc manual 
> regarding -Wunused-function.
> 
> #define IS_PAGE_ALIGNED(__size)	(!(__size & ((size_t)PAGE_SIZE - 1)))
> static inline int is_page_aligned(size_t size)
> {
> 	return !(size & ((size_t)PAGE_SIZE - 1));
> }
> 
> Gcc will not warn about either of these being unused, regardless of -Wall, 
> -Wunused-function, or -pedantic.  Clang, correct me if I'm wrong, will 
> only warn about is_page_aligned().

Indeed, clang does not warn about unused defines.

> So the argument could be made that one of the additional benefits of 
> static inline functions is that a subset of compilers, heavily in the 
> minority, will detect whether it's unused and we'll get patches that 
> remove them.  Functionally, it would only result in LOC reduction.  But, 
> isn't adding #ifdef's to silence the warning just adding more LOC?

The LOC reduction comes from the removal of the actual dead code that
is spotted because the warning was enabled and pointed it out :)

Using #ifdef is one option, in most cases the function can be marked as
__maybe_unused, which technically doesn't (necessarily) increase
LOC. However some maintainers prefer the use of #ifdef over
__maybe_unused in certain cases.

> I have no preference either way, I think it would be up to the person who 
> is maintaining the code and has to deal with the patches.

I think it would be good to have a general policy/agreement, to either
disable the warning completely (not my preference) or 'allow' the use
of one of the available mechanism to suppress the warning for
functions that are not used in some configurations or only kept around
for reference/debugging/symmetry.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[Doug Anderson]

Hi,

On Wed, May 24, 2017 at 3:09 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> Hi David,
>
> El Wed, May 24, 2017 at 01:36:21PM -0700 David Rientjes ha dit:
>
>> On Tue, 23 May 2017, Matthias Kaehlcke wrote:
>>
>> > > diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
>> > > index de179993e039..e1895ce6fa1b 100644
>> > > --- a/include/linux/compiler-clang.h
>> > > +++ b/include/linux/compiler-clang.h
>> > > @@ -15,3 +15,8 @@
>> > >   * with any version that can compile the kernel
>> > >   */
>> > >  #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
>> > > +
>> > > +#ifdef inline
>> > > +#undef inline
>> > > +#define inline __attribute__((unused))
>> > > +#endif
>> >
>> > Thanks for the suggestion!
>> >
>> > Nothing breaks and the warnings are silenced. It seems we could use
>> > this if there is a stong opposition against having warnings on unused
>> > static inline functions in .c files.
>> >
>>
>> It would be slightly different, it would be:
>>
>> #define inline inline __attribute__((unused))
>>
>> to still inline the functions, I was just seeing if there was anything
>> else that clang was warning about that was unrelated to a function's
>> inlining.
>>
>> > Still I am not convinced that gcc's behavior is preferable in this
>> > case. True, it saves us from adding a bunch of __maybe_unused or
>> > #ifdefs, on the other hand the warning is a useful tool to spot truly
>> > unused code. So far about 50% of the warnings I looked into fall into
>> > this category.
>> >
>>
>> I think gcc's behavior is a result of how it does preprocessing and is a
>> clearly defined and long-standing semantic given in the gcc manual
>> regarding -Wunused-function.
>>
>> #define IS_PAGE_ALIGNED(__size)       (!(__size & ((size_t)PAGE_SIZE - 1)))
>> static inline int is_page_aligned(size_t size)
>> {
>>       return !(size & ((size_t)PAGE_SIZE - 1));
>> }
>>
>> Gcc will not warn about either of these being unused, regardless of -Wall,
>> -Wunused-function, or -pedantic.  Clang, correct me if I'm wrong, will
>> only warn about is_page_aligned().
>
> Indeed, clang does not warn about unused defines.
>
>> So the argument could be made that one of the additional benefits of
>> static inline functions is that a subset of compilers, heavily in the
>> minority, will detect whether it's unused and we'll get patches that
>> remove them.  Functionally, it would only result in LOC reduction.  But,
>> isn't adding #ifdef's to silence the warning just adding more LOC?
>
> The LOC reduction comes from the removal of the actual dead code that
> is spotted because the warning was enabled and pointed it out :)
>
> Using #ifdef is one option, in most cases the function can be marked as
> __maybe_unused, which technically doesn't (necessarily) increase
> LOC. However some maintainers prefer the use of #ifdef over
> __maybe_unused in certain cases.
>
>> I have no preference either way, I think it would be up to the person who
>> is maintaining the code and has to deal with the patches.
>
> I think it would be good to have a general policy/agreement, to either
> disable the warning completely (not my preference) or 'allow' the use
> of one of the available mechanism to suppress the warning for
> functions that are not used in some configurations or only kept around
> for reference/debugging/symmetry.

I would tend to agree with Matthias that we need some type of general
policy / agreement with enough maintainers.  It's nice to consider all
warnings as important and if there are a few outliers it makes it
easier to ignore all warnings.

BTW: just as extra motivation showing the usefulness of this work, see
a patch I just posted that deletes a bunch of unneeded code in an ASoC
driver:

https://patchwork.kernel.org/patch/9750813/

I don't know anything about this driver but the clang warning made it
obvious that something was wrong.  Either we were doing a bit of
useless saving or (perhaps) the restore was actually supposed to be
called somewhere and we had a bug.

-Doug

Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[Doug Anderson]

Hi,

On Wed, May 24, 2017 at 3:09 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> Hi David,
>
> El Wed, May 24, 2017 at 01:36:21PM -0700 David Rientjes ha dit:
>
>> On Tue, 23 May 2017, Matthias Kaehlcke wrote:
>>
>> > > diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
>> > > index de179993e039..e1895ce6fa1b 100644
>> > > --- a/include/linux/compiler-clang.h
>> > > +++ b/include/linux/compiler-clang.h
>> > > @@ -15,3 +15,8 @@
>> > >   * with any version that can compile the kernel
>> > >   */
>> > >  #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
>> > > +
>> > > +#ifdef inline
>> > > +#undef inline
>> > > +#define inline __attribute__((unused))
>> > > +#endif
>> >
>> > Thanks for the suggestion!
>> >
>> > Nothing breaks and the warnings are silenced. It seems we could use
>> > this if there is a stong opposition against having warnings on unused
>> > static inline functions in .c files.
>> >
>>
>> It would be slightly different, it would be:
>>
>> #define inline inline __attribute__((unused))
>>
>> to still inline the functions, I was just seeing if there was anything
>> else that clang was warning about that was unrelated to a function's
>> inlining.
>>
>> > Still I am not convinced that gcc's behavior is preferable in this
>> > case. True, it saves us from adding a bunch of __maybe_unused or
>> > #ifdefs, on the other hand the warning is a useful tool to spot truly
>> > unused code. So far about 50% of the warnings I looked into fall into
>> > this category.
>> >
>>
>> I think gcc's behavior is a result of how it does preprocessing and is a
>> clearly defined and long-standing semantic given in the gcc manual
>> regarding -Wunused-function.
>>
>> #define IS_PAGE_ALIGNED(__size)       (!(__size & ((size_t)PAGE_SIZE - 1)))
>> static inline int is_page_aligned(size_t size)
>> {
>>       return !(size & ((size_t)PAGE_SIZE - 1));
>> }
>>
>> Gcc will not warn about either of these being unused, regardless of -Wall,
>> -Wunused-function, or -pedantic.  Clang, correct me if I'm wrong, will
>> only warn about is_page_aligned().
>
> Indeed, clang does not warn about unused defines.
>
>> So the argument could be made that one of the additional benefits of
>> static inline functions is that a subset of compilers, heavily in the
>> minority, will detect whether it's unused and we'll get patches that
>> remove them.  Functionally, it would only result in LOC reduction.  But,
>> isn't adding #ifdef's to silence the warning just adding more LOC?
>
> The LOC reduction comes from the removal of the actual dead code that
> is spotted because the warning was enabled and pointed it out :)
>
> Using #ifdef is one option, in most cases the function can be marked as
> __maybe_unused, which technically doesn't (necessarily) increase
> LOC. However some maintainers prefer the use of #ifdef over
> __maybe_unused in certain cases.
>
>> I have no preference either way, I think it would be up to the person who
>> is maintaining the code and has to deal with the patches.
>
> I think it would be good to have a general policy/agreement, to either
> disable the warning completely (not my preference) or 'allow' the use
> of one of the available mechanism to suppress the warning for
> functions that are not used in some configurations or only kept around
> for reference/debugging/symmetry.

I would tend to agree with Matthias that we need some type of general
policy / agreement with enough maintainers.  It's nice to consider all
warnings as important and if there are a few outliers it makes it
easier to ignore all warnings.

BTW: just as extra motivation showing the usefulness of this work, see
a patch I just posted that deletes a bunch of unneeded code in an ASoC
driver:

https://patchwork.kernel.org/patch/9750813/

I don't know anything about this driver but the clang warning made it
obvious that something was wrong.  Either we were doing a bit of
useless saving or (perhaps) the restore was actually supposed to be
called somewhere and we had a bug.

-Doug

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>