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’
Signed-off-by: Con Kolivas <[email protected]>
---
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
Con Kolivas <[email protected]> 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.
On 5/10/06, Con Kolivas <[email protected]> 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
On Wednesday 10 May 2006 21:38, Andrew Morton wrote:
> Con Kolivas <[email protected]> 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: ‘__used__’ attribute ignored
In file included from include/linux/suspend.h:8,
from init/do_mounts.c:7:
include/linux/swap.h:82: warning: ‘__used__’ attribute ignored
In file included from arch/i386/mm/init.c:22:
include/linux/swap.h:82: warning: ‘__used__’ 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
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: ‘__used__’ attribute ignored
> In file included from include/linux/suspend.h:8,
> from init/do_mounts.c:7:
> include/linux/swap.h:82: warning: ‘__used__’ attribute ignored
> In file included from arch/i386/mm/init.c:22:
> include/linux/swap.h:82: warning: ‘__used__’ 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
On Wed, 2006-05-10 at 04:38 -0700, Andrew Morton wrote:
> Con Kolivas <[email protected]> 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
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.
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?
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
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
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 ‘remove_mapping’:
mm/vmscan.c:387: warning: unused variable ‘swap’
Signed-off-by: Con Kolivas <[email protected]>
---
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
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.
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?
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 ‘remove_mapping’:
mm/vmscan.c:387: warning: unused variable ‘swap’
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(-)
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