mm: cleanup swap unused warning

PATCH In review Con Kolivas 6 participants 13 messages May 10, 2006 – May 17, 2006

Subsystems (2)

Memory management MM swap

lore

Share & tools

lore

Con Kolivas

author

2006-05-10 11:32 UTC #0 lore raw

PATCH 1/1 mm: cleanup swap unused warning

Are there any users of swp_entry_t when CONFIG_SWAP is not defined?

This patch fixes a warning for !CONFIG_SWAP for me.

---
if CONFIG_SWAP is not defined we get:

mm/vmscan.c: In function a??remove_mappinga??:
mm/vmscan.c:387: warning: unused variable a??swapa??

Signed-off-by Con Kolivas <[email protected]>

---
 include/linux/swap.h |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

include/linux/swap.h +11 -4

Index: linux-2.6.17-rc3-mm1/include/linux/swap.h
===================================================================
@@ -67,13 +67,20 @@ union swap_header {
 	} info;
 };
 
- /* A swap entry has to fit into a "unsigned long", as
-  * the entry is hidden in the "index" field of the
-  * swapper address space.
-  */
+/*
+ * A swap entry has to fit into a "unsigned long", as
+ * the entry is hidden in the "index" field of the
+ * swapper address space.
+ */
+#ifdef CONFIG_SWAP
 typedef struct {
 	unsigned long val;
 } swp_entry_t;
+#else
+typedef struct {
+	unsigned long val;
+} swp_entry_t __attribute__((__unused__));
+#endif
 
 /*
  * current->reclaim_state points to one of these when a task is running

Signature · 7 lines

-ck

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

Andrew Morton

2006-05-10 11:38 UTC #1 lore raw

Quoted 2 lines · Con Kolivas

Are there any users of swp_entry_t when CONFIG_SWAP is not defined?

Well there shouldn't be. Making accesses to swp_entry_t.val fail to compile if !CONFIG_SWAP might be useful.

Quoted 4 lines

include/linux/swap.h +4 -0

+/*
+ * A swap entry has to fit into a "unsigned long", as
+ * the entry is hidden in the "index" field of the
+ * swapper address space.

include/linux/swap.h union swap_header { · line 83

Quoted 10 lines

include/linux/swap.h +7 -0

+ */
+#ifdef CONFIG_SWAP
 typedef struct {
 	unsigned long val;
 } swp_entry_t;
+#else
+typedef struct {
+	unsigned long val;
+} swp_entry_t __attribute__((__unused__));
+#endif

We have __attribute_used__, which hides a gcc oddity.

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

Con Kolivas

author

2006-05-10 11:46 UTC #2 lore raw

Quoted 6 lines · Andrew Morton

Quoted 1 line · Con Kolivas
Are there any users of swp_entry_t when CONFIG_SWAP is not defined?
Well there shouldn't be. Making accesses to swp_entry_t.val fail to compile if !CONFIG_SWAP might be useful.

Quoted 16 lines

Quoted 4 lines
include/linux/swap.h +4 -0
+/*
+ * A swap entry has to fit into a "unsigned long", as
+ * the entry is hidden in the "index" field of the
+ * swapper address space.
include/linux/swap.h union swap_header { · line 83
Quoted 10 lines
include/linux/swap.h +7 -0
+ */
+#ifdef CONFIG_SWAP
 typedef struct {
 	unsigned long val;
 } swp_entry_t;
+#else
+typedef struct {
+	unsigned long val;
+} swp_entry_t __attribute__((__unused__));
+#endif
We have __attribute_used__, which hides a gcc oddity.

I tried that.

In file included from arch/i386/mm/pgtable.c:11:
include/linux/swap.h:82: warning: a??__used__a?? attribute ignored In file included from include/linux/suspend.h:8,
from init/do_mounts.c:7:
include/linux/swap.h:82: warning: a??__used__a?? attribute ignored In file included from arch/i386/mm/init.c:22:
include/linux/swap.h:82: warning: a??__used__a?? attribute ignored
AS arch/i386/kernel/vsyscall-sysenter.o

etc..

and doesn't fix the warning in vmscan.c. __attribute_used__ is handled differently by gcc4 it seems (this is 4.1.0)

Signature · 7 lines

-ck

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

Con Kolivas

author

2006-05-10 11:56 UTC #3 lore raw

Quoted 14 lines · Con Kolivas

Quoted 1 line · Andrew Morton
We have __attribute_used__, which hides a gcc oddity.
I tried that.
In file included from arch/i386/mm/pgtable.c:11:
include/linux/swap.h:82: warning: a??__used__a?? attribute ignored In file included from include/linux/suspend.h:8,
from init/do_mounts.c:7:
include/linux/swap.h:82: warning: a??__used__a?? attribute ignored In file included from arch/i386/mm/init.c:22:
include/linux/swap.h:82: warning: a??__used__a?? attribute ignored
AS arch/i386/kernel/vsyscall-sysenter.o

