Re: [PATCHv2 7/9] mm: break up swap_writepage() for frontswap backends

2013-01-28 Thread Minchan Kim 
On Mon, Jan 28, 2013 at 11:26:43AM -0600, Seth Jennings wrote:
> On 01/27/2013 10:22 PM, Minchan Kim wrote:
> > On Mon, Jan 07, 2013 at 02:24:38PM -0600, Seth Jennings wrote:
> >> swap_writepage() is currently where frontswap hooks into the swap
> >> write path to capture pages with the frontswap_store() function.
> >> However, if a frontswap backend wants to "resume" the writeback of
> >> a page to the swap device, it can't call swap_writepage() as
> >> the page will simply reenter the backend.
> >>
> >> This patch separates swap_writepage() into a top and bottom half, the
> >> bottom half named __swap_writepage() to allow a frontswap backend,
> >> like zswap, to resume writeback beyond the frontswap_store() hook and
> >> by notified when the writeback completes.
> >>
> >> Signed-off-by: Seth Jennings 
> > 
> > Looks good to me except few nitpicks.
> 
> I broke these changes out from the main zswap patch (patch 8/9) in
> response to a request for Dan.  You are right in that there are
> changes here unrelated to the commit message.
> 
> The other changes are related to allowing zswap to do accounting on
> the number of outstanding flushes to the swap device.
> 
> If I'm going to break those out though the two resulting patches would be:
> 1. breakup __swap_writepage() and un-static __add_to_swap_cache() to
> allow resuming of swap page writeback
> 2. change __swap_writepage() signature to include end_write_func and
> un-static end_swap_bio_write() for accounting of outstanding flushes
> 

Or please add comment why you want to export some functions.

Thanks!

