[PATCH] mm: cleanup swap unused warning

[PATCH] mm: cleanup swap unused warning

[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 a??remove_mappinga??:
mm/vmscan.c:387: warning: unused variable a??swapa??

Signed-off-by: Con Kolivas <kernel@kolivas.org>

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

Index: linux-2.6.17-rc3-mm1/include/linux/swap.h
===================================================================
--- linux-2.6.17-rc3-mm1.orig/include/linux/swap.h	2006-05-10 21:14:41.000000000 +1000
+++ linux-2.6.17-rc3-mm1/include/linux/swap.h	2006-05-10 21:24:31.000000000 +1000
@@ -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

-- 
-ck

--
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] mm: cleanup swap unused warning

[Andrew Morton]

Con Kolivas <kernel@kolivas.org> wrote:
>
> 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.

> +/*
> + * 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

We have __attribute_used__, which hides a gcc oddity.

--
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] mm: cleanup swap unused warning

[Pekka Enberg]

On 5/10/06, Con Kolivas <kernel@kolivas.org> wrote:
> +/*
> + * 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

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

                                                     Pekka

--
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] mm: cleanup swap unused warning

[Con Kolivas]

On Wednesday 10 May 2006 21:38, Andrew Morton wrote:
> Con Kolivas <kernel@kolivas.org> wrote:
> > 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.
>
> > +/*
> > + * 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
>
> 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)

-- 
-ck

--
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] mm: cleanup swap unused warning

[Con Kolivas]

On Wednesday 10 May 2006 21:46, Con Kolivas wrote:
> On Wednesday 10 May 2006 21:38, Andrew Morton wrote:
> > 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)

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.

-- 
-ck

--
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] mm: cleanup swap unused warning

[Daniel Walker]

On Wed, 2006-05-10 at 04:38 -0700, Andrew Morton wrote:
> Con Kolivas <kernel@kolivas.org> wrote:
> >
> > 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

--
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] mm: cleanup swap unused warning

[Christoph Lameter]

On Wed, 10 May 2006, Con Kolivas wrote:

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

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

--
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] mm: cleanup swap unused warning

[Con Kolivas]

On Thursday 11 May 2006 09:04, Christoph Lameter wrote:
> On Wed, 10 May 2006, Con Kolivas wrote:
> > 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.

-- 
-ck

--
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] mm: cleanup swap unused warning

[Roman Zippel]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 467 bytes --]

Hi,

On Wed, 10 May 2006, Con Kolivas wrote:

> 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

[PATCH] mm: cleanup swap unused warning

[Con Kolivas]

On Tuesday 16 May 2006 20:55, Con Kolivas wrote:
> 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 <kernel@kolivas.org>

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

Index: linux-2.6.17-rc4/include/linux/swap.h
===================================================================
--- linux-2.6.17-rc4.orig/include/linux/swap.h	2006-05-16 23:07:35.000000000 +1000
+++ linux-2.6.17-rc4/include/linux/swap.h	2006-05-16 23:08:08.000000000 +1000
@@ -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

-- 
-ck

--
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] mm: cleanup swap unused warning

[Christoph Lameter]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 775 bytes --]

On Tue, 16 May 2006, Con Kolivas wrote:

> On Thursday 11 May 2006 09:04, Christoph Lameter wrote:
> > On Wed, 10 May 2006, Con Kolivas wrote:
> > > 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.
 
> 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.

Re: [PATCH] mm: cleanup swap unused warning

[Christoph Lameter]

On Tue, 16 May 2006, Con Kolivas wrote:

> 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?

--
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][respin] mm: cleanup swap unused warning

[Con Kolivas]

On Wednesday 17 May 2006 02:00, Christoph Lameter wrote:
> On Tue, 16 May 2006, Con Kolivas wrote:
> > 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 <kernel@kolivas.org>

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

Index: linux-2.6.17-rc4-mm1/include/linux/swap.h
===================================================================
--- linux-2.6.17-rc4-mm1.orig/include/linux/swap.h	2006-05-17 15:57:48.000000000 +1000
+++ linux-2.6.17-rc4-mm1/include/linux/swap.h	2006-05-17 16:21:03.000000000 +1000
@@ -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)

-- 
-ck

--
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>