Quoted 4 lines

etc..
and doesn't fix the warning in vmscan.c. __attribute_used__ is handled differently by gcc4 it seems (this is 4.1.0)

in compiler-gcc3.h
#if __GNUC_MINOR__ >= 3

# define __attribute_used__     __attribute__((__used__))
#else
# define __attribute_used__     __attribute__((__unused__))
#endif

and in compiler-gcc4.h
#define __attribute_used__ __attribute__((__used__))

it looks like the pre gcc3.3 version is suited here or I'm misusing the __attribute_used__ extension somehow.

Signature · 7 lines

-ck

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

Daniel Walker

2006-05-10 18:20 UTC #4 lore raw

In reply to Andrew Morton

Quoted 6 lines · Andrew Morton

Quoted 2 lines · Con Kolivas
Are there any users of swp_entry_t when CONFIG_SWAP is not defined?
Well there shouldn't be. Making accesses to swp_entry_t.val fail to compile if !CONFIG_SWAP might be useful.

In mm/vmscan.c line 387 it defined swp_entry_t and sets val regardless of CONFIG_SWAP , but the value never really gets used .. Showed up in my warning reviews.

Daniel

Pekka Enberg

2006-05-10 11:42 UTC #5 lore raw

In reply to Con Kolivas

Quoted 4 lines · Con Kolivas

include/linux/swap.h +4 -0

+/*
+ * A swap entry has to fit into a "unsigned long", as
+ * the entry is hidden in the "index" field of the
+ * swapper address space.

include/linux/swap.h union swap_header { · line 83

Quoted 10 lines

include/linux/swap.h +7 -0

+ */
+#ifdef CONFIG_SWAP
 typedef struct {
        unsigned long val;
 } swp_entry_t;
+#else
+typedef struct {
+       unsigned long val;
+} swp_entry_t __attribute__((__unused__));
+#endif

Or we could make swap_free() an empty static inline function for the non-CONFIG_SWAP case.

Pekka

Christoph Lameter

2006-05-10 23:04 UTC #6 lore raw

In reply to Con Kolivas

Quoted 1 line · Con Kolivas

Are there any users of swp_entry_t when CONFIG_SWAP is not defined?

Yes, a migration entry is a form of swap entry.

Con Kolivas

author

2006-05-16 10:55 UTC #7 lore raw

Quoted 4 lines · Christoph Lameter

Quoted 1 line · Con Kolivas
Are there any users of swp_entry_t when CONFIG_SWAP is not defined?
Yes, a migration entry is a form of swap entry.

mm/vmscan.c: In function a??remove_mappinga??:
mm/vmscan.c:387: warning: unused variable a??swapa??