> Is that what you'd like to see?
> 
> Seth
> 
> > 
> > Acked-by: Minchan Kim 
> > 
> >> ---
> >>  include/linux/swap.h |4 
> >>  mm/page_io.c |   22 +-
> >>  mm/swap_state.c  |2 +-
> >>  3 files changed, 22 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/include/linux/swap.h b/include/linux/swap.h
> >> index 8c66486..a3da829 100644
> >> --- a/include/linux/swap.h
> >> +++ b/include/linux/swap.h
> >> @@ -321,6 +321,9 @@ static inline void 
> >> mem_cgroup_uncharge_swap(swp_entry_t ent)
> >>  /* linux/mm/page_io.c */
> >>  extern int swap_readpage(struct page *);
> >>  extern int swap_writepage(struct page *page, struct writeback_control 
> >> *wbc);
> >> +extern void end_swap_bio_write(struct bio *bio, int err);
> >> +extern int __swap_writepage(struct page *page, struct writeback_control 
> >> *wbc,
> >> +  void (*end_write_func)(struct bio *, int));
> >>  extern int swap_set_page_dirty(struct page *page);
> >>  extern void end_swap_bio_read(struct bio *bio, int err);
> >>  
> >> @@ -335,6 +338,7 @@ extern struct address_space swapper_space;
> >>  extern void show_swap_cache_info(void);
> >>  extern int add_to_swap(struct page *);
> >>  extern int add_to_swap_cache(struct page *, swp_entry_t, gfp_t);
> >> +extern int __add_to_swap_cache(struct page *page, swp_entry_t entry);
> > 
> > What's related __add_to_swap_cache with this patch?
> > 
> >>  extern void __delete_from_swap_cache(struct page *);
> >>  extern void delete_from_swap_cache(struct page *);
> >>  extern void free_page_and_swap_cache(struct page *);
> >> diff --git a/mm/page_io.c b/mm/page_io.c
> >> index c535d39..806085e 100644
> >> --- a/mm/page_io.c
> >> +++ b/mm/page_io.c
> >> @@ -43,7 +43,7 @@ static struct bio *get_swap_bio(gfp_t gfp_flags,
> >>return bio;
> >>  }
> >>  
> >> -static void end_swap_bio_write(struct bio *bio, int err)
> >> +void end_swap_bio_write(struct bio *bio, int err)
> > 
> > Why do you remove static in this patch? It's not related to the patch.
> > 
> >>  {
> >>const int uptodate = test_bit(BIO_UPTODATE, >bi_flags);
> >>struct page *page = bio->bi_io_vec[0].bv_page;
> >> @@ -180,15 +180,16 @@ bad_bmap:
> >>goto out;
> >>  }
> >>  
> >> +int __swap_writepage(struct page *page, struct writeback_control *wbc,
> >> +  void (*end_write_func)(struct bio *, int));
> >> +
> >>  /*
> >>   * We may have stale swap cache pages in memory: notice
> >>   * them here and get rid of the unnecessary final write.
> >>   */
> >>  int swap_writepage(struct page *page, struct writeback_control *wbc)
> >>  {
> >> -  struct bio *bio;
> >> -  int ret = 0, rw = WRITE;
> >> -  struct swap_info_struct *sis = page_swap_info(page);
> >> +  int ret = 0;
> >>  
> >>if (try_to_free_swap(page)) {
> >>unlock_page(page);
> >> @@ -200,6 +201,17 @@ int swap_writepage(struct page *page, struct 
> >> writeback_control *wbc)
> >>end_page_writeback(page);
> >>goto out;
> >>}
> >> +  ret = __swap_writepage(page, wbc, end_swap_bio_write);
> >> +out:
> >> +  return ret;
> >> +}
> >> +
> >> +int __swap_writepage(struct page *page, struct writeback_control *wbc,
> >> +  void (*end_write_func)(struct bio *, int))
> >> +{
> >> +  struct bio *bio;
> >> +  int ret = 0, rw = WRITE;
> >> +  struct swap_info_struct *sis = page_swap_info(page);
> >>  
> >>if (sis->flags & SWP_FILE) {
> >>

Re: [PATCHv2 7/9] mm: break up swap_writepage() for frontswap backends

2013-01-28 Thread Seth Jennings 
On 01/27/2013 10:22 PM, Minchan Kim wrote:
> On Mon, Jan 07, 2013 at 02:24:38PM -0600, Seth Jennings wrote:
>> swap_writepage() is currently where frontswap hooks into the swap
>> write path to capture pages with the frontswap_store() function.
>> However, if a frontswap backend wants to "resume" the writeback of
>> a page to the swap device, it can't call swap_writepage() as
>> the page will simply reenter the backend.
>>
>> This patch separates swap_writepage() into a top and bottom half, the
>> bottom half named __swap_writepage() to allow a frontswap backend,
>> like zswap, to resume writeback beyond the frontswap_store() hook and
>> by notified when the writeback completes.
>>
>> Signed-off-by: Seth Jennings 
> 
> Looks good to me except few nitpicks.

I broke these changes out from the main zswap patch (patch 8/9) in
response to a request for Dan.  You are right in that there are
changes here unrelated to the commit message.

The other changes are related to allowing zswap to do accounting on
the number of outstanding flushes to the swap device.

If I'm going to break those out though the two resulting patches would be:
1. breakup __swap_writepage() and un-static __add_to_swap_cache() to
allow resuming of swap page writeback
2. change __swap_writepage() signature to include end_write_func and
un-static end_swap_bio_write() for accounting of outstanding flushes

Is that what you'd like to see?

Seth

> 
> Acked-by: Minchan Kim 
> 
>> ---
>>  include/linux/swap.h |4 
>>  mm/page_io.c |   22 +-
>>  mm/swap_state.c  |2 +-
>>  3 files changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/linux/swap.h b/include/linux/swap.h
>> index 8c66486..a3da829 100644
>> --- a/include/linux/swap.h
>> +++ b/include/linux/swap.h
>> @@ -321,6 +321,9 @@ static inline void mem_cgroup_uncharge_swap(swp_entry_t 
>> ent)
>>  /* linux/mm/page_io.c */
>>  extern int swap_readpage(struct page *);
>>  extern int swap_writepage(struct page *page, struct writeback_control *wbc);
>> +extern void end_swap_bio_write(struct bio *bio, int err);
>> +extern int __swap_writepage(struct page *page, struct writeback_control 
>> *wbc,
>> +void (*end_write_func)(struct bio *, int));
>>  extern int swap_set_page_dirty(struct page *page);
>>  extern void end_swap_bio_read(struct bio *bio, int err);
>>  
>> @@ -335,6 +338,7 @@ extern struct address_space swapper_space;
>>  extern void show_swap_cache_info(void);
>>  extern int add_to_swap(struct page *);
>>  extern int add_to_swap_cache(struct page *, swp_entry_t, gfp_t);
>> +extern int __add_to_swap_cache(struct page *page, swp_entry_t entry);
> 
> What's related __add_to_swap_cache with this patch?
> 
>>  extern void __delete_from_swap_cache(struct page *);
>>  extern void delete_from_swap_cache(struct page *);
>>  extern void free_page_and_swap_cache(struct page *);
>> diff --git a/mm/page_io.c b/mm/page_io.c
>> index c535d39..806085e 100644
>> --- a/mm/page_io.c
>> +++ b/mm/page_io.c
>> @@ -43,7 +43,7 @@ static struct bio *get_swap_bio(gfp_t gfp_flags,
>>  return bio;
>>  }
>>  
>> -static void end_swap_bio_write(struct bio *bio, int err)
>> +void end_swap_bio_write(struct bio *bio, int err)
> 
> Why do you remove static in this patch? It's not related to the patch.
> 
>>  {
>>  const int uptodate = test_bit(BIO_UPTODATE, >bi_flags);
>>  struct page *page = bio->bi_io_vec[0].bv_page;
>> @@ -180,15 +180,16 @@ bad_bmap:
>>  goto out;
>>  }
>>  
>> +int __swap_writepage(struct page *page, struct writeback_control *wbc,
>> +void (*end_write_func)(struct bio *, int));
>> +
>>  /*
>>   * We may have stale swap cache pages in memory: notice
>>   * them here and get rid of the unnecessary final write.
>>   */
>>  int swap_writepage(struct page *page, struct writeback_control *wbc)
>>  {
>> -struct bio *bio;
>> -int ret = 0, rw = WRITE;
>> -struct swap_info_struct *sis = page_swap_info(page);
>> +int ret = 0;
>>  
>>  if (try_to_free_swap(page)) {
>>  unlock_page(page);
>> @@ -200,6 +201,17 @@ int swap_writepage(struct page *page, struct 
>> writeback_control *wbc)
>>  end_page_writeback(page);
>>  goto out;
>>  }
>> +ret = __swap_writepage(page, wbc, end_swap_bio_write);
>> +out:
>> +return ret;
>> +}
>> +
>> +int __swap_writepage(struct page *page, struct writeback_control *wbc,
>> +void (*end_write_func)(struct bio *, int))
>> +{
>> +struct bio *bio;
>> +int ret = 0, rw = WRITE;
>> +struct swap_info_struct *sis = page_swap_info(page);
>>  
>>  if (sis->flags & SWP_FILE) {
>>  struct kiocb kiocb;
>> @@ -227,7 +239,7 @@ int swap_writepage(struct page *page, struct 
>> writeback_control *wbc)
>>  return ret;
>>  }
>>  
>> -bio = get_swap_bio(GFP_NOIO, page, end_swap_bio_write);
>> +bio = get_swap_bio(GFP_NOIO, page, end_write_func);
>>  if (bio == NULL) {
>>  set_page_dirty(page);

Re: [PATCHv2 7/9] mm: break up swap_writepage() for frontswap backends

2013-01-28 Thread Seth Jennings 
On 01/27/2013 10:22 PM, Minchan Kim wrote:
 On Mon, Jan 07, 2013 at 02:24:38PM -0600, Seth Jennings wrote:
 swap_writepage() is currently where frontswap hooks into the swap
 write path to capture pages with the frontswap_store() function.
 However, if a frontswap backend wants to resume the writeback of
 a page to the swap device, it can't call swap_writepage() as
 the page will simply reenter the backend.

 This patch separates swap_writepage() into a top and bottom half, the
 bottom half named __swap_writepage() to allow a frontswap backend,
 like zswap, to resume writeback beyond the frontswap_store() hook and
 by notified when the writeback completes.

 Signed-off-by: Seth Jennings sjenn...@linux.vnet.ibm.com
 
 Looks good to me except few nitpicks.

I broke these changes out from the main zswap patch (patch 8/9) in
response to a request for Dan.  You are right in that there are
changes here unrelated to the commit message.

The other changes are related to allowing zswap to do accounting on
the number of outstanding flushes to the swap device.

If I'm going to break those out though the two resulting patches would be:
1. breakup __swap_writepage() and un-static __add_to_swap_cache() to
allow resuming of swap page writeback
2. change __swap_writepage() signature to include end_write_func and
un-static end_swap_bio_write() for accounting of outstanding flushes

Is that what you'd like to see?

Seth

 
 Acked-by: Minchan Kim minc...@kernel.org
 
 ---
  include/linux/swap.h |4 
  mm/page_io.c |   22 +-
  mm/swap_state.c  |2 +-
  3 files changed, 22 insertions(+), 6 deletions(-)

 diff --git a/include/linux/swap.h b/include/linux/swap.h
 index 8c66486..a3da829 100644
 --- a/include/linux/swap.h
 +++ b/include/linux/swap.h
 @@ -321,6 +321,9 @@ static inline void mem_cgroup_uncharge_swap(swp_entry_t 
 ent)
  /* linux/mm/page_io.c */
  extern int swap_readpage(struct page *);
  extern int swap_writepage(struct page *page, struct writeback_control *wbc);
 +extern void end_swap_bio_write(struct bio *bio, int err);
 +extern int __swap_writepage(struct page *page, struct writeback_control 
 *wbc,
 +void (*end_write_func)(struct bio *, int));
  extern int swap_set_page_dirty(struct page *page);
  extern void end_swap_bio_read(struct bio *bio, int err);
  
 @@ -335,6 +338,7 @@ extern struct address_space swapper_space;
  extern void show_swap_cache_info(void);
  extern int add_to_swap(struct page *);
  extern int add_to_swap_cache(struct page *, swp_entry_t, gfp_t);
 +extern int __add_to_swap_cache(struct page *page, swp_entry_t entry);
 
 What's related __add_to_swap_cache with this patch?
 
  extern void __delete_from_swap_cache(struct page *);
  extern void delete_from_swap_cache(struct page *);
  extern void free_page_and_swap_cache(struct page *);
 diff --git a/mm/page_io.c b/mm/page_io.c
 index c535d39..806085e 100644
 --- a/mm/page_io.c
 +++ b/mm/page_io.c
 @@ -43,7 +43,7 @@ static struct bio *get_swap_bio(gfp_t gfp_flags,
  return bio;
  }
  
 -static void end_swap_bio_write(struct bio *bio, int err)
 +void end_swap_bio_write(struct bio *bio, int err)
 
 Why do you remove static in this patch? It's not related to the patch.
 
  {
  const int uptodate = test_bit(BIO_UPTODATE, bio-bi_flags);
  struct page *page = bio-bi_io_vec[0].bv_page;
 @@ -180,15 +180,16 @@ bad_bmap:
  goto out;
  }
  
 +int __swap_writepage(struct page *page, struct writeback_control *wbc,
 +void (*end_write_func)(struct bio *, int));
 +
  /*
   * We may have stale swap cache pages in memory: notice
   * them here and get rid of the unnecessary final write.
   */
  int swap_writepage(struct page *page, struct writeback_control *wbc)
  {
 -struct bio *bio;
 -int ret = 0, rw = WRITE;
 -struct swap_info_struct *sis = page_swap_info(page);
 +int ret = 0;
  
  if (try_to_free_swap(page)) {
  unlock_page(page);
 @@ -200,6 +201,17 @@ int swap_writepage(struct page *page, struct 
 writeback_control *wbc)
  end_page_writeback(page);
  goto out;
  }
 +ret = __swap_writepage(page, wbc, end_swap_bio_write);
 +out:
 +return ret;
 +}
 +
 +int __swap_writepage(struct page *page, struct writeback_control *wbc,
 +void (*end_write_func)(struct bio *, int))
 +{
 +struct bio *bio;
 +int ret = 0, rw = WRITE;
 +struct swap_info_struct *sis = page_swap_info(page);
  
  if (sis-flags  SWP_FILE) {
  struct kiocb kiocb;
 @@ -227,7 +239,7 @@ int swap_writepage(struct page *page, struct 
 writeback_control *wbc)
  return ret;
  }
  
 -bio = get_swap_bio(GFP_NOIO, page, end_swap_bio_write);
 +bio = get_swap_bio(GFP_NOIO, page, end_write_func);
  if (bio == NULL) {
  set_page_dirty(page);
  unlock_page(page);
 diff --git a/mm/swap_state.c b/mm/swap_state.c
 index 0cb36fb..7eded9c 100644
 --- a/mm/swap_state.c
 +++ b/mm/swap_state.c
 @@ -67,7 +67,7

Re: [PATCHv2 7/9] mm: break up swap_writepage() for frontswap backends

2013-01-28 Thread Minchan Kim 
On Mon, Jan 28, 2013 at 11:26:43AM -0600, Seth Jennings wrote:
 On 01/27/2013 10:22 PM, Minchan Kim wrote:
  On Mon, Jan 07, 2013 at 02:24:38PM -0600, Seth Jennings wrote:
  swap_writepage() is currently where frontswap hooks into the swap
  write path to capture pages with the frontswap_store() function.
  However, if a frontswap backend wants to resume the writeback of
  a page to the swap device, it can't call swap_writepage() as
  the page will simply reenter the backend.
 
  This patch separates swap_writepage() into a top and bottom half, the
  bottom half named __swap_writepage() to allow a frontswap backend,
  like zswap, to resume writeback beyond the frontswap_store() hook and
  by notified when the writeback completes.
 
  Signed-off-by: Seth Jennings sjenn...@linux.vnet.ibm.com
  
  Looks good to me except few nitpicks.
 
 I broke these changes out from the main zswap patch (patch 8/9) in
 response to a request for Dan.  You are right in that there are
 changes here unrelated to the commit message.
 
 The other changes are related to allowing zswap to do accounting on
 the number of outstanding flushes to the swap device.
 
 If I'm going to break those out though the two resulting patches would be:
 1. breakup __swap_writepage() and un-static __add_to_swap_cache() to
 allow resuming of swap page writeback
 2. change __swap_writepage() signature to include end_write_func and
 un-static end_swap_bio_write() for accounting of outstanding flushes
 

Or please add comment why you want to export some functions.

Thanks!

 Is that what you'd like to see?
 
 Seth
 
  
  Acked-by: Minchan Kim minc...@kernel.org
  
  ---
   include/linux/swap.h |4 
   mm/page_io.c |   22 +-
   mm/swap_state.c  |2 +-
   3 files changed, 22 insertions(+), 6 deletions(-)
 
  diff --git a/include/linux/swap.h b/include/linux/swap.h
  index 8c66486..a3da829 100644
  --- a/include/linux/swap.h
  +++ b/include/linux/swap.h
  @@ -321,6 +321,9 @@ static inline void 
  mem_cgroup_uncharge_swap(swp_entry_t ent)
   /* linux/mm/page_io.c */
   extern int swap_readpage(struct page *);
   extern int swap_writepage(struct page *page, struct writeback_control 
  *wbc);
  +extern void end_swap_bio_write(struct bio *bio, int err);
  +extern int __swap_writepage(struct page *page, struct writeback_control 
  *wbc,
  +  void (*end_write_func)(struct bio *, int));
   extern int swap_set_page_dirty(struct page *page);
   extern void end_swap_bio_read(struct bio *bio, int err);
   
  @@ -335,6 +338,7 @@ extern struct address_space swapper_space;
   extern void show_swap_cache_info(void);
   extern int add_to_swap(struct page *);
   extern int add_to_swap_cache(struct page *, swp_entry_t, gfp_t);
  +extern int __add_to_swap_cache(struct page *page, swp_entry_t entry);
  
  What's related __add_to_swap_cache with this patch?
  
   extern void __delete_from_swap_cache(struct page *);
   extern void delete_from_swap_cache(struct page *);
   extern void free_page_and_swap_cache(struct page *);
  diff --git a/mm/page_io.c b/mm/page_io.c
  index c535d39..806085e 100644
  --- a/mm/page_io.c
  +++ b/mm/page_io.c
  @@ -43,7 +43,7 @@ static struct bio *get_swap_bio(gfp_t gfp_flags,
 return bio;
   }
   
  -static void end_swap_bio_write(struct bio *bio, int err)
  +void end_swap_bio_write(struct bio *bio, int err)
  
  Why do you remove static in this patch? It's not related to the patch.
  
   {
 const int uptodate = test_bit(BIO_UPTODATE, bio-bi_flags);
 struct page *page = bio-bi_io_vec[0].bv_page;
  @@ -180,15 +180,16 @@ bad_bmap:
 goto out;
   }
   
  +int __swap_writepage(struct page *page, struct writeback_control *wbc,
  +  void (*end_write_func)(struct bio *, int));
  +
   /*
* We may have stale swap cache pages in memory: notice
* them here and get rid of the unnecessary final write.
*/
   int swap_writepage(struct page *page, struct writeback_control *wbc)
   {
  -  struct bio *bio;
  -  int ret = 0, rw = WRITE;
  -  struct swap_info_struct *sis = page_swap_info(page);
  +  int ret = 0;
   
 if (try_to_free_swap(page)) {
 unlock_page(page);
  @@ -200,6 +201,17 @@ int swap_writepage(struct page *page, struct 
  writeback_control *wbc)
 end_page_writeback(page);
 goto out;
 }
  +  ret = __swap_writepage(page, wbc, end_swap_bio_write);
  +out:
  +  return ret;
  +}
  +
  +int __swap_writepage(struct page *page, struct writeback_control *wbc,
  +  void (*end_write_func)(struct bio *, int))
  +{
  +  struct bio *bio;
  +  int ret = 0, rw = WRITE;
  +  struct swap_info_struct *sis = page_swap_info(page);
   
 if (sis-flags  SWP_FILE) {
 struct kiocb kiocb;
  @@ -227,7 +239,7 @@ int swap_writepage(struct page *page, struct 
  writeback_control *wbc)
 return ret;
 }
   
  -  bio = get_swap_bio(GFP_NOIO, page, end_swap_bio_write);
  +  bio = get_swap_bio(GFP_NOIO, page, end_write_func);
 if (bio ==

Re: [PATCHv2 7/9] mm: break up swap_writepage() for frontswap backends

2013-01-27 Thread Minchan Kim 
On Mon, Jan 07, 2013 at 02:24:38PM -0600, Seth Jennings wrote:
> swap_writepage() is currently where frontswap hooks into the swap
> write path to capture pages with the frontswap_store() function.
> However, if a frontswap backend wants to "resume" the writeback of
> a page to the swap device, it can't call swap_writepage() as
> the page will simply reenter the backend.
> 
> This patch separates swap_writepage() into a top and bottom half, the
> bottom half named __swap_writepage() to allow a frontswap backend,
> like zswap, to resume writeback beyond the frontswap_store() hook and
> by notified when the writeback completes.
> 
> Signed-off-by: Seth Jennings 

Looks good to me except few nitpicks.

Acked-by: Minchan Kim 

> ---
>  include/linux/swap.h |4 
>  mm/page_io.c |   22 +-
>  mm/swap_state.c  |2 +-
>  3 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index 8c66486..a3da829 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -321,6 +321,9 @@ static inline void mem_cgroup_uncharge_swap(swp_entry_t 
> ent)
>  /* linux/mm/page_io.c */
>  extern int swap_readpage(struct page *);
>  extern int swap_writepage(struct page *page, struct writeback_control *wbc);
> +extern void end_swap_bio_write(struct bio *bio, int err);
> +extern int __swap_writepage(struct page *page, struct writeback_control *wbc,
> + void (*end_write_func)(struct bio *, int));
>  extern int swap_set_page_dirty(struct page *page);
>  extern void end_swap_bio_read(struct bio *bio, int err);
>  
> @@ -335,6 +338,7 @@ extern struct address_space swapper_space;
>  extern void show_swap_cache_info(void);
>  extern int add_to_swap(struct page *);
>  extern int add_to_swap_cache(struct page *, swp_entry_t, gfp_t);
> +extern int __add_to_swap_cache(struct page *page, swp_entry_t entry);

What's related __add_to_swap_cache with this patch?

>  extern void __delete_from_swap_cache(struct page *);
>  extern void delete_from_swap_cache(struct page *);
>  extern void free_page_and_swap_cache(struct page *);
> diff --git a/mm/page_io.c b/mm/page_io.c
> index c535d39..806085e 100644
> --- a/mm/page_io.c
> +++ b/mm/page_io.c
> @@ -43,7 +43,7 @@ static struct bio *get_swap_bio(gfp_t gfp_flags,
>   return bio;
>  }
>  
> -static void end_swap_bio_write(struct bio *bio, int err)
> +void end_swap_bio_write(struct bio *bio, int err)

Why do you remove static in this patch? It's not related to the patch.

>  {
>   const int uptodate = test_bit(BIO_UPTODATE, >bi_flags);
>   struct page *page = bio->bi_io_vec[0].bv_page;
> @@ -180,15 +180,16 @@ bad_bmap:
>   goto out;
>  }
>  
> +int __swap_writepage(struct page *page, struct writeback_control *wbc,
> + void (*end_write_func)(struct bio *, int));
> +
>  /*
>   * We may have stale swap cache pages in memory: notice
>   * them here and get rid of the unnecessary final write.
>   */
>  int swap_writepage(struct page *page, struct writeback_control *wbc)
>  {
> - struct bio *bio;
> - int ret = 0, rw = WRITE;
> - struct swap_info_struct *sis = page_swap_info(page);
> + int ret = 0;
>  
>   if (try_to_free_swap(page)) {
>   unlock_page(page);
> @@ -200,6 +201,17 @@ int swap_writepage(struct page *page, struct 
> writeback_control *wbc)
>   end_page_writeback(page);
>   goto out;
>   }
> + ret = __swap_writepage(page, wbc, end_swap_bio_write);
> +out:
> + return ret;
> +}
> +
> +int __swap_writepage(struct page *page, struct writeback_control *wbc,
> + void (*end_write_func)(struct bio *, int))
> +{
> + struct bio *bio;
> + int ret = 0, rw = WRITE;
> + struct swap_info_struct *sis = page_swap_info(page);
>  
>   if (sis->flags & SWP_FILE) {
>   struct kiocb kiocb;
> @@ -227,7 +239,7 @@ int swap_writepage(struct page *page, struct 
> writeback_control *wbc)
>   return ret;
>   }
>  
> - bio = get_swap_bio(GFP_NOIO, page, end_swap_bio_write);
> + bio = get_swap_bio(GFP_NOIO, page, end_write_func);
>   if (bio == NULL) {
>   set_page_dirty(page);
>   unlock_page(page);
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 0cb36fb..7eded9c 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -67,7 +67,7 @@ void show_swap_cache_info(void)
>   * __add_to_swap_cache resembles add_to_page_cache_locked on swapper_space,
>   * but sets SwapCache flag and private instead of mapping and index.
>   */
> -static int __add_to_swap_cache(struct page *page, swp_entry_t entry)
> +int __add_to_swap_cache(struct page *page, swp_entry_t entry)

Ditto

>  {
>   int error;
>  
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

-- 
Kind

Re: [PATCHv2 7/9] mm: break up swap_writepage() for frontswap backends

2013-01-27 Thread Minchan Kim 
On Mon, Jan 07, 2013 at 02:24:38PM -0600, Seth Jennings wrote:
 swap_writepage() is currently where frontswap hooks into the swap
 write path to capture pages with the frontswap_store() function.
 However, if a frontswap backend wants to resume the writeback of
 a page to the swap device, it can't call swap_writepage() as
 the page will simply reenter the backend.
 
 This patch separates swap_writepage() into a top and bottom half, the
 bottom half named __swap_writepage() to allow a frontswap backend,
 like zswap, to resume writeback beyond the frontswap_store() hook and
 by notified when the writeback completes.
 
 Signed-off-by: Seth Jennings sjenn...@linux.vnet.ibm.com

Looks good to me except few nitpicks.

Acked-by: Minchan Kim minc...@kernel.org

 ---
  include/linux/swap.h |4 
  mm/page_io.c |   22 +-
  mm/swap_state.c  |2 +-
  3 files changed, 22 insertions(+), 6 deletions(-)
 
 diff --git a/include/linux/swap.h b/include/linux/swap.h
 index 8c66486..a3da829 100644
 --- a/include/linux/swap.h
 +++ b/include/linux/swap.h
 @@ -321,6 +321,9 @@ static inline void mem_cgroup_uncharge_swap(swp_entry_t 
 ent)
  /* linux/mm/page_io.c */
  extern int swap_readpage(struct page *);
  extern int swap_writepage(struct page *page, struct writeback_control *wbc);
 +extern void end_swap_bio_write(struct bio *bio, int err);
 +extern int __swap_writepage(struct page *page, struct writeback_control *wbc,
 + void (*end_write_func)(struct bio *, int));
  extern int swap_set_page_dirty(struct page *page);
  extern void end_swap_bio_read(struct bio *bio, int err);
  
 @@ -335,6 +338,7 @@ extern struct address_space swapper_space;
  extern void show_swap_cache_info(void);
  extern int add_to_swap(struct page *);
  extern int add_to_swap_cache(struct page *, swp_entry_t, gfp_t);
 +extern int __add_to_swap_cache(struct page *page, swp_entry_t entry);

What's related __add_to_swap_cache with this patch?

  extern void __delete_from_swap_cache(struct page *);
  extern void delete_from_swap_cache(struct page *);
  extern void free_page_and_swap_cache(struct page *);
 diff --git a/mm/page_io.c b/mm/page_io.c
 index c535d39..806085e 100644
 --- a/mm/page_io.c
 +++ b/mm/page_io.c
 @@ -43,7 +43,7 @@ static struct bio *get_swap_bio(gfp_t gfp_flags,
   return bio;
  }
  
 -static void end_swap_bio_write(struct bio *bio, int err)
 +void end_swap_bio_write(struct bio *bio, int err)

Why do you remove static in this patch? It's not related to the patch.

  {
   const int uptodate = test_bit(BIO_UPTODATE, bio-bi_flags);
   struct page *page = bio-bi_io_vec[0].bv_page;
 @@ -180,15 +180,16 @@ bad_bmap:
   goto out;
  }
  
 +int __swap_writepage(struct page *page, struct writeback_control *wbc,
 + void (*end_write_func)(struct bio *, int));
 +
  /*
   * We may have stale swap cache pages in memory: notice
   * them here and get rid of the unnecessary final write.
   */
  int swap_writepage(struct page *page, struct writeback_control *wbc)
  {
 - struct bio *bio;
 - int ret = 0, rw = WRITE;
 - struct swap_info_struct *sis = page_swap_info(page);
 + int ret = 0;
  
   if (try_to_free_swap(page)) {
   unlock_page(page);
 @@ -200,6 +201,17 @@ int swap_writepage(struct page *page, struct 
 writeback_control *wbc)
   end_page_writeback(page);
   goto out;
   }
 + ret = __swap_writepage(page, wbc, end_swap_bio_write);
 +out:
 + return ret;
 +}
 +
 +int __swap_writepage(struct page *page, struct writeback_control *wbc,
 + void (*end_write_func)(struct bio *, int))
 +{
 + struct bio *bio;
 + int ret = 0, rw = WRITE;
 + struct swap_info_struct *sis = page_swap_info(page);
  
   if (sis-flags  SWP_FILE) {
   struct kiocb kiocb;
 @@ -227,7 +239,7 @@ int swap_writepage(struct page *page, struct 
 writeback_control *wbc)
   return ret;
   }
  
 - bio = get_swap_bio(GFP_NOIO, page, end_swap_bio_write);
 + bio = get_swap_bio(GFP_NOIO, page, end_write_func);
   if (bio == NULL) {
   set_page_dirty(page);
   unlock_page(page);
 diff --git a/mm/swap_state.c b/mm/swap_state.c
 index 0cb36fb..7eded9c 100644
 --- a/mm/swap_state.c
 +++ b/mm/swap_state.c
 @@ -67,7 +67,7 @@ void show_swap_cache_info(void)
   * __add_to_swap_cache resembles add_to_page_cache_locked on swapper_space,
   * but sets SwapCache flag and private instead of mapping and index.
   */
 -static int __add_to_swap_cache(struct page *page, swp_entry_t entry)
 +int __add_to_swap_cache(struct page *page, swp_entry_t entry)

Ditto

  {
   int error;
  
 -- 
 1.7.9.5
 
 --
 To unsubscribe, send a message with 'unsubscribe linux-mm' in
 the body to majord...@kvack.org.  For more info on Linux MM,
 see: http://www.linux-mm.org/ .
 Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a

-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line

[PATCHv2 7/9] mm: break up swap_writepage() for frontswap backends

2013-01-07 Thread Seth Jennings 
swap_writepage() is currently where frontswap hooks into the swap
write path to capture pages with the frontswap_store() function.
However, if a frontswap backend wants to "resume" the writeback of
a page to the swap device, it can't call swap_writepage() as
the page will simply reenter the backend.

This patch separates swap_writepage() into a top and bottom half, the
bottom half named __swap_writepage() to allow a frontswap backend,
like zswap, to resume writeback beyond the frontswap_store() hook and
by notified when the writeback completes.

Signed-off-by: Seth Jennings 
---
 include/linux/swap.h |4 
 mm/page_io.c |   22 +-
 mm/swap_state.c  |2 +-
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index 8c66486..a3da829 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -321,6 +321,9 @@ static inline void mem_cgroup_uncharge_swap(swp_entry_t ent)
 /* linux/mm/page_io.c */
 extern int swap_readpage(struct page *);
 extern int swap_writepage(struct page *page, struct writeback_control *wbc);
+extern void end_swap_bio_write(struct bio *bio, int err);
+extern int __swap_writepage(struct page *page, struct writeback_control *wbc,
+   void (*end_write_func)(struct bio *, int));
 extern int swap_set_page_dirty(struct page *page);
 extern void end_swap_bio_read(struct bio *bio, int err);
 
@@ -335,6 +338,7 @@ extern struct address_space swapper_space;
 extern void show_swap_cache_info(void);
 extern int add_to_swap(struct page *);
 extern int add_to_swap_cache(struct page *, swp_entry_t, gfp_t);
+extern int __add_to_swap_cache(struct page *page, swp_entry_t entry);
 extern void __delete_from_swap_cache(struct page *);
 extern void delete_from_swap_cache(struct page *);
 extern void free_page_and_swap_cache(struct page *);
diff --git a/mm/page_io.c b/mm/page_io.c
index c535d39..806085e 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -43,7 +43,7 @@ static struct bio *get_swap_bio(gfp_t gfp_flags,
return bio;
 }
 
-static void end_swap_bio_write(struct bio *bio, int err)
+void end_swap_bio_write(struct bio *bio, int err)
 {
const int uptodate = test_bit(BIO_UPTODATE, >bi_flags);
struct page *page = bio->bi_io_vec[0].bv_page;
@@ -180,15 +180,16 @@ bad_bmap:
goto out;
 }
 
+int __swap_writepage(struct page *page, struct writeback_control *wbc,
+   void (*end_write_func)(struct bio *, int));
+
 /*
  * We may have stale swap cache pages in memory: notice
  * them here and get rid of the unnecessary final write.
  */
 int swap_writepage(struct page *page, struct writeback_control *wbc)
 {
-   struct bio *bio;
-   int ret = 0, rw = WRITE;
-   struct swap_info_struct *sis = page_swap_info(page);
+   int ret = 0;
 
if (try_to_free_swap(page)) {
unlock_page(page);
@@ -200,6 +201,17 @@ int swap_writepage(struct page *page, struct 
writeback_control *wbc)
end_page_writeback(page);
goto out;
}
+   ret = __swap_writepage(page, wbc, end_swap_bio_write);
+out:
+   return ret;
+}
+
+int __swap_writepage(struct page *page, struct writeback_control *wbc,
+   void (*end_write_func)(struct bio *, int))
+{
+   struct bio *bio;
+   int ret = 0, rw = WRITE;
+   struct swap_info_struct *sis = page_swap_info(page);
 
if (sis->flags & SWP_FILE) {
struct kiocb kiocb;
@@ -227,7 +239,7 @@ int swap_writepage(struct page *page, struct 
writeback_control *wbc)
return ret;
}
 
-   bio = get_swap_bio(GFP_NOIO, page, end_swap_bio_write);
+   bio = get_swap_bio(GFP_NOIO, page, end_write_func);
if (bio == NULL) {
set_page_dirty(page);
unlock_page(page);
diff --git a/mm/swap_state.c b/mm/swap_state.c
index 0cb36fb..7eded9c 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -67,7 +67,7 @@ void show_swap_cache_info(void)
  * __add_to_swap_cache resembles add_to_page_cache_locked on swapper_space,
  * but sets SwapCache flag and private instead of mapping and index.
  */
-static int __add_to_swap_cache(struct page *page, swp_entry_t entry)
+int __add_to_swap_cache(struct page *page, swp_entry_t entry)
 {
int error;
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCHv2 7/9] mm: break up swap_writepage() for frontswap backends

2013-01-07 Thread Seth Jennings 
swap_writepage() is currently where frontswap hooks into the swap
write path to capture pages with the frontswap_store() function.
However, if a frontswap backend wants to resume the writeback of
a page to the swap device, it can't call swap_writepage() as
the page will simply reenter the backend.

This patch separates swap_writepage() into a top and bottom half, the
bottom half named __swap_writepage() to allow a frontswap backend,
like zswap, to resume writeback beyond the frontswap_store() hook and
by notified when the writeback completes.

Signed-off-by: Seth Jennings sjenn...@linux.vnet.ibm.com
---
 include/linux/swap.h |4 
 mm/page_io.c |   22 +-
 mm/swap_state.c  |2 +-
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index 8c66486..a3da829 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -321,6 +321,9 @@ static inline void mem_cgroup_uncharge_swap(swp_entry_t ent)
 /* linux/mm/page_io.c */
 extern int swap_readpage(struct page *);
 extern int swap_writepage(struct page *page, struct writeback_control *wbc);
+extern void end_swap_bio_write(struct bio *bio, int err);
+extern int __swap_writepage(struct page *page, struct writeback_control *wbc,
+   void (*end_write_func)(struct bio *, int));
 extern int swap_set_page_dirty(struct page *page);
 extern void end_swap_bio_read(struct bio *bio, int err);
 
@@ -335,6 +338,7 @@ extern struct address_space swapper_space;
 extern void show_swap_cache_info(void);
 extern int add_to_swap(struct page *);
 extern int add_to_swap_cache(struct page *, swp_entry_t, gfp_t);
+extern int __add_to_swap_cache(struct page *page, swp_entry_t entry);
 extern void __delete_from_swap_cache(struct page *);
 extern void delete_from_swap_cache(struct page *);
 extern void free_page_and_swap_cache(struct page *);
diff --git a/mm/page_io.c b/mm/page_io.c
index c535d39..806085e 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -43,7 +43,7 @@ static struct bio *get_swap_bio(gfp_t gfp_flags,
return bio;
 }
 
-static void end_swap_bio_write(struct bio *bio, int err)
+void end_swap_bio_write(struct bio *bio, int err)
 {
const int uptodate = test_bit(BIO_UPTODATE, bio-bi_flags);
struct page *page = bio-bi_io_vec[0].bv_page;
@@ -180,15 +180,16 @@ bad_bmap:
goto out;
 }
 
+int __swap_writepage(struct page *page, struct writeback_control *wbc,
+   void (*end_write_func)(struct bio *, int));
+
 /*
  * We may have stale swap cache pages in memory: notice
  * them here and get rid of the unnecessary final write.
  */
 int swap_writepage(struct page *page, struct writeback_control *wbc)
 {
-   struct bio *bio;
-   int ret = 0, rw = WRITE;
-   struct swap_info_struct *sis = page_swap_info(page);
+   int ret = 0;
 
if (try_to_free_swap(page)) {
unlock_page(page);
@@ -200,6 +201,17 @@ int swap_writepage(struct page *page, struct 
writeback_control *wbc)
end_page_writeback(page);
goto out;
}
+   ret = __swap_writepage(page, wbc, end_swap_bio_write);
+out:
+   return ret;
+}
+
+int __swap_writepage(struct page *page, struct writeback_control *wbc,
+   void (*end_write_func)(struct bio *, int))
+{
+   struct bio *bio;
+   int ret = 0, rw = WRITE;
+   struct swap_info_struct *sis = page_swap_info(page);
 
if (sis-flags  SWP_FILE) {
struct kiocb kiocb;
@@ -227,7 +239,7 @@ int swap_writepage(struct page *page, struct 
writeback_control *wbc)
return ret;
}
 
-   bio = get_swap_bio(GFP_NOIO, page, end_swap_bio_write);
+   bio = get_swap_bio(GFP_NOIO, page, end_write_func);
if (bio == NULL) {
set_page_dirty(page);
unlock_page(page);
diff --git a/mm/swap_state.c b/mm/swap_state.c
index 0cb36fb..7eded9c 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -67,7 +67,7 @@ void show_swap_cache_info(void)
  * __add_to_swap_cache resembles add_to_page_cache_locked on swapper_space,
  * but sets SwapCache flag and private instead of mapping and index.
  */
-static int __add_to_swap_cache(struct page *page, swp_entry_t entry)
+int __add_to_swap_cache(struct page *page, swp_entry_t entry)
 {
int error;
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/