Re: [PATCH v1 1/5] zram: generalize swap_slot_free_notify

2014-09-22 Thread Minchan Kim
Hi Andrew,

On Mon, Sep 22, 2014 at 01:41:09PM -0700, Andrew Morton wrote:
> On Mon, 22 Sep 2014 09:03:07 +0900 Minchan Kim  wrote:
> 
> > Currently, swap_slot_free_notify is used for zram to free
> > duplicated copy page for memory efficiency when it knows
> > there is no reference to the swap slot.
> > 
> > This patch generalizes it to be able to use for other
> > swap hint to communicate with VM.
> > 
> 
> I really think we need to do a better job of documenting the code.
> 
> > index 94d93b1f8b53..c262bfbeafa9 100644
> > --- a/Documentation/filesystems/Locking
> > +++ b/Documentation/filesystems/Locking
> > @@ -405,7 +405,7 @@ prototypes:
> > void (*unlock_native_capacity) (struct gendisk *);
> > int (*revalidate_disk) (struct gendisk *);
> > int (*getgeo)(struct block_device *, struct hd_geometry *);
> > -   void (*swap_slot_free_notify) (struct block_device *, unsigned long);
> > +   int (*swap_hint) (struct block_device *, unsigned int, void *);
> >  
> >  locking rules:
> > bd_mutex
> > @@ -418,7 +418,7 @@ media_changed:  no
> >  unlock_native_capacity:no
> >  revalidate_disk:   no
> >  getgeo:no
> > -swap_slot_free_notify: no  (see below)
> > +swap_hint: no  (see below)
> 
> This didn't tell anyone anythnig much.