Ok so if we fix it by making swp_entry_t __attribute__((__unused__) we break swap migration code?

If we make swap_free() an empty static inline function then gcc compiles in the variable needlessly and we won't know it.

For the moment let's continue putting up with the warning.

Signature · 7 lines

-ck

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

Con Kolivas

author

2006-05-16 13:14 UTC #8 lore raw

PATCH 1/1 mm: cleanup swap unused warning

Quoted 5 lines · Con Kolivas

Ok so if we fix it by making swp_entry_t __attribute__((__unused__) we break swap migration code?
If we make swap_free() an empty static inline function then gcc compiles in the variable needlessly and we won't know it.

Rather than assume I checked the generated code and I was wrong (which is something I'm getting good at being).

The variable is not compiled in so the empty static inline as suggested by Pekka suffices to silence this warning.

---
if CONFIG_SWAP is not defined we get:

mm/vmscan.c: In function a??remove_mappinga??:
mm/vmscan.c:387: warning: unused variable a??swapa??

Signed-off-by Con Kolivas <[email protected]>

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

include/linux/swap.h +4 -1

Index: linux-2.6.17-rc4/include/linux/swap.h
===================================================================
@@ -292,7 +292,10 @@ static inline void disable_swap_token(vo
 #define show_swap_cache_info()			/*NOTHING*/
 #define free_swap_and_cache(swp)		/*NOTHING*/
 #define swap_duplicate(swp)			/*NOTHING*/
-#define swap_free(swp)				/*NOTHING*/
+static inline void swap_free(swp_entry_t swp)
+{
+}
+
 #define read_swap_cache_async(swp,vma,addr)	NULL
 #define lookup_swap_cache(swp)			NULL
 #define valid_swaphandles(swp, off)		0

Signature · 7 lines

-ck

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

Christoph Lameter

2006-05-16 16:00 UTC #9 lore raw

Quoted 2 lines · Con Kolivas

The variable is not compiled in so the empty static inline as suggested by Pekka suffices to silence this warning.

Maybe you could redo the whole thing? Is it a problem to make all the similar functions inlines?

Con Kolivas

author

2006-05-17 06:27 UTC #10 lore raw

PATCH 1/1 mm: cleanup swap unused warning

Quoted 6 lines · Christoph Lameter

Quoted 2 lines · Con Kolivas
The variable is not compiled in so the empty static inline as suggested by Pekka suffices to silence this warning.
Maybe you could redo the whole thing? Is it a problem to make all the similar functions inlines?

No problem.

---
if CONFIG_SWAP is not defined we get:

mm/vmscan.c: In function a??remove_mappinga??:
mm/vmscan.c:387: warning: unused variable a??swapa??

Convert defines in swap.h into blank inline functions to fix this warning and be consistent.

Signed-off-by Con Kolivas <[email protected]>

---
 include/linux/swap.h |   64 ++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 53 insertions(+), 11 deletions(-)

include/linux/swap.h +53 ~~-11~~

Index: linux-2.6.17-rc4-mm1/include/linux/swap.h
===================================================================
@@ -286,18 +286,60 @@ static inline void disable_swap_token(vo
 #define free_pages_and_swap_cache(pages, nr) \
 	release_pages((pages), (nr), 0);
 
-#define show_swap_cache_info()			/*NOTHING*/
-#define free_swap_and_cache(swp)		/*NOTHING*/
-#define swap_duplicate(swp)			/*NOTHING*/
-#define swap_free(swp)				/*NOTHING*/
-#define read_swap_cache_async(swp,vma,addr)	NULL
-#define lookup_swap_cache(swp)			NULL
-#define valid_swaphandles(swp, off)		0
+static inline void show_swap_cache_info(void)
+{
+}
+
+static inline void free_swap_and_cache(swp_entry_t swp)
+{
+}
+
+static inline int swap_duplicate(swp_entry_t swp)
+{
+	return 0;
+}
+
+static inline void swap_free(swp_entry_t swp)
+{
+}
+
+static inline struct page *read_swap_cache_async(swp_entry_t swp,
+			struct vm_area_struct *vma, unsigned long addr)
+{
+	return NULL;
+}
+
+static inline struct page *lookup_swap_cache(swp_entry_t swp)
+{
+	return NULL;
+}
+
+static inline int valid_swaphandles(swp_entry_t entry, unsigned long *offset)
+{
+	return 0;
+}
+
 #define can_share_swap_page(p)			(page_mapcount(p) == 1)
-#define move_to_swap_cache(p, swp)		1
-#define move_from_swap_cache(p, i, m)		1
-#define __delete_from_swap_cache(p)		/*NOTHING*/
-#define delete_from_swap_cache(p)		/*NOTHING*/
+
+static inline int move_to_swap_cache(struct page *page, swp_entry_t entry)
+{
+	return 1;
+}
+
+static inline int move_from_swap_cache(struct page *page, unsigned long index,
+					struct address_space *mapping)
+{
+	return 1;
+}
+
+static inline void __delete_from_swap_cache(struct page *page)
+{
+}
+
+static inline void delete_from_swap_cache(struct page *page)
+{
+}
+
 #define swap_token_default_timeout		0
 
 static inline int remove_exclusive_swap_page(struct page *p)

Signature · 7 lines

-ck

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

Christoph Lameter

2006-05-16 15:58 UTC #11 lore raw

In reply to Con Kolivas

Quoted 11 lines · Con Kolivas

Quoted 4 lines · Christoph Lameter
Quoted 1 line · Con Kolivas
Are there any users of swp_entry_t when CONFIG_SWAP is not defined?
Yes, a migration entry is a form of swap entry.
mm/vmscan.c: In function ÿÿremove_mappingÿÿ:
mm/vmscan.c:387: warning: unused variable ÿÿswapÿÿ
Ok so if we fix it by making swp_entry_t __attribute__((__unused__) we break swap migration code?

This will generally break page migration in mm.

Quoted 2 lines

If we make swap_free() an empty static inline function then gcc compiles in the variable needlessly and we won't know it.

PageSwapCache() returns false if CONFIG_SWAP is not set and therefore no code is generated.

Roman Zippel

2006-05-16 11:24 UTC #12 lore raw

In reply to Con Kolivas

Hi,

Quoted 9 lines · Con Kolivas

Are there any users of swp_entry_t when CONFIG_SWAP is not defined?
This patch fixes a warning for !CONFIG_SWAP for me.
---
if CONFIG_SWAP is not defined we get:
mm/vmscan.c: In function ‘remove_mapping’:
mm/vmscan.c:387: warning: unused variable ‘swap’

In similiar cases (e.g. spinlocks) we usually do something like this:

#define swap_free(swp) ((void)(swp))

bye, Roman