Subject: [PATCH V7 1/4] mm: frontswap: swap data structure changes

Subject: [PATCH V7 1/4] mm: frontswap: swap data structure changes

[Dan Magenheimer]

From: Dan Magenheimer <dan.magenheimer@oracle.com>
Subject: [PATCH V7 1/4] mm: frontswap: swap data structure changes

This first patch of four in the frontswap series makes available core
swap data structures (swap_lock, swap_list and swap_info) that are
needed by frontswap.c but we don't need to expose them to the dozens
of files that include swap.h so we create a new swapfile.h just to
extern-ify these.

Also add frontswap-related elements to swap_info_struct.  Frontswap_map
points to vzalloc'ed one-bit-per-swap-page metadata that indicates
whether the swap page is in frontswap or in the device and frontswap_pages
counts how many pages are in frontswap.

[v7: rebase to 3.0-rc3]
[v7: JBeulich@novell.com: add new swap struct elements only if config'd]
[v6: rebase to 3.0-rc1]
[v5: no change from v4]
[v4: rebase to 2.6.39]
Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
Reviewed-by: Konrad Wilk <konrad.wilk@oracle.com>
Acked-by: Jan Beulich <JBeulich@novell.com>
Acked-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: Hugh Dickins <hughd@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Nitin Gupta <ngupta@vflare.org>
Cc: Matthew Wilcox <matthew@wil.cx>
Cc: Chris Mason <chris.mason@oracle.com>
Cc: Rik Riel <riel@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>

--- linux/include/linux/swapfile.h	1969-12-31 17:00:00.000000000 -0700
+++ frontswap/include/linux/swapfile.h	2011-08-23 08:20:09.757818005 -0600
@@ -0,0 +1,13 @@
+#ifndef _LINUX_SWAPFILE_H
+#define _LINUX_SWAPFILE_H
+
+/*
+ * these were static in swapfile.c but frontswap.c needs them and we don't
+ * want to expose them to the dozens of source files that include swap.h
+ */
+extern spinlock_t swap_lock;
+extern struct swap_list_t swap_list;
+extern struct swap_info_struct *swap_info[];
+extern int try_to_unuse(unsigned int, bool, unsigned long);
+
+#endif /* _LINUX_SWAPFILE_H */
--- linux/include/linux/swap.h	2011-08-08 08:19:25.880690134 -0600
+++ frontswap/include/linux/swap.h	2011-08-23 08:27:57.584694638 -0600
@@ -194,6 +194,10 @@ struct swap_info_struct {
 	struct block_device *bdev;	/* swap device or bdev of swap file */
 	struct file *swap_file;		/* seldom referenced */
 	unsigned int old_block_size;	/* seldom referenced */
+#ifdef CONFIG_FRONTSWAP
+	unsigned long *frontswap_map;	/* frontswap in-use, one bit per page */
+	unsigned int frontswap_pages;	/* frontswap pages in-use counter */
+#endif
 };
 
 struct swap_list_t {

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Subject: [PATCH V7 1/4] mm: frontswap: swap data structure changes

[KAMEZAWA Hiroyuki]

On Tue, 23 Aug 2011 07:57:55 -0700
Dan Magenheimer <dan.magenheimer@oracle.com> wrote:

> From: Dan Magenheimer <dan.magenheimer@oracle.com>
> Subject: [PATCH V7 1/4] mm: frontswap: swap data structure changes
> 
> This first patch of four in the frontswap series makes available core
> swap data structures (swap_lock, swap_list and swap_info) that are
> needed by frontswap.c but we don't need to expose them to the dozens
> of files that include swap.h so we create a new swapfile.h just to
> extern-ify these.
> 
> Also add frontswap-related elements to swap_info_struct.  Frontswap_map
> points to vzalloc'ed one-bit-per-swap-page metadata that indicates
> whether the swap page is in frontswap or in the device and frontswap_pages
> counts how many pages are in frontswap.
> 
> [v7: rebase to 3.0-rc3]
> [v7: JBeulich@novell.com: add new swap struct elements only if config'd]
> [v6: rebase to 3.0-rc1]
> [v5: no change from v4]
> [v4: rebase to 2.6.39]
> Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
> Reviewed-by: Konrad Wilk <konrad.wilk@oracle.com>
> Acked-by: Jan Beulich <JBeulich@novell.com>
> Acked-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> Cc: Jeremy Fitzhardinge <jeremy@goop.org>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Nitin Gupta <ngupta@vflare.org>
> Cc: Matthew Wilcox <matthew@wil.cx>
> Cc: Chris Mason <chris.mason@oracle.com>
> Cc: Rik Riel <riel@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>

Hmm....could you modify mm/swapfile.c and remove 'static' in the same patch ?


--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

RE: Subject: [PATCH V7 1/4] mm: frontswap: swap data structure changes

[Dan Magenheimer]

> From: KAMEZAWA Hiroyuki [mailto:kamezawa.hiroyu@jp.fujitsu.com]
> Subject: Re: Subject: [PATCH V7 1/4] mm: frontswap: swap data structure changes

Hi Kamezawa-san --

Domo arigato for the review and feedback!

> Hmm....could you modify mm/swapfile.c and remove 'static' in the same patch ?

I separated out this header patch because I thought it would
make the key swap data structure changes more visible.  Are you
saying that it is more confusing?  Or does your compiler
have a problem after only this patch is applied? (My
compiler is fine with it.)

Thanks,
Dan

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Subject: [PATCH V7 1/4] mm: frontswap: swap data structure changes

[KAMEZAWA Hiroyuki]

On Thu, 25 Aug 2011 10:11:11 -0700 (PDT)
Dan Magenheimer <dan.magenheimer@oracle.com> wrote:

> > From: KAMEZAWA Hiroyuki [mailto:kamezawa.hiroyu@jp.fujitsu.com]
> > Subject: Re: Subject: [PATCH V7 1/4] mm: frontswap: swap data structure changes
> 
> Hi Kamezawa-san --
> 
> Domo arigato for the review and feedback!
> 
> > Hmm....could you modify mm/swapfile.c and remove 'static' in the same patch ?
> 
> I separated out this header patch because I thought it would
> make the key swap data structure changes more visible.  Are you
> saying that it is more confusing?

Yes. I know you add a new header file which is not included but..


At reviewing patch, I check whether all required changes are done.
In this case, you turned out the function to be externed but you
leave the function definition as 'static'. This unbalance confues me.

I always read patches from 1 to END. When I found an incomplete change
in patch 1, I remember it and need to find missng part from patch 2->End. 
This makes my review confused a little.

In another case, when a patch adds a new file, I check Makefile change.
Considering dependency, the patch order should be

	[patch 1] Documentaion/Config
	[patch 2] Makefile + add new file.

But plesse note: This is my thought. Other guys may have other idea.

Thanks,
-Kame

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

RE: Subject: [PATCH V7 1/4] mm: frontswap: swap data structure changes

[Dan Magenheimer]

> From: KAMEZAWA Hiroyuki [mailto:kamezawa.hiroyu@jp.fujitsu.com]
> Subject: Re: Subject: [PATCH V7 1/4] mm: frontswap: swap data structure changes
> 
> > > From: KAMEZAWA Hiroyuki [mailto:kamezawa.hiroyu@jp.fujitsu.com]
> > > Subject: Re: Subject: [PATCH V7 1/4] mm: frontswap: swap data structure changes
> >
> > Hi Kamezawa-san --
> >
> > Domo arigato for the review and feedback!
> >
> > > Hmm....could you modify mm/swapfile.c and remove 'static' in the same patch ?
> >
> > I separated out this header patch because I thought it would
> > make the key swap data structure changes more visible.  Are you
> > saying that it is more confusing?
> 
> Yes. I know you add a new header file which is not included but..
> 
> At reviewing patch, I check whether all required changes are done.
> In this case, you turned out the function to be externed but you
> leave the function definition as 'static'. This unbalance confues me.
> 
> I always read patches from 1 to END. When I found an incomplete change
> in patch 1, I remember it and need to find missng part from patch 2->End.
> This makes my review confused a little.
> 
> In another case, when a patch adds a new file, I check Makefile change.
> Considering dependency, the patch order should be
> 
> 	[patch 1] Documentaion/Config
> 	[patch 2] Makefile + add new file.
> 
> But plesse note: This is my thought. Other guys may have other idea.

I think that is probably a good approach.  I will try to use it
for future patches.  But since this frontswap patchset is already
on V7, I hope it is OK if I continue to organize it for V8 the same
as it has been, as it might be confusing to previous reviewers
to change the organization now.

Thanks,
Dan

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Subject: [PATCH V7 1/4] mm: frontswap: swap data structure changes

[Konrad Rzeszutek Wilk]

On Fri, Aug 26, 2011 at 07:15:30AM -0700, Dan Magenheimer wrote:
> > From: KAMEZAWA Hiroyuki [mailto:kamezawa.hiroyu@jp.fujitsu.com]
> > Subject: Re: Subject: [PATCH V7 1/4] mm: frontswap: swap data structure changes
> > 
> > > > From: KAMEZAWA Hiroyuki [mailto:kamezawa.hiroyu@jp.fujitsu.com]
> > > > Subject: Re: Subject: [PATCH V7 1/4] mm: frontswap: swap data structure changes
> > >
> > > Hi Kamezawa-san --
> > >
> > > Domo arigato for the review and feedback!
> > >
> > > > Hmm....could you modify mm/swapfile.c and remove 'static' in the same patch ?
> > >
> > > I separated out this header patch because I thought it would
> > > make the key swap data structure changes more visible.  Are you
> > > saying that it is more confusing?
> > 
> > Yes. I know you add a new header file which is not included but..
> > 
> > At reviewing patch, I check whether all required changes are done.
> > In this case, you turned out the function to be externed but you
> > leave the function definition as 'static'. This unbalance confues me.
> > 
> > I always read patches from 1 to END. When I found an incomplete change
> > in patch 1, I remember it and need to find missng part from patch 2->End.
> > This makes my review confused a little.
> > 
> > In another case, when a patch adds a new file, I check Makefile change.
> > Considering dependency, the patch order should be
> > 
> > 	[patch 1] Documentaion/Config
> > 	[patch 2] Makefile + add new file.
> > 
> > But plesse note: This is my thought. Other guys may have other idea.
> 
> I think that is probably a good approach.  I will try to use it
> for future patches.  But since this frontswap patchset is already
> on V7, I hope it is OK if I continue to organize it for V8 the same
> as it has been, as it might be confusing to previous reviewers
> to change the organization now.

Nah, that is what part of the review process is - keep us on our toes.

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>