Yeb. :(

> 
> > index d78b245bae06..22a37764c409 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -926,7 +926,8 @@ error:
> > bio_io_error(bio);
> >  }
> >  
> > -static void zram_slot_free_notify(struct block_device *bdev,
> > +/* this callback is with swap_lock and sometimes page table lock held */
> 
> OK, that was useful.
> 
> It's called "page_table_lock".
> 
> Also *which* page_table_lock?  current->mm?

It depends on ALLOC_SPLIT_PTLOCKS so it could be page->ptl, too.
So, it would be better to call it as *ptlock*?
Since it's ptlock, it isn't related to which mm struct.
What we should make sure is just ptlock which belong to the page
table pointed to this swap page.

So, I want this.

diff --git a/Documentation/filesystems/Locking 
b/Documentation/filesystems/Locking
index c262bfbeafa9..19d2726e34f4 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -423,8 +423,8 @@ swap_hint:  no  (see below)
 media_changed, unlock_native_capacity and revalidate_disk are called only from
 check_disk_change().
 
-swap_slot_free_notify is called with swap_lock and sometimes the page lock
-held.
+swap_hint is called with swap_info_struct->lock and sometimes the ptlock
+of the page table pointed to the swap page.
 
 
 --- file_operations ---

> 
> > +static int zram_slot_free_notify(struct block_device *bdev,
> > unsigned long index)
> >  {
> > struct zram *zram;
> >
> > ...
> >
> > --- a/include/linux/blkdev.h
> > +++ b/include/linux/blkdev.h
> > @@ -1609,6 +1609,10 @@ static inline bool 
> > blk_integrity_is_initialized(struct gendisk *g)
> >  
> >  #endif /* CONFIG_BLK_DEV_INTEGRITY */
> >  
> > +enum swap_blk_hint {
> > +   SWAP_FREE,
> > +};
> 
> This would be a great place to document SWAP_FREE.

Yes,

> 
> >  struct block_device_operations {
> > int (*open) (struct block_device *, fmode_t);
> > void (*release) (struct gendisk *, fmode_t);
> > @@ -1624,8 +1628,7 @@ struct block_device_operations {
> > void (*unlock_native_capacity) (struct gendisk *);
> > int (*revalidate_disk) (struct gendisk *);
> > int (*getgeo)(struct block_device *, struct hd_geometry *);
> > -   /* this callback is with swap_lock and sometimes page table lock held */
> > -   void (*swap_slot_free_notify) (struct block_device *, unsigned long);
> > +   int (*swap_hint)(struct block_device *, unsigned int, void *);
> 
> And this would be a suitable place to document ->swap_hint().

If we consider to be able to add more hints in future so it could
be verbose, IMO, it would be better to desribe it in enum swap_hint. :)

> 
> - Hint from who to who?  Is it the caller providing the callee a hint
>   or is the caller asking the callee for a hint?
> 
> - What is the meaning of the return value?
> 
> - What are the meaning of the arguments?

Okay.

> 
> Please don't omit the argument names like this.  They are useful!  How
> is a reader to know what that "unsigned int" and "void *" actually
> *do*?

Yes.

> 
> The second arg-which-doesn't-have-a-name should have had type
> swap_blk_hint, yes?

Yes.

> 
> swap_blk_hint should be called swap_block_hint.  I assume that's what
> "blk" means.  Why does the name have "block" in there anyway?  It has
> something to do with disk blocks?  How is anyone supposed to work that
> out?

Yeb, I think we don't need block in name. I will remove it.

> 
> ->swap_hint was converted to return an `int', but all the callers
> simply ignore the return value.

You're right. All caller doesn't use it in this patch but this 

Re: [PATCH v1 1/5] zram: generalize swap_slot_free_notify

2014-09-22 Thread Andrew Morton
On Mon, 22 Sep 2014 09:03:07 +0900 Minchan Kim  wrote:

> Currently, swap_slot_free_notify is used for zram to free
> duplicated copy page for memory efficiency when it knows
> there is no reference to the swap slot.
> 
> This patch generalizes it to be able to use for other
> swap hint to communicate with VM.
> 

I really think we need to do a better job of documenting the code.

> index 94d93b1f8b53..c262bfbeafa9 100644
> --- a/Documentation/filesystems/Locking
> +++ b/Documentation/filesystems/Locking
> @@ -405,7 +405,7 @@ prototypes:
>   void (*unlock_native_capacity) (struct gendisk *);
>   int (*revalidate_disk) (struct gendisk *);
>   int (*getgeo)(struct block_device *, struct hd_geometry *);
> - void (*swap_slot_free_notify) (struct block_device *, unsigned long);
> + int (*swap_hint) (struct block_device *, unsigned int, void *);
>  
>  locking rules:
>   bd_mutex
> @@ -418,7 +418,7 @@ media_changed:no
>  unlock_native_capacity:  no
>  revalidate_disk: no
>  getgeo:  no
> -swap_slot_free_notify:   no  (see below)
> +swap_hint:   no  (see below)

This didn't tell anyone anythnig much.

> index d78b245bae06..22a37764c409 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -926,7 +926,8 @@ error:
>   bio_io_error(bio);
>  }
>  
> -static void zram_slot_free_notify(struct block_device *bdev,
> +/* this callback is with swap_lock and sometimes page table lock held */

OK, that was useful.

It's called "page_table_lock".

Also *which* page_table_lock?  current->mm?

> +static int zram_slot_free_notify(struct block_device *bdev,
>   unsigned long index)
>  {
>   struct zram *zram;
>
> ...
>
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1609,6 +1609,10 @@ static inline bool blk_integrity_is_initialized(struct 
> gendisk *g)
>  
>  #endif /* CONFIG_BLK_DEV_INTEGRITY */
>  
> +enum swap_blk_hint {
> + SWAP_FREE,
> +};

This would be a great place to document SWAP_FREE.

>  struct block_device_operations {
>   int (*open) (struct block_device *, fmode_t);
>   void (*release) (struct gendisk *, fmode_t);
> @@ -1624,8 +1628,7 @@ struct block_device_operations {
>   void (*unlock_native_capacity) (struct gendisk *);
>   int (*revalidate_disk) (struct gendisk *);
>   int (*getgeo)(struct block_device *, struct hd_geometry *);
> - /* this callback is with swap_lock and sometimes page table lock held */
> - void (*swap_slot_free_notify) (struct block_device *, unsigned long);
> + int (*swap_hint)(struct block_device *, unsigned int, void *);

And this would be a suitable place to document ->swap_hint().

- Hint from who to who?  Is it the caller providing the callee a hint
  or is the caller asking the callee for a hint?

- What is the meaning of the return value?

- What are the meaning of the arguments?

Please don't omit the argument names like this.  They are useful!  How
is a reader to know what that "unsigned int" and "void *" actually
*do*?

The second arg-which-doesn't-have-a-name should have had type
swap_blk_hint, yes?

swap_blk_hint should be called swap_block_hint.  I assume that's what
"blk" means.  Why does the name have "block" in there anyway?  It has
something to do with disk blocks?  How is anyone supposed to work that
out?

->swap_hint was converted to return an `int', but all the callers
simply ignore the return value.

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


Re: [PATCH v1 1/5] zram: generalize swap_slot_free_notify

2014-09-22 Thread Andrew Morton
On Mon, 22 Sep 2014 09:03:07 +0900 Minchan Kim minc...@kernel.org wrote:

 Currently, swap_slot_free_notify is used for zram to free
 duplicated copy page for memory efficiency when it knows
 there is no reference to the swap slot.
 
 This patch generalizes it to be able to use for other
 swap hint to communicate with VM.
 

I really think we need to do a better job of documenting the code.

 index 94d93b1f8b53..c262bfbeafa9 100644
 --- a/Documentation/filesystems/Locking
 +++ b/Documentation/filesystems/Locking
 @@ -405,7 +405,7 @@ prototypes:
   void (*unlock_native_capacity) (struct gendisk *);
   int (*revalidate_disk) (struct gendisk *);
   int (*getgeo)(struct block_device *, struct hd_geometry *);
 - void (*swap_slot_free_notify) (struct block_device *, unsigned long);
 + int (*swap_hint) (struct block_device *, unsigned int, void *);
  
  locking rules:
   bd_mutex
 @@ -418,7 +418,7 @@ media_changed:no
  unlock_native_capacity:  no
  revalidate_disk: no
  getgeo:  no
 -swap_slot_free_notify:   no  (see below)
 +swap_hint:   no  (see below)

This didn't tell anyone anythnig much.

 index d78b245bae06..22a37764c409 100644
 --- a/drivers/block/zram/zram_drv.c
 +++ b/drivers/block/zram/zram_drv.c
 @@ -926,7 +926,8 @@ error:
   bio_io_error(bio);
  }
  
 -static void zram_slot_free_notify(struct block_device *bdev,
 +/* this callback is with swap_lock and sometimes page table lock held */

OK, that was useful.

It's called page_table_lock.

Also *which* page_table_lock?  current-mm?

 +static int zram_slot_free_notify(struct block_device *bdev,
   unsigned long index)
  {
   struct zram *zram;

 ...

 --- a/include/linux/blkdev.h
 +++ b/include/linux/blkdev.h
 @@ -1609,6 +1609,10 @@ static inline bool blk_integrity_is_initialized(struct 
 gendisk *g)
  
  #endif /* CONFIG_BLK_DEV_INTEGRITY */
  
 +enum swap_blk_hint {
 + SWAP_FREE,
 +};

This would be a great place to document SWAP_FREE.

  struct block_device_operations {
   int (*open) (struct block_device *, fmode_t);
   void (*release) (struct gendisk *, fmode_t);
 @@ -1624,8 +1628,7 @@ struct block_device_operations {
   void (*unlock_native_capacity) (struct gendisk *);
   int (*revalidate_disk) (struct gendisk *);
   int (*getgeo)(struct block_device *, struct hd_geometry *);
 - /* this callback is with swap_lock and sometimes page table lock held */
 - void (*swap_slot_free_notify) (struct block_device *, unsigned long);
 + int (*swap_hint)(struct block_device *, unsigned int, void *);

And this would be a suitable place to document -swap_hint().

- Hint from who to who?  Is it the caller providing the callee a hint
  or is the caller asking the callee for a hint?

- What is the meaning of the return value?

- What are the meaning of the arguments?

Please don't omit the argument names like this.  They are useful!  How
is a reader to know what that unsigned int and void * actually
*do*?

The second arg-which-doesn't-have-a-name should have had type
swap_blk_hint, yes?

swap_blk_hint should be called swap_block_hint.  I assume that's what
blk means.  Why does the name have block in there anyway?  It has
something to do with disk blocks?  How is anyone supposed to work that
out?

-swap_hint was converted to return an `int', but all the callers
simply ignore the return value.

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


Re: [PATCH v1 1/5] zram: generalize swap_slot_free_notify

2014-09-22 Thread Minchan Kim
Hi Andrew,

On Mon, Sep 22, 2014 at 01:41:09PM -0700, Andrew Morton wrote:
 On Mon, 22 Sep 2014 09:03:07 +0900 Minchan Kim minc...@kernel.org wrote:
 
  Currently, swap_slot_free_notify is used for zram to free
  duplicated copy page for memory efficiency when it knows
  there is no reference to the swap slot.
  
  This patch generalizes it to be able to use for other
  swap hint to communicate with VM.
  
 
 I really think we need to do a better job of documenting the code.
 
  index 94d93b1f8b53..c262bfbeafa9 100644
  --- a/Documentation/filesystems/Locking
  +++ b/Documentation/filesystems/Locking
  @@ -405,7 +405,7 @@ prototypes:
  void (*unlock_native_capacity) (struct gendisk *);
  int (*revalidate_disk) (struct gendisk *);
  int (*getgeo)(struct block_device *, struct hd_geometry *);
  -   void (*swap_slot_free_notify) (struct block_device *, unsigned long);
  +   int (*swap_hint) (struct block_device *, unsigned int, void *);
   
   locking rules:
  bd_mutex
  @@ -418,7 +418,7 @@ media_changed:  no
   unlock_native_capacity:no
   revalidate_disk:   no
   getgeo:no
  -swap_slot_free_notify: no  (see below)
  +swap_hint: no  (see below)
 
 This didn't tell anyone anythnig much.

Yeb. :(

 
  index d78b245bae06..22a37764c409 100644
  --- a/drivers/block/zram/zram_drv.c
  +++ b/drivers/block/zram/zram_drv.c
  @@ -926,7 +926,8 @@ error:
  bio_io_error(bio);
   }
   
  -static void zram_slot_free_notify(struct block_device *bdev,
  +/* this callback is with swap_lock and sometimes page table lock held */
 
 OK, that was useful.
 
 It's called page_table_lock.
 
 Also *which* page_table_lock?  current-mm?

It depends on ALLOC_SPLIT_PTLOCKS so it could be page-ptl, too.
So, it would be better to call it as *ptlock*?
Since it's ptlock, it isn't related to which mm struct.
What we should make sure is just ptlock which belong to the page
table pointed to this swap page.

So, I want this.

diff --git a/Documentation/filesystems/Locking 
b/Documentation/filesystems/Locking
index c262bfbeafa9..19d2726e34f4 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -423,8 +423,8 @@ swap_hint:  no  (see below)
 media_changed, unlock_native_capacity and revalidate_disk are called only from
 check_disk_change().
 
-swap_slot_free_notify is called with swap_lock and sometimes the page lock
-held.
+swap_hint is called with swap_info_struct-lock and sometimes the ptlock
+of the page table pointed to the swap page.
 
 
 --- file_operations ---

 
  +static int zram_slot_free_notify(struct block_device *bdev,
  unsigned long index)
   {
  struct zram *zram;
 
  ...
 
  --- a/include/linux/blkdev.h
  +++ b/include/linux/blkdev.h
  @@ -1609,6 +1609,10 @@ static inline bool 
  blk_integrity_is_initialized(struct gendisk *g)
   
   #endif /* CONFIG_BLK_DEV_INTEGRITY */
   
  +enum swap_blk_hint {
  +   SWAP_FREE,
  +};
 
 This would be a great place to document SWAP_FREE.

Yes,

 
   struct block_device_operations {
  int (*open) (struct block_device *, fmode_t);
  void (*release) (struct gendisk *, fmode_t);
  @@ -1624,8 +1628,7 @@ struct block_device_operations {
  void (*unlock_native_capacity) (struct gendisk *);
  int (*revalidate_disk) (struct gendisk *);
  int (*getgeo)(struct block_device *, struct hd_geometry *);
  -   /* this callback is with swap_lock and sometimes page table lock held */
  -   void (*swap_slot_free_notify) (struct block_device *, unsigned long);
  +   int (*swap_hint)(struct block_device *, unsigned int, void *);
 
 And this would be a suitable place to document -swap_hint().

If we consider to be able to add more hints in future so it could
be verbose, IMO, it would be better to desribe it in enum swap_hint. :)

 
 - Hint from who to who?  Is it the caller providing the callee a hint
   or is the caller asking the callee for a hint?
 
 - What is the meaning of the return value?
 
 - What are the meaning of the arguments?

Okay.

 
 Please don't omit the argument names like this.  They are useful!  How
 is a reader to know what that unsigned int and void * actually
 *do*?

Yes.

 
 The second arg-which-doesn't-have-a-name should have had type
 swap_blk_hint, yes?

Yes.

 
 swap_blk_hint should be called swap_block_hint.  I assume that's what
 blk means.  Why does the name have block in there anyway?  It has
 something to do with disk blocks?  How is anyone supposed to work that
 out?

Yeb, I think we don't need block in name. I will remove it.

 
 -swap_hint was converted to return an `int', but all the callers
 simply ignore the return value.

You're right. All caller doesn't use it in this patch but this patch
makes it generalize and in later patch, SWAP_FULL will use it so
I want to make return as int. One more thought, SWAP_FULL could use
void * as output param 

[PATCH v1 1/5] zram: generalize swap_slot_free_notify

2014-09-21 Thread Minchan Kim
Currently, swap_slot_free_notify is used for zram to free
duplicated copy page for memory efficiency when it knows
there is no reference to the swap slot.

This patch generalizes it to be able to use for other
swap hint to communicate with VM.

Signed-off-by: Minchan Kim 
---
 Documentation/filesystems/Locking |  4 ++--
 drivers/block/zram/zram_drv.c | 18 --
 include/linux/blkdev.h|  7 +--
 mm/page_io.c  |  6 +++---
 mm/swapfile.c |  6 +++---
 5 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/Documentation/filesystems/Locking 
b/Documentation/filesystems/Locking
index 94d93b1f8b53..c262bfbeafa9 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -405,7 +405,7 @@ prototypes:
void (*unlock_native_capacity) (struct gendisk *);
int (*revalidate_disk) (struct gendisk *);
int (*getgeo)(struct block_device *, struct hd_geometry *);
-   void (*swap_slot_free_notify) (struct block_device *, unsigned long);
+   int (*swap_hint) (struct block_device *, unsigned int, void *);
 
 locking rules:
bd_mutex
@@ -418,7 +418,7 @@ media_changed:  no
 unlock_native_capacity:no
 revalidate_disk:   no
 getgeo:no
-swap_slot_free_notify: no  (see below)
+swap_hint: no  (see below)
 
 media_changed, unlock_native_capacity and revalidate_disk are called only from
 check_disk_change().
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index d78b245bae06..22a37764c409 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -926,7 +926,8 @@ error:
bio_io_error(bio);
 }
 
-static void zram_slot_free_notify(struct block_device *bdev,
+/* this callback is with swap_lock and sometimes page table lock held */
+static int zram_slot_free_notify(struct block_device *bdev,
unsigned long index)
 {
struct zram *zram;
@@ -939,10 +940,23 @@ static void zram_slot_free_notify(struct block_device 
*bdev,
zram_free_page(zram, index);
bit_spin_unlock(ZRAM_ACCESS, >table[index].value);
atomic64_inc(>stats.notify_free);
+
+   return 0;
+}
+
+static int zram_swap_hint(struct block_device *bdev,
+   unsigned int hint, void *arg)
+{
+   int ret = -EINVAL;
+
+   if (hint == SWAP_FREE)
+   ret = zram_slot_free_notify(bdev, (unsigned long)arg);
+
+   return ret;
 }
 
 static const struct block_device_operations zram_devops = {
-   .swap_slot_free_notify = zram_slot_free_notify,
+   .swap_hint = zram_swap_hint,
.owner = THIS_MODULE
 };
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index e267bf0db559..c7220409456c 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1609,6 +1609,10 @@ static inline bool blk_integrity_is_initialized(struct 
gendisk *g)
 
 #endif /* CONFIG_BLK_DEV_INTEGRITY */
 
+enum swap_blk_hint {
+   SWAP_FREE,
+};
+
 struct block_device_operations {
int (*open) (struct block_device *, fmode_t);
void (*release) (struct gendisk *, fmode_t);
@@ -1624,8 +1628,7 @@ struct block_device_operations {
void (*unlock_native_capacity) (struct gendisk *);
int (*revalidate_disk) (struct gendisk *);
int (*getgeo)(struct block_device *, struct hd_geometry *);
-   /* this callback is with swap_lock and sometimes page table lock held */
-   void (*swap_slot_free_notify) (struct block_device *, unsigned long);
+   int (*swap_hint)(struct block_device *, unsigned int, void *);
struct module *owner;
 };
 
diff --git a/mm/page_io.c b/mm/page_io.c
index 955db8b0d497..c6cc19655e97 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -114,7 +114,7 @@ void end_swap_bio_read(struct bio *bio, int err)
 * we again wish to reclaim it.
 */
struct gendisk *disk = sis->bdev->bd_disk;
-   if (disk->fops->swap_slot_free_notify) {
+   if (disk->fops->swap_hint) {
swp_entry_t entry;
unsigned long offset;
 
@@ -122,8 +122,8 @@ void end_swap_bio_read(struct bio *bio, int err)
offset = swp_offset(entry);
 
SetPageDirty(page);
-   disk->fops->swap_slot_free_notify(sis->bdev,
-   offset);
+   disk->fops->swap_hint(sis->bdev,
+   SWAP_FREE, (void *)offset);
}
}
}
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 8798b2e0ac59..c07f7f4912e9 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -816,9 +816,9 @@ static unsigned char 

[PATCH v1 1/5] zram: generalize swap_slot_free_notify

2014-09-21 Thread Minchan Kim
Currently, swap_slot_free_notify is used for zram to free
duplicated copy page for memory efficiency when it knows
there is no reference to the swap slot.

This patch generalizes it to be able to use for other
swap hint to communicate with VM.

Signed-off-by: Minchan Kim minc...@kernel.org
---
 Documentation/filesystems/Locking |  4 ++--
 drivers/block/zram/zram_drv.c | 18 --
 include/linux/blkdev.h|  7 +--
 mm/page_io.c  |  6 +++---
 mm/swapfile.c |  6 +++---
 5 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/Documentation/filesystems/Locking 
b/Documentation/filesystems/Locking
index 94d93b1f8b53..c262bfbeafa9 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -405,7 +405,7 @@ prototypes:
void (*unlock_native_capacity) (struct gendisk *);
int (*revalidate_disk) (struct gendisk *);
int (*getgeo)(struct block_device *, struct hd_geometry *);
-   void (*swap_slot_free_notify) (struct block_device *, unsigned long);
+   int (*swap_hint) (struct block_device *, unsigned int, void *);
 
 locking rules:
bd_mutex
@@ -418,7 +418,7 @@ media_changed:  no
 unlock_native_capacity:no
 revalidate_disk:   no
 getgeo:no
-swap_slot_free_notify: no  (see below)
+swap_hint: no  (see below)
 
 media_changed, unlock_native_capacity and revalidate_disk are called only from
 check_disk_change().
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index d78b245bae06..22a37764c409 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -926,7 +926,8 @@ error:
bio_io_error(bio);
 }
 
-static void zram_slot_free_notify(struct block_device *bdev,
+/* this callback is with swap_lock and sometimes page table lock held */
+static int zram_slot_free_notify(struct block_device *bdev,
unsigned long index)
 {
struct zram *zram;
@@ -939,10 +940,23 @@ static void zram_slot_free_notify(struct block_device 
*bdev,
zram_free_page(zram, index);
bit_spin_unlock(ZRAM_ACCESS, meta-table[index].value);
atomic64_inc(zram-stats.notify_free);
+
+   return 0;
+}
+
+static int zram_swap_hint(struct block_device *bdev,
+   unsigned int hint, void *arg)
+{
+   int ret = -EINVAL;
+
+   if (hint == SWAP_FREE)
+   ret = zram_slot_free_notify(bdev, (unsigned long)arg);
+
+   return ret;
 }
 
 static const struct block_device_operations zram_devops = {
-   .swap_slot_free_notify = zram_slot_free_notify,
+   .swap_hint = zram_swap_hint,
.owner = THIS_MODULE
 };
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index e267bf0db559..c7220409456c 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1609,6 +1609,10 @@ static inline bool blk_integrity_is_initialized(struct 
gendisk *g)
 
 #endif /* CONFIG_BLK_DEV_INTEGRITY */
 
+enum swap_blk_hint {
+   SWAP_FREE,
+};
+
 struct block_device_operations {
int (*open) (struct block_device *, fmode_t);
void (*release) (struct gendisk *, fmode_t);
@@ -1624,8 +1628,7 @@ struct block_device_operations {
void (*unlock_native_capacity) (struct gendisk *);
int (*revalidate_disk) (struct gendisk *);
int (*getgeo)(struct block_device *, struct hd_geometry *);
-   /* this callback is with swap_lock and sometimes page table lock held */
-   void (*swap_slot_free_notify) (struct block_device *, unsigned long);
+   int (*swap_hint)(struct block_device *, unsigned int, void *);
struct module *owner;
 };
 
diff --git a/mm/page_io.c b/mm/page_io.c
index 955db8b0d497..c6cc19655e97 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -114,7 +114,7 @@ void end_swap_bio_read(struct bio *bio, int err)
 * we again wish to reclaim it.
 */
struct gendisk *disk = sis-bdev-bd_disk;
-   if (disk-fops-swap_slot_free_notify) {
+   if (disk-fops-swap_hint) {
swp_entry_t entry;
unsigned long offset;
 
@@ -122,8 +122,8 @@ void end_swap_bio_read(struct bio *bio, int err)
offset = swp_offset(entry);
 
SetPageDirty(page);
-   disk-fops-swap_slot_free_notify(sis-bdev,
-   offset);
+   disk-fops-swap_hint(sis-bdev,
+   SWAP_FREE, (void *)offset);
}
}
}
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 8798b2e0ac59..c07f7f4912e9 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -816,9 +816,9 @@ static unsigned