[PATCH] zram: add num_discards for discarded pages stat

[PATCH] zram: add num_discards for discarded pages stat

[Chao Yu]

Now we have supported handling discard request which is sended by filesystem,
but no interface could be used to show information of discard.
This patch adds num_discards to stat discarded pages, then export it to sysfs
for displaying.

Signed-off-by: Chao Yu <chao2.yu@samsung.com>
---
 Documentation/ABI/testing/sysfs-block-zram | 10 ++++++++++
 drivers/block/zram/zram_drv.c              |  3 +++
 drivers/block/zram/zram_drv.h              |  1 +
 3 files changed, 14 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-block-zram b/Documentation/ABI/testing/sysfs-block-zram
index 70ec992..fa8936e 100644
--- a/Documentation/ABI/testing/sysfs-block-zram
+++ b/Documentation/ABI/testing/sysfs-block-zram
@@ -57,6 +57,16 @@ Description:
 		The failed_writes file is read-only and specifies the number of
 		failed writes happened on this device.
 
+
+What:		/sys/block/zram<id>/num_discards
+Date:		August 2014
+Contact:	Chao Yu <chao2.yu@samsung.com>
+Description:
+		The num_discards file is read-only and specifies the number of
+		physical blocks which are discarded by this device. These blocks
+		are included in discard request which is sended by filesystem as
+		the blocks are no longer used.
+
 What:		/sys/block/zram<id>/max_comp_streams
 Date:		February 2014
 Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index d00831c..904e7a5 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -606,6 +606,7 @@ static void zram_bio_discard(struct zram *zram, u32 index,
 		bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
 		zram_free_page(zram, index);
 		bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
+		atomic64_inc(&zram->stats.num_discards);
 		index++;
 		n -= PAGE_SIZE;
 	}
@@ -866,6 +867,7 @@ ZRAM_ATTR_RO(num_reads);
 ZRAM_ATTR_RO(num_writes);
 ZRAM_ATTR_RO(failed_reads);
 ZRAM_ATTR_RO(failed_writes);
+ZRAM_ATTR_RO(num_discards);
 ZRAM_ATTR_RO(invalid_io);
 ZRAM_ATTR_RO(notify_free);
 ZRAM_ATTR_RO(zero_pages);
@@ -879,6 +881,7 @@ static struct attribute *zram_disk_attrs[] = {
 	&dev_attr_num_writes.attr,
 	&dev_attr_failed_reads.attr,
 	&dev_attr_failed_writes.attr,
+	&dev_attr_num_discards.attr,
 	&dev_attr_invalid_io.attr,
 	&dev_attr_notify_free.attr,
 	&dev_attr_zero_pages.attr,
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index e0f725c..2994aaf 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -86,6 +86,7 @@ struct zram_stats {
 	atomic64_t num_writes;	/* --do-- */
 	atomic64_t failed_reads;	/* can happen when memory is too low */
 	atomic64_t failed_writes;	/* can happen when memory is too low */
+	atomic64_t num_discards;	/* no. of discarded pages */
 	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
 	atomic64_t notify_free;	/* no. of swap slot free notifications */
 	atomic64_t zero_pages;		/* no. of zero filled pages */
-- 
2.0.1.474.g72c7794

[PATCH] zram: add num_discards for discarded pages stat

[Chao Yu]

Now we have supported handling discard request which is sended by filesystem,
but no interface could be used to show information of discard.
This patch adds num_discards to stat discarded pages, then export it to sysfs
for displaying.

Signed-off-by: Chao Yu <chao2.yu@samsung.com>
---
 Documentation/ABI/testing/sysfs-block-zram | 10 ++++++++++
 drivers/block/zram/zram_drv.c              |  3 +++
 drivers/block/zram/zram_drv.h              |  1 +
 3 files changed, 14 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-block-zram b/Documentation/ABI/testing/sysfs-block-zram
index 70ec992..fa8936e 100644
--- a/Documentation/ABI/testing/sysfs-block-zram
+++ b/Documentation/ABI/testing/sysfs-block-zram
@@ -57,6 +57,16 @@ Description:
 		The failed_writes file is read-only and specifies the number of
 		failed writes happened on this device.
 
+
+What:		/sys/block/zram<id>/num_discards
+Date:		August 2014
+Contact:	Chao Yu <chao2.yu@samsung.com>
+Description:
+		The num_discards file is read-only and specifies the number of
+		physical blocks which are discarded by this device. These blocks
+		are included in discard request which is sended by filesystem as
+		the blocks are no longer used.
+
 What:		/sys/block/zram<id>/max_comp_streams
 Date:		February 2014
 Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index d00831c..904e7a5 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -606,6 +606,7 @@ static void zram_bio_discard(struct zram *zram, u32 index,
 		bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
 		zram_free_page(zram, index);
 		bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
+		atomic64_inc(&zram->stats.num_discards);
 		index++;
 		n -= PAGE_SIZE;
 	}
@@ -866,6 +867,7 @@ ZRAM_ATTR_RO(num_reads);
 ZRAM_ATTR_RO(num_writes);
 ZRAM_ATTR_RO(failed_reads);
 ZRAM_ATTR_RO(failed_writes);
+ZRAM_ATTR_RO(num_discards);
 ZRAM_ATTR_RO(invalid_io);
 ZRAM_ATTR_RO(notify_free);
 ZRAM_ATTR_RO(zero_pages);
@@ -879,6 +881,7 @@ static struct attribute *zram_disk_attrs[] = {
 	&dev_attr_num_writes.attr,
 	&dev_attr_failed_reads.attr,
 	&dev_attr_failed_writes.attr,
+	&dev_attr_num_discards.attr,
 	&dev_attr_invalid_io.attr,
 	&dev_attr_notify_free.attr,
 	&dev_attr_zero_pages.attr,
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index e0f725c..2994aaf 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -86,6 +86,7 @@ struct zram_stats {
 	atomic64_t num_writes;	/* --do-- */
 	atomic64_t failed_reads;	/* can happen when memory is too low */
 	atomic64_t failed_writes;	/* can happen when memory is too low */
+	atomic64_t num_discards;	/* no. of discarded pages */
 	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
 	atomic64_t notify_free;	/* no. of swap slot free notifications */
 	atomic64_t zero_pages;		/* no. of zero filled pages */
-- 
2.0.1.474.g72c7794


--
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] zram: add num_discards for discarded pages stat

[Sergey Senozhatsky]

On (08/15/14 11:27), Chao Yu wrote:
> Now we have supported handling discard request which is sended by filesystem,
> but no interface could be used to show information of discard.
> This patch adds num_discards to stat discarded pages, then export it to sysfs
> for displaying.
> 

a side question: we account discarded pages via slot free notify in
notify_free and via req_discard in num_discards. how about accounting
both of them in num_discards? because, after all, they account a number
of discarded pages (zram_free_page()). or there any particular reason we
want to distinguish.

	-ss

> Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> ---
>  Documentation/ABI/testing/sysfs-block-zram | 10 ++++++++++
>  drivers/block/zram/zram_drv.c              |  3 +++
>  drivers/block/zram/zram_drv.h              |  1 +
>  3 files changed, 14 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-block-zram b/Documentation/ABI/testing/sysfs-block-zram
> index 70ec992..fa8936e 100644
> --- a/Documentation/ABI/testing/sysfs-block-zram
> +++ b/Documentation/ABI/testing/sysfs-block-zram
> @@ -57,6 +57,16 @@ Description:
>  		The failed_writes file is read-only and specifies the number of
>  		failed writes happened on this device.
>  
> +
> +What:		/sys/block/zram<id>/num_discards
> +Date:		August 2014
> +Contact:	Chao Yu <chao2.yu@samsung.com>
> +Description:
> +		The num_discards file is read-only and specifies the number of
> +		physical blocks which are discarded by this device. These blocks
> +		are included in discard request which is sended by filesystem as
> +		the blocks are no longer used.
> +
>  What:		/sys/block/zram<id>/max_comp_streams
>  Date:		February 2014
>  Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index d00831c..904e7a5 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -606,6 +606,7 @@ static void zram_bio_discard(struct zram *zram, u32 index,
>  		bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
>  		zram_free_page(zram, index);
>  		bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> +		atomic64_inc(&zram->stats.num_discards);
>  		index++;
>  		n -= PAGE_SIZE;
>  	}
> @@ -866,6 +867,7 @@ ZRAM_ATTR_RO(num_reads);
>  ZRAM_ATTR_RO(num_writes);
>  ZRAM_ATTR_RO(failed_reads);
>  ZRAM_ATTR_RO(failed_writes);
> +ZRAM_ATTR_RO(num_discards);
>  ZRAM_ATTR_RO(invalid_io);
>  ZRAM_ATTR_RO(notify_free);
>  ZRAM_ATTR_RO(zero_pages);
> @@ -879,6 +881,7 @@ static struct attribute *zram_disk_attrs[] = {
>  	&dev_attr_num_writes.attr,
>  	&dev_attr_failed_reads.attr,
>  	&dev_attr_failed_writes.attr,
> +	&dev_attr_num_discards.attr,
>  	&dev_attr_invalid_io.attr,
>  	&dev_attr_notify_free.attr,
>  	&dev_attr_zero_pages.attr,
> diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> index e0f725c..2994aaf 100644
> --- a/drivers/block/zram/zram_drv.h
> +++ b/drivers/block/zram/zram_drv.h
> @@ -86,6 +86,7 @@ struct zram_stats {
>  	atomic64_t num_writes;	/* --do-- */
>  	atomic64_t failed_reads;	/* can happen when memory is too low */
>  	atomic64_t failed_writes;	/* can happen when memory is too low */
> +	atomic64_t num_discards;	/* no. of discarded pages */
>  	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
>  	atomic64_t notify_free;	/* no. of swap slot free notifications */
>  	atomic64_t zero_pages;		/* no. of zero filled pages */
> -- 
> 2.0.1.474.g72c7794
> 
> 

Re: [PATCH] zram: add num_discards for discarded pages stat

[Sergey Senozhatsky]

On (08/15/14 11:27), Chao Yu wrote:
> Now we have supported handling discard request which is sended by filesystem,
> but no interface could be used to show information of discard.
> This patch adds num_discards to stat discarded pages, then export it to sysfs
> for displaying.
> 

a side question: we account discarded pages via slot free notify in
notify_free and via req_discard in num_discards. how about accounting
both of them in num_discards? because, after all, they account a number
of discarded pages (zram_free_page()). or there any particular reason we
want to distinguish.

	-ss

> Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> ---
>  Documentation/ABI/testing/sysfs-block-zram | 10 ++++++++++
>  drivers/block/zram/zram_drv.c              |  3 +++
>  drivers/block/zram/zram_drv.h              |  1 +
>  3 files changed, 14 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-block-zram b/Documentation/ABI/testing/sysfs-block-zram
> index 70ec992..fa8936e 100644
> --- a/Documentation/ABI/testing/sysfs-block-zram
> +++ b/Documentation/ABI/testing/sysfs-block-zram
> @@ -57,6 +57,16 @@ Description:
>  		The failed_writes file is read-only and specifies the number of
>  		failed writes happened on this device.
>  
> +
> +What:		/sys/block/zram<id>/num_discards
> +Date:		August 2014
> +Contact:	Chao Yu <chao2.yu@samsung.com>
> +Description:
> +		The num_discards file is read-only and specifies the number of
> +		physical blocks which are discarded by this device. These blocks
> +		are included in discard request which is sended by filesystem as
> +		the blocks are no longer used.
> +
>  What:		/sys/block/zram<id>/max_comp_streams
>  Date:		February 2014
>  Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index d00831c..904e7a5 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -606,6 +606,7 @@ static void zram_bio_discard(struct zram *zram, u32 index,
>  		bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
>  		zram_free_page(zram, index);
>  		bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> +		atomic64_inc(&zram->stats.num_discards);
>  		index++;
>  		n -= PAGE_SIZE;
>  	}
> @@ -866,6 +867,7 @@ ZRAM_ATTR_RO(num_reads);
>  ZRAM_ATTR_RO(num_writes);
>  ZRAM_ATTR_RO(failed_reads);
>  ZRAM_ATTR_RO(failed_writes);
> +ZRAM_ATTR_RO(num_discards);
>  ZRAM_ATTR_RO(invalid_io);
>  ZRAM_ATTR_RO(notify_free);
>  ZRAM_ATTR_RO(zero_pages);
> @@ -879,6 +881,7 @@ static struct attribute *zram_disk_attrs[] = {
>  	&dev_attr_num_writes.attr,
>  	&dev_attr_failed_reads.attr,
>  	&dev_attr_failed_writes.attr,
> +	&dev_attr_num_discards.attr,
>  	&dev_attr_invalid_io.attr,
>  	&dev_attr_notify_free.attr,
>  	&dev_attr_zero_pages.attr,
> diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> index e0f725c..2994aaf 100644
> --- a/drivers/block/zram/zram_drv.h
> +++ b/drivers/block/zram/zram_drv.h
> @@ -86,6 +86,7 @@ struct zram_stats {
>  	atomic64_t num_writes;	/* --do-- */
>  	atomic64_t failed_reads;	/* can happen when memory is too low */
>  	atomic64_t failed_writes;	/* can happen when memory is too low */
> +	atomic64_t num_discards;	/* no. of discarded pages */
>  	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
>  	atomic64_t notify_free;	/* no. of swap slot free notifications */
>  	atomic64_t zero_pages;		/* no. of zero filled pages */
> -- 
> 2.0.1.474.g72c7794
> 
> 

--
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] zram: add num_discards for discarded pages stat

[Chao Yu]

> -----Original Message-----
> From: owner-linux-mm@kvack.org [mailto:owner-linux-mm@kvack.org] On Behalf Of Sergey
> Senozhatsky
> Sent: Friday, August 15, 2014 2:12 PM
> To: Chao Yu
> Cc: minchan@kernel.org; linux-kernel@vger.kernel.org; linux-mm@kvack.org; ngupta@vflare.org;
> 'Jerome Marchand'; 'Sergey Senozhatsky'; 'Andrew Morton'
> Subject: Re: [PATCH] zram: add num_discards for discarded pages stat
> 
> On (08/15/14 11:27), Chao Yu wrote:
> > Now we have supported handling discard request which is sended by filesystem,
> > but no interface could be used to show information of discard.
> > This patch adds num_discards to stat discarded pages, then export it to sysfs
> > for displaying.
> >
> 
> a side question: we account discarded pages via slot free notify in
> notify_free and via req_discard in num_discards. how about accounting
> both of them in num_discards? because, after all, they account a number
> of discarded pages (zram_free_page()). or there any particular reason we
> want to distinguish.

Yeah, I agree with you as I have no such reason unless there are our users'
explicitly requirement for showing notify_free/num_discards separately later.

How do you think of sending another patch to merge these two counts?

One more thing is that I am missing to update document of zram, sorry about
that, let me update it in v2.

Thanks,
Yu

> 
> 	-ss
> 
> > Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> > ---
> >  Documentation/ABI/testing/sysfs-block-zram | 10 ++++++++++
> >  drivers/block/zram/zram_drv.c              |  3 +++
> >  drivers/block/zram/zram_drv.h              |  1 +
> >  3 files changed, 14 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-block-zram
> b/Documentation/ABI/testing/sysfs-block-zram
> > index 70ec992..fa8936e 100644
> > --- a/Documentation/ABI/testing/sysfs-block-zram
> > +++ b/Documentation/ABI/testing/sysfs-block-zram
> > @@ -57,6 +57,16 @@ Description:
> >  		The failed_writes file is read-only and specifies the number of
> >  		failed writes happened on this device.
> >
> > +
> > +What:		/sys/block/zram<id>/num_discards
> > +Date:		August 2014
> > +Contact:	Chao Yu <chao2.yu@samsung.com>
> > +Description:
> > +		The num_discards file is read-only and specifies the number of
> > +		physical blocks which are discarded by this device. These blocks
> > +		are included in discard request which is sended by filesystem as
> > +		the blocks are no longer used.
> > +
> >  What:		/sys/block/zram<id>/max_comp_streams
> >  Date:		February 2014
> >  Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > index d00831c..904e7a5 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -606,6 +606,7 @@ static void zram_bio_discard(struct zram *zram, u32 index,
> >  		bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> >  		zram_free_page(zram, index);
> >  		bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> > +		atomic64_inc(&zram->stats.num_discards);
> >  		index++;
> >  		n -= PAGE_SIZE;
> >  	}
> > @@ -866,6 +867,7 @@ ZRAM_ATTR_RO(num_reads);
> >  ZRAM_ATTR_RO(num_writes);
> >  ZRAM_ATTR_RO(failed_reads);
> >  ZRAM_ATTR_RO(failed_writes);
> > +ZRAM_ATTR_RO(num_discards);
> >  ZRAM_ATTR_RO(invalid_io);
> >  ZRAM_ATTR_RO(notify_free);
> >  ZRAM_ATTR_RO(zero_pages);
> > @@ -879,6 +881,7 @@ static struct attribute *zram_disk_attrs[] = {
> >  	&dev_attr_num_writes.attr,
> >  	&dev_attr_failed_reads.attr,
> >  	&dev_attr_failed_writes.attr,
> > +	&dev_attr_num_discards.attr,
> >  	&dev_attr_invalid_io.attr,
> >  	&dev_attr_notify_free.attr,
> >  	&dev_attr_zero_pages.attr,
> > diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> > index e0f725c..2994aaf 100644
> > --- a/drivers/block/zram/zram_drv.h
> > +++ b/drivers/block/zram/zram_drv.h
> > @@ -86,6 +86,7 @@ struct zram_stats {
> >  	atomic64_t num_writes;	/* --do-- */
> >  	atomic64_t failed_reads;	/* can happen when memory is too low */
> >  	atomic64_t failed_writes;	/* can happen when memory is too low */
> > +	atomic64_t num_discards;	/* no. of discarded pages */
> >  	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
> >  	atomic64_t notify_free;	/* no. of swap slot free notifications */
> >  	atomic64_t zero_pages;		/* no. of zero filled pages */
> > --
> > 2.0.1.474.g72c7794
> >
> >
> 
> --
> 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] zram: add num_discards for discarded pages stat

[Chao Yu]

> -----Original Message-----
> From: owner-linux-mm@kvack.org [mailto:owner-linux-mm@kvack.org] On Behalf Of Sergey
> Senozhatsky
> Sent: Friday, August 15, 2014 2:12 PM
> To: Chao Yu
> Cc: minchan@kernel.org; linux-kernel@vger.kernel.org; linux-mm@kvack.org; ngupta@vflare.org;
> 'Jerome Marchand'; 'Sergey Senozhatsky'; 'Andrew Morton'
> Subject: Re: [PATCH] zram: add num_discards for discarded pages stat
> 
> On (08/15/14 11:27), Chao Yu wrote:
> > Now we have supported handling discard request which is sended by filesystem,
> > but no interface could be used to show information of discard.
> > This patch adds num_discards to stat discarded pages, then export it to sysfs
> > for displaying.
> >
> 
> a side question: we account discarded pages via slot free notify in
> notify_free and via req_discard in num_discards. how about accounting
> both of them in num_discards? because, after all, they account a number
> of discarded pages (zram_free_page()). or there any particular reason we
> want to distinguish.

Yeah, I agree with you as I have no such reason unless there are our users'
explicitly requirement for showing notify_free/num_discards separately later.

How do you think of sending another patch to merge these two counts?

One more thing is that I am missing to update document of zram, sorry about
that, let me update it in v2.

Thanks,
Yu

> 
> 	-ss
> 
> > Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> > ---
> >  Documentation/ABI/testing/sysfs-block-zram | 10 ++++++++++
> >  drivers/block/zram/zram_drv.c              |  3 +++
> >  drivers/block/zram/zram_drv.h              |  1 +
> >  3 files changed, 14 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-block-zram
> b/Documentation/ABI/testing/sysfs-block-zram
> > index 70ec992..fa8936e 100644
> > --- a/Documentation/ABI/testing/sysfs-block-zram
> > +++ b/Documentation/ABI/testing/sysfs-block-zram
> > @@ -57,6 +57,16 @@ Description:
> >  		The failed_writes file is read-only and specifies the number of
> >  		failed writes happened on this device.
> >
> > +
> > +What:		/sys/block/zram<id>/num_discards
> > +Date:		August 2014
> > +Contact:	Chao Yu <chao2.yu@samsung.com>
> > +Description:
> > +		The num_discards file is read-only and specifies the number of
> > +		physical blocks which are discarded by this device. These blocks
> > +		are included in discard request which is sended by filesystem as
> > +		the blocks are no longer used.
> > +
> >  What:		/sys/block/zram<id>/max_comp_streams
> >  Date:		February 2014
> >  Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > index d00831c..904e7a5 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -606,6 +606,7 @@ static void zram_bio_discard(struct zram *zram, u32 index,
> >  		bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> >  		zram_free_page(zram, index);
> >  		bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> > +		atomic64_inc(&zram->stats.num_discards);
> >  		index++;
> >  		n -= PAGE_SIZE;
> >  	}
> > @@ -866,6 +867,7 @@ ZRAM_ATTR_RO(num_reads);
> >  ZRAM_ATTR_RO(num_writes);
> >  ZRAM_ATTR_RO(failed_reads);
> >  ZRAM_ATTR_RO(failed_writes);
> > +ZRAM_ATTR_RO(num_discards);
> >  ZRAM_ATTR_RO(invalid_io);
> >  ZRAM_ATTR_RO(notify_free);
> >  ZRAM_ATTR_RO(zero_pages);
> > @@ -879,6 +881,7 @@ static struct attribute *zram_disk_attrs[] = {
> >  	&dev_attr_num_writes.attr,
> >  	&dev_attr_failed_reads.attr,
> >  	&dev_attr_failed_writes.attr,
> > +	&dev_attr_num_discards.attr,
> >  	&dev_attr_invalid_io.attr,
> >  	&dev_attr_notify_free.attr,
> >  	&dev_attr_zero_pages.attr,
> > diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> > index e0f725c..2994aaf 100644
> > --- a/drivers/block/zram/zram_drv.h
> > +++ b/drivers/block/zram/zram_drv.h
> > @@ -86,6 +86,7 @@ struct zram_stats {
> >  	atomic64_t num_writes;	/* --do-- */
> >  	atomic64_t failed_reads;	/* can happen when memory is too low */
> >  	atomic64_t failed_writes;	/* can happen when memory is too low */
> > +	atomic64_t num_discards;	/* no. of discarded pages */
> >  	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
> >  	atomic64_t notify_free;	/* no. of swap slot free notifications */
> >  	atomic64_t zero_pages;		/* no. of zero filled pages */
> > --
> > 2.0.1.474.g72c7794
> >
> >
> 
> --
> 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>

--
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] zram: add num_discards for discarded pages stat

[Sergey Senozhatsky]

Hello,

On (08/19/14 13:45), Chao Yu wrote:
> > On (08/15/14 11:27), Chao Yu wrote:
> > > Now we have supported handling discard request which is sended by filesystem,
> > > but no interface could be used to show information of discard.
> > > This patch adds num_discards to stat discarded pages, then export it to sysfs
> > > for displaying.
> > >
> > 
> > a side question: we account discarded pages via slot free notify in
> > notify_free and via req_discard in num_discards. how about accounting
> > both of them in num_discards? because, after all, they account a number
> > of discarded pages (zram_free_page()). or there any particular reason we
> > want to distinguish.
> 
> Yeah, I agree with you as I have no such reason unless there are our users'
> explicitly requirement for showing notify_free/num_discards separately later.
> 
> How do you think of sending another patch to merge these two counts?
> 

Minchan, what do you think? let's account discarded pages in one place.

> One more thing is that I am missing to update document of zram, sorry about
> that, let me update it in v2.

thanks.

	-ss

> Thanks,
> Yu
> 
> > 
> > 	-ss
> > 
> > > Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> > > ---
> > >  Documentation/ABI/testing/sysfs-block-zram | 10 ++++++++++
> > >  drivers/block/zram/zram_drv.c              |  3 +++
> > >  drivers/block/zram/zram_drv.h              |  1 +
> > >  3 files changed, 14 insertions(+)
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-block-zram
> > b/Documentation/ABI/testing/sysfs-block-zram
> > > index 70ec992..fa8936e 100644
> > > --- a/Documentation/ABI/testing/sysfs-block-zram
> > > +++ b/Documentation/ABI/testing/sysfs-block-zram
> > > @@ -57,6 +57,16 @@ Description:
> > >  		The failed_writes file is read-only and specifies the number of
> > >  		failed writes happened on this device.
> > >
> > > +
> > > +What:		/sys/block/zram<id>/num_discards
> > > +Date:		August 2014
> > > +Contact:	Chao Yu <chao2.yu@samsung.com>
> > > +Description:
> > > +		The num_discards file is read-only and specifies the number of
> > > +		physical blocks which are discarded by this device. These blocks
> > > +		are included in discard request which is sended by filesystem as
> > > +		the blocks are no longer used.
> > > +
> > >  What:		/sys/block/zram<id>/max_comp_streams
> > >  Date:		February 2014
> > >  Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > > index d00831c..904e7a5 100644
> > > --- a/drivers/block/zram/zram_drv.c
> > > +++ b/drivers/block/zram/zram_drv.c
> > > @@ -606,6 +606,7 @@ static void zram_bio_discard(struct zram *zram, u32 index,
> > >  		bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> > >  		zram_free_page(zram, index);
> > >  		bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> > > +		atomic64_inc(&zram->stats.num_discards);
> > >  		index++;
> > >  		n -= PAGE_SIZE;
> > >  	}
> > > @@ -866,6 +867,7 @@ ZRAM_ATTR_RO(num_reads);
> > >  ZRAM_ATTR_RO(num_writes);
> > >  ZRAM_ATTR_RO(failed_reads);
> > >  ZRAM_ATTR_RO(failed_writes);
> > > +ZRAM_ATTR_RO(num_discards);
> > >  ZRAM_ATTR_RO(invalid_io);
> > >  ZRAM_ATTR_RO(notify_free);
> > >  ZRAM_ATTR_RO(zero_pages);
> > > @@ -879,6 +881,7 @@ static struct attribute *zram_disk_attrs[] = {
> > >  	&dev_attr_num_writes.attr,
> > >  	&dev_attr_failed_reads.attr,
> > >  	&dev_attr_failed_writes.attr,
> > > +	&dev_attr_num_discards.attr,
> > >  	&dev_attr_invalid_io.attr,
> > >  	&dev_attr_notify_free.attr,
> > >  	&dev_attr_zero_pages.attr,
> > > diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> > > index e0f725c..2994aaf 100644
> > > --- a/drivers/block/zram/zram_drv.h
> > > +++ b/drivers/block/zram/zram_drv.h
> > > @@ -86,6 +86,7 @@ struct zram_stats {
> > >  	atomic64_t num_writes;	/* --do-- */
> > >  	atomic64_t failed_reads;	/* can happen when memory is too low */
> > >  	atomic64_t failed_writes;	/* can happen when memory is too low */
> > > +	atomic64_t num_discards;	/* no. of discarded pages */
> > >  	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
> > >  	atomic64_t notify_free;	/* no. of swap slot free notifications */
> > >  	atomic64_t zero_pages;		/* no. of zero filled pages */
> > > --
> > > 2.0.1.474.g72c7794
> > >
> > >
> > 
> > --
> > 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] zram: add num_discards for discarded pages stat

[Sergey Senozhatsky]

Hello,

On (08/19/14 13:45), Chao Yu wrote:
> > On (08/15/14 11:27), Chao Yu wrote:
> > > Now we have supported handling discard request which is sended by filesystem,
> > > but no interface could be used to show information of discard.
> > > This patch adds num_discards to stat discarded pages, then export it to sysfs
> > > for displaying.
> > >
> > 
> > a side question: we account discarded pages via slot free notify in
> > notify_free and via req_discard in num_discards. how about accounting
> > both of them in num_discards? because, after all, they account a number
> > of discarded pages (zram_free_page()). or there any particular reason we
> > want to distinguish.
> 
> Yeah, I agree with you as I have no such reason unless there are our users'
> explicitly requirement for showing notify_free/num_discards separately later.
> 
> How do you think of sending another patch to merge these two counts?
> 

Minchan, what do you think? let's account discarded pages in one place.

> One more thing is that I am missing to update document of zram, sorry about
> that, let me update it in v2.

thanks.

	-ss

> Thanks,
> Yu
> 
> > 
> > 	-ss
> > 
> > > Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> > > ---
> > >  Documentation/ABI/testing/sysfs-block-zram | 10 ++++++++++
> > >  drivers/block/zram/zram_drv.c              |  3 +++
> > >  drivers/block/zram/zram_drv.h              |  1 +
> > >  3 files changed, 14 insertions(+)
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-block-zram
> > b/Documentation/ABI/testing/sysfs-block-zram
> > > index 70ec992..fa8936e 100644
> > > --- a/Documentation/ABI/testing/sysfs-block-zram
> > > +++ b/Documentation/ABI/testing/sysfs-block-zram
> > > @@ -57,6 +57,16 @@ Description:
> > >  		The failed_writes file is read-only and specifies the number of
> > >  		failed writes happened on this device.
> > >
> > > +
> > > +What:		/sys/block/zram<id>/num_discards
> > > +Date:		August 2014
> > > +Contact:	Chao Yu <chao2.yu@samsung.com>
> > > +Description:
> > > +		The num_discards file is read-only and specifies the number of
> > > +		physical blocks which are discarded by this device. These blocks
> > > +		are included in discard request which is sended by filesystem as
> > > +		the blocks are no longer used.
> > > +
> > >  What:		/sys/block/zram<id>/max_comp_streams
> > >  Date:		February 2014
> > >  Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > > index d00831c..904e7a5 100644
> > > --- a/drivers/block/zram/zram_drv.c
> > > +++ b/drivers/block/zram/zram_drv.c
> > > @@ -606,6 +606,7 @@ static void zram_bio_discard(struct zram *zram, u32 index,
> > >  		bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> > >  		zram_free_page(zram, index);
> > >  		bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> > > +		atomic64_inc(&zram->stats.num_discards);
> > >  		index++;
> > >  		n -= PAGE_SIZE;
> > >  	}
> > > @@ -866,6 +867,7 @@ ZRAM_ATTR_RO(num_reads);
> > >  ZRAM_ATTR_RO(num_writes);
> > >  ZRAM_ATTR_RO(failed_reads);
> > >  ZRAM_ATTR_RO(failed_writes);
> > > +ZRAM_ATTR_RO(num_discards);
> > >  ZRAM_ATTR_RO(invalid_io);
> > >  ZRAM_ATTR_RO(notify_free);
> > >  ZRAM_ATTR_RO(zero_pages);
> > > @@ -879,6 +881,7 @@ static struct attribute *zram_disk_attrs[] = {
> > >  	&dev_attr_num_writes.attr,
> > >  	&dev_attr_failed_reads.attr,
> > >  	&dev_attr_failed_writes.attr,
> > > +	&dev_attr_num_discards.attr,
> > >  	&dev_attr_invalid_io.attr,
> > >  	&dev_attr_notify_free.attr,
> > >  	&dev_attr_zero_pages.attr,
> > > diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> > > index e0f725c..2994aaf 100644
> > > --- a/drivers/block/zram/zram_drv.h
> > > +++ b/drivers/block/zram/zram_drv.h
> > > @@ -86,6 +86,7 @@ struct zram_stats {
> > >  	atomic64_t num_writes;	/* --do-- */
> > >  	atomic64_t failed_reads;	/* can happen when memory is too low */
> > >  	atomic64_t failed_writes;	/* can happen when memory is too low */
> > > +	atomic64_t num_discards;	/* no. of discarded pages */
> > >  	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
> > >  	atomic64_t notify_free;	/* no. of swap slot free notifications */
> > >  	atomic64_t zero_pages;		/* no. of zero filled pages */
> > > --
> > > 2.0.1.474.g72c7794
> > >
> > >
> > 
> > --
> > 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>
> 

--
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] zram: add num_discards for discarded pages stat

[Minchan Kim]

Hi Sergey,

On Tue, Aug 19, 2014 at 08:25:00PM +0900, Sergey Senozhatsky wrote:
> Hello,
> 
> On (08/19/14 13:45), Chao Yu wrote:
> > > On (08/15/14 11:27), Chao Yu wrote:
> > > > Now we have supported handling discard request which is sended by filesystem,
> > > > but no interface could be used to show information of discard.
> > > > This patch adds num_discards to stat discarded pages, then export it to sysfs
> > > > for displaying.
> > > >
> > > 
> > > a side question: we account discarded pages via slot free notify in
> > > notify_free and via req_discard in num_discards. how about accounting
> > > both of them in num_discards? because, after all, they account a number
> > > of discarded pages (zram_free_page()). or there any particular reason we
> > > want to distinguish.
> > 
> > Yeah, I agree with you as I have no such reason unless there are our users'
> > explicitly requirement for showing notify_free/num_discards separately later.
> > 
> > How do you think of sending another patch to merge these two counts?
> > 
> 
> Minchan, what do you think? let's account discarded pages in one place.

First of all, I'd like to know why we need num_discards.
It should be in description and depends on it whether we should merge both
counts or separate.

Thanks.



> 
> > One more thing is that I am missing to update document of zram, sorry about
> > that, let me update it in v2.
> 
> thanks.
> 
> 	-ss
> 
> > Thanks,
> > Yu
> > 
> > > 
> > > 	-ss
> > > 
> > > > Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> > > > ---
> > > >  Documentation/ABI/testing/sysfs-block-zram | 10 ++++++++++
> > > >  drivers/block/zram/zram_drv.c              |  3 +++
> > > >  drivers/block/zram/zram_drv.h              |  1 +
> > > >  3 files changed, 14 insertions(+)
> > > >
> > > > diff --git a/Documentation/ABI/testing/sysfs-block-zram
> > > b/Documentation/ABI/testing/sysfs-block-zram
> > > > index 70ec992..fa8936e 100644
> > > > --- a/Documentation/ABI/testing/sysfs-block-zram
> > > > +++ b/Documentation/ABI/testing/sysfs-block-zram
> > > > @@ -57,6 +57,16 @@ Description:
> > > >  		The failed_writes file is read-only and specifies the number of
> > > >  		failed writes happened on this device.
> > > >
> > > > +
> > > > +What:		/sys/block/zram<id>/num_discards
> > > > +Date:		August 2014
> > > > +Contact:	Chao Yu <chao2.yu@samsung.com>
> > > > +Description:
> > > > +		The num_discards file is read-only and specifies the number of
> > > > +		physical blocks which are discarded by this device. These blocks
> > > > +		are included in discard request which is sended by filesystem as
> > > > +		the blocks are no longer used.
> > > > +
> > > >  What:		/sys/block/zram<id>/max_comp_streams
> > > >  Date:		February 2014
> > > >  Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > > > index d00831c..904e7a5 100644
> > > > --- a/drivers/block/zram/zram_drv.c
> > > > +++ b/drivers/block/zram/zram_drv.c
> > > > @@ -606,6 +606,7 @@ static void zram_bio_discard(struct zram *zram, u32 index,
> > > >  		bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> > > >  		zram_free_page(zram, index);
> > > >  		bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> > > > +		atomic64_inc(&zram->stats.num_discards);
> > > >  		index++;
> > > >  		n -= PAGE_SIZE;
> > > >  	}
> > > > @@ -866,6 +867,7 @@ ZRAM_ATTR_RO(num_reads);
> > > >  ZRAM_ATTR_RO(num_writes);
> > > >  ZRAM_ATTR_RO(failed_reads);
> > > >  ZRAM_ATTR_RO(failed_writes);
> > > > +ZRAM_ATTR_RO(num_discards);
> > > >  ZRAM_ATTR_RO(invalid_io);
> > > >  ZRAM_ATTR_RO(notify_free);
> > > >  ZRAM_ATTR_RO(zero_pages);
> > > > @@ -879,6 +881,7 @@ static struct attribute *zram_disk_attrs[] = {
> > > >  	&dev_attr_num_writes.attr,
> > > >  	&dev_attr_failed_reads.attr,
> > > >  	&dev_attr_failed_writes.attr,
> > > > +	&dev_attr_num_discards.attr,
> > > >  	&dev_attr_invalid_io.attr,
> > > >  	&dev_attr_notify_free.attr,
> > > >  	&dev_attr_zero_pages.attr,
> > > > diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> > > > index e0f725c..2994aaf 100644
> > > > --- a/drivers/block/zram/zram_drv.h
> > > > +++ b/drivers/block/zram/zram_drv.h
> > > > @@ -86,6 +86,7 @@ struct zram_stats {
> > > >  	atomic64_t num_writes;	/* --do-- */
> > > >  	atomic64_t failed_reads;	/* can happen when memory is too low */
> > > >  	atomic64_t failed_writes;	/* can happen when memory is too low */
> > > > +	atomic64_t num_discards;	/* no. of discarded pages */
> > > >  	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
> > > >  	atomic64_t notify_free;	/* no. of swap slot free notifications */
> > > >  	atomic64_t zero_pages;		/* no. of zero filled pages */
> > > > --
> > > > 2.0.1.474.g72c7794
> > > >
> > > >
> > > 
> > > --
> > > 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>
> > 
> 
> --
> 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>

-- 
Kind regards,
Minchan Kim

Re: [PATCH] zram: add num_discards for discarded pages stat

[Minchan Kim]

Hi Sergey,

On Tue, Aug 19, 2014 at 08:25:00PM +0900, Sergey Senozhatsky wrote:
> Hello,
> 
> On (08/19/14 13:45), Chao Yu wrote:
> > > On (08/15/14 11:27), Chao Yu wrote:
> > > > Now we have supported handling discard request which is sended by filesystem,
> > > > but no interface could be used to show information of discard.
> > > > This patch adds num_discards to stat discarded pages, then export it to sysfs
> > > > for displaying.
> > > >
> > > 
> > > a side question: we account discarded pages via slot free notify in
> > > notify_free and via req_discard in num_discards. how about accounting
> > > both of them in num_discards? because, after all, they account a number
> > > of discarded pages (zram_free_page()). or there any particular reason we
> > > want to distinguish.
> > 
> > Yeah, I agree with you as I have no such reason unless there are our users'
> > explicitly requirement for showing notify_free/num_discards separately later.
> > 
> > How do you think of sending another patch to merge these two counts?
> > 
> 
> Minchan, what do you think? let's account discarded pages in one place.

First of all, I'd like to know why we need num_discards.
It should be in description and depends on it whether we should merge both
counts or separate.

Thanks.



> 
> > One more thing is that I am missing to update document of zram, sorry about
> > that, let me update it in v2.
> 
> thanks.
> 
> 	-ss
> 
> > Thanks,
> > Yu
> > 
> > > 
> > > 	-ss
> > > 
> > > > Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> > > > ---
> > > >  Documentation/ABI/testing/sysfs-block-zram | 10 ++++++++++
> > > >  drivers/block/zram/zram_drv.c              |  3 +++
> > > >  drivers/block/zram/zram_drv.h              |  1 +
> > > >  3 files changed, 14 insertions(+)
> > > >
> > > > diff --git a/Documentation/ABI/testing/sysfs-block-zram
> > > b/Documentation/ABI/testing/sysfs-block-zram
> > > > index 70ec992..fa8936e 100644
> > > > --- a/Documentation/ABI/testing/sysfs-block-zram
> > > > +++ b/Documentation/ABI/testing/sysfs-block-zram
> > > > @@ -57,6 +57,16 @@ Description:
> > > >  		The failed_writes file is read-only and specifies the number of
> > > >  		failed writes happened on this device.
> > > >
> > > > +
> > > > +What:		/sys/block/zram<id>/num_discards
> > > > +Date:		August 2014
> > > > +Contact:	Chao Yu <chao2.yu@samsung.com>
> > > > +Description:
> > > > +		The num_discards file is read-only and specifies the number of
> > > > +		physical blocks which are discarded by this device. These blocks
> > > > +		are included in discard request which is sended by filesystem as
> > > > +		the blocks are no longer used.
> > > > +
> > > >  What:		/sys/block/zram<id>/max_comp_streams
> > > >  Date:		February 2014
> > > >  Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > > > index d00831c..904e7a5 100644
> > > > --- a/drivers/block/zram/zram_drv.c
> > > > +++ b/drivers/block/zram/zram_drv.c
> > > > @@ -606,6 +606,7 @@ static void zram_bio_discard(struct zram *zram, u32 index,
> > > >  		bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> > > >  		zram_free_page(zram, index);
> > > >  		bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> > > > +		atomic64_inc(&zram->stats.num_discards);
> > > >  		index++;
> > > >  		n -= PAGE_SIZE;
> > > >  	}
> > > > @@ -866,6 +867,7 @@ ZRAM_ATTR_RO(num_reads);
> > > >  ZRAM_ATTR_RO(num_writes);
> > > >  ZRAM_ATTR_RO(failed_reads);
> > > >  ZRAM_ATTR_RO(failed_writes);
> > > > +ZRAM_ATTR_RO(num_discards);
> > > >  ZRAM_ATTR_RO(invalid_io);
> > > >  ZRAM_ATTR_RO(notify_free);
> > > >  ZRAM_ATTR_RO(zero_pages);
> > > > @@ -879,6 +881,7 @@ static struct attribute *zram_disk_attrs[] = {
> > > >  	&dev_attr_num_writes.attr,
> > > >  	&dev_attr_failed_reads.attr,
> > > >  	&dev_attr_failed_writes.attr,
> > > > +	&dev_attr_num_discards.attr,
> > > >  	&dev_attr_invalid_io.attr,
> > > >  	&dev_attr_notify_free.attr,
> > > >  	&dev_attr_zero_pages.attr,
> > > > diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> > > > index e0f725c..2994aaf 100644
> > > > --- a/drivers/block/zram/zram_drv.h
> > > > +++ b/drivers/block/zram/zram_drv.h
> > > > @@ -86,6 +86,7 @@ struct zram_stats {
> > > >  	atomic64_t num_writes;	/* --do-- */
> > > >  	atomic64_t failed_reads;	/* can happen when memory is too low */
> > > >  	atomic64_t failed_writes;	/* can happen when memory is too low */
> > > > +	atomic64_t num_discards;	/* no. of discarded pages */
> > > >  	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
> > > >  	atomic64_t notify_free;	/* no. of swap slot free notifications */
> > > >  	atomic64_t zero_pages;		/* no. of zero filled pages */
> > > > --
> > > > 2.0.1.474.g72c7794
> > > >
> > > >
> > > 
> > > --
> > > 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>
> > 
> 
> --
> 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>

-- 
Kind regards,
Minchan Kim

--
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] zram: add num_discards for discarded pages stat

[Chao Yu]

Hi Minchan,

> -----Original Message-----
> From: Minchan Kim [mailto:minchan@kernel.org]
> Sent: Wednesday, August 20, 2014 10:09 AM
> To: Sergey Senozhatsky
> Cc: Chao Yu; linux-kernel@vger.kernel.org; linux-mm@kvack.org; ngupta@vflare.org; 'Jerome
> Marchand'; 'Andrew Morton'
> Subject: Re: [PATCH] zram: add num_discards for discarded pages stat
> 
> Hi Sergey,
> 
> On Tue, Aug 19, 2014 at 08:25:00PM +0900, Sergey Senozhatsky wrote:
> > Hello,
> >
> > On (08/19/14 13:45), Chao Yu wrote:
> > > > On (08/15/14 11:27), Chao Yu wrote:
> > > > > Now we have supported handling discard request which is sended by filesystem,
> > > > > but no interface could be used to show information of discard.
> > > > > This patch adds num_discards to stat discarded pages, then export it to sysfs
> > > > > for displaying.
> > > > >
> > > >
> > > > a side question: we account discarded pages via slot free notify in
> > > > notify_free and via req_discard in num_discards. how about accounting
> > > > both of them in num_discards? because, after all, they account a number
> > > > of discarded pages (zram_free_page()). or there any particular reason we
> > > > want to distinguish.
> > >
> > > Yeah, I agree with you as I have no such reason unless there are our users'
> > > explicitly requirement for showing notify_free/num_discards separately later.
> > >
> > > How do you think of sending another patch to merge these two counts?
> > >
> >
> > Minchan, what do you think? let's account discarded pages in one place.
> 
> First of all, I'd like to know why we need num_discards.
> It should be in description and depends on it whether we should merge both
> counts or separate.

Oh, it's my mistaken.

When commit 	9b9913d80b2896ecd9e0a1a8f167ccad66fac79c (Staging: zram: Update
zram documentation) and commit e98419c23b1a189c932775f7833e94cb5230a16b (Staging:
zram: Document sysfs entries) description related to 'discard' stat was designed
and added to zram.txt and sysfs-block-zram, but without implementation of function
for handling discard request, description in documents were removed in commit
8dd1d3247e6c00b50ef83934ea8b22a1590015de (zram: document failed_reads,
failed_writes stats)

For now, we have already supported discard handling, so it's better to resume
the stat of discard number, this discard stat supports user one more kind of runtime
information of zram as other stats supported.

How do you think?

> 
> Thanks.
> 
> 
> 
> >
> > > One more thing is that I am missing to update document of zram, sorry about
> > > that, let me update it in v2.
> >
> > thanks.
> >
> > 	-ss
> >
> > > Thanks,
> > > Yu
> > >
> > > >
> > > > 	-ss
> > > >
> > > > > Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> > > > > ---
> > > > >  Documentation/ABI/testing/sysfs-block-zram | 10 ++++++++++
> > > > >  drivers/block/zram/zram_drv.c              |  3 +++
> > > > >  drivers/block/zram/zram_drv.h              |  1 +
> > > > >  3 files changed, 14 insertions(+)
> > > > >
> > > > > diff --git a/Documentation/ABI/testing/sysfs-block-zram
> > > > b/Documentation/ABI/testing/sysfs-block-zram
> > > > > index 70ec992..fa8936e 100644
> > > > > --- a/Documentation/ABI/testing/sysfs-block-zram
> > > > > +++ b/Documentation/ABI/testing/sysfs-block-zram
> > > > > @@ -57,6 +57,16 @@ Description:
> > > > >  		The failed_writes file is read-only and specifies the number of
> > > > >  		failed writes happened on this device.
> > > > >
> > > > > +
> > > > > +What:		/sys/block/zram<id>/num_discards
> > > > > +Date:		August 2014
> > > > > +Contact:	Chao Yu <chao2.yu@samsung.com>
> > > > > +Description:
> > > > > +		The num_discards file is read-only and specifies the number of
> > > > > +		physical blocks which are discarded by this device. These blocks
> > > > > +		are included in discard request which is sended by filesystem as
> > > > > +		the blocks are no longer used.
> > > > > +
> > > > >  What:		/sys/block/zram<id>/max_comp_streams
> > > > >  Date:		February 2014
> > > > >  Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > > > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > > > > index d00831c..904e7a5 100644
> > > > > --- a/drivers/block/zram/zram_drv.c
> > > > > +++ b/drivers/block/zram/zram_drv.c
> > > > > @@ -606,6 +606,7 @@ static void zram_bio_discard(struct zram *zram, u32 index,
> > > > >  		bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> > > > >  		zram_free_page(zram, index);
> > > > >  		bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> > > > > +		atomic64_inc(&zram->stats.num_discards);
> > > > >  		index++;
> > > > >  		n -= PAGE_SIZE;
> > > > >  	}
> > > > > @@ -866,6 +867,7 @@ ZRAM_ATTR_RO(num_reads);
> > > > >  ZRAM_ATTR_RO(num_writes);
> > > > >  ZRAM_ATTR_RO(failed_reads);
> > > > >  ZRAM_ATTR_RO(failed_writes);
> > > > > +ZRAM_ATTR_RO(num_discards);
> > > > >  ZRAM_ATTR_RO(invalid_io);
> > > > >  ZRAM_ATTR_RO(notify_free);
> > > > >  ZRAM_ATTR_RO(zero_pages);
> > > > > @@ -879,6 +881,7 @@ static struct attribute *zram_disk_attrs[] = {
> > > > >  	&dev_attr_num_writes.attr,
> > > > >  	&dev_attr_failed_reads.attr,
> > > > >  	&dev_attr_failed_writes.attr,
> > > > > +	&dev_attr_num_discards.attr,
> > > > >  	&dev_attr_invalid_io.attr,
> > > > >  	&dev_attr_notify_free.attr,
> > > > >  	&dev_attr_zero_pages.attr,
> > > > > diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> > > > > index e0f725c..2994aaf 100644
> > > > > --- a/drivers/block/zram/zram_drv.h
> > > > > +++ b/drivers/block/zram/zram_drv.h
> > > > > @@ -86,6 +86,7 @@ struct zram_stats {
> > > > >  	atomic64_t num_writes;	/* --do-- */
> > > > >  	atomic64_t failed_reads;	/* can happen when memory is too low */
> > > > >  	atomic64_t failed_writes;	/* can happen when memory is too low */
> > > > > +	atomic64_t num_discards;	/* no. of discarded pages */
> > > > >  	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
> > > > >  	atomic64_t notify_free;	/* no. of swap slot free notifications */
> > > > >  	atomic64_t zero_pages;		/* no. of zero filled pages */
> > > > > --
> > > > > 2.0.1.474.g72c7794
> > > > >
> > > > >
> > > >
> > > > --
> > > > 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>
> > >
> >
> > --
> > 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>
> 
> --
> Kind regards,
> Minchan Kim

RE: [PATCH] zram: add num_discards for discarded pages stat

[Chao Yu]

Hi Minchan,

> -----Original Message-----
> From: Minchan Kim [mailto:minchan@kernel.org]
> Sent: Wednesday, August 20, 2014 10:09 AM
> To: Sergey Senozhatsky
> Cc: Chao Yu; linux-kernel@vger.kernel.org; linux-mm@kvack.org; ngupta@vflare.org; 'Jerome
> Marchand'; 'Andrew Morton'
> Subject: Re: [PATCH] zram: add num_discards for discarded pages stat
> 
> Hi Sergey,
> 
> On Tue, Aug 19, 2014 at 08:25:00PM +0900, Sergey Senozhatsky wrote:
> > Hello,
> >
> > On (08/19/14 13:45), Chao Yu wrote:
> > > > On (08/15/14 11:27), Chao Yu wrote:
> > > > > Now we have supported handling discard request which is sended by filesystem,
> > > > > but no interface could be used to show information of discard.
> > > > > This patch adds num_discards to stat discarded pages, then export it to sysfs
> > > > > for displaying.
> > > > >
> > > >
> > > > a side question: we account discarded pages via slot free notify in
> > > > notify_free and via req_discard in num_discards. how about accounting
> > > > both of them in num_discards? because, after all, they account a number
> > > > of discarded pages (zram_free_page()). or there any particular reason we
> > > > want to distinguish.
> > >
> > > Yeah, I agree with you as I have no such reason unless there are our users'
> > > explicitly requirement for showing notify_free/num_discards separately later.
> > >
> > > How do you think of sending another patch to merge these two counts?
> > >
> >
> > Minchan, what do you think? let's account discarded pages in one place.
> 
> First of all, I'd like to know why we need num_discards.
> It should be in description and depends on it whether we should merge both
> counts or separate.

Oh, it's my mistaken.

When commit 	9b9913d80b2896ecd9e0a1a8f167ccad66fac79c (Staging: zram: Update
zram documentation) and commit e98419c23b1a189c932775f7833e94cb5230a16b (Staging:
zram: Document sysfs entries) description related to 'discard' stat was designed
and added to zram.txt and sysfs-block-zram, but without implementation of function
for handling discard request, description in documents were removed in commit
8dd1d3247e6c00b50ef83934ea8b22a1590015de (zram: document failed_reads,
failed_writes stats)

For now, we have already supported discard handling, so it's better to resume
the stat of discard number, this discard stat supports user one more kind of runtime
information of zram as other stats supported.

How do you think?

> 
> Thanks.
> 
> 
> 
> >
> > > One more thing is that I am missing to update document of zram, sorry about
> > > that, let me update it in v2.
> >
> > thanks.
> >
> > 	-ss
> >
> > > Thanks,
> > > Yu
> > >
> > > >
> > > > 	-ss
> > > >
> > > > > Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> > > > > ---
> > > > >  Documentation/ABI/testing/sysfs-block-zram | 10 ++++++++++
> > > > >  drivers/block/zram/zram_drv.c              |  3 +++
> > > > >  drivers/block/zram/zram_drv.h              |  1 +
> > > > >  3 files changed, 14 insertions(+)
> > > > >
> > > > > diff --git a/Documentation/ABI/testing/sysfs-block-zram
> > > > b/Documentation/ABI/testing/sysfs-block-zram
> > > > > index 70ec992..fa8936e 100644
> > > > > --- a/Documentation/ABI/testing/sysfs-block-zram
> > > > > +++ b/Documentation/ABI/testing/sysfs-block-zram
> > > > > @@ -57,6 +57,16 @@ Description:
> > > > >  		The failed_writes file is read-only and specifies the number of
> > > > >  		failed writes happened on this device.
> > > > >
> > > > > +
> > > > > +What:		/sys/block/zram<id>/num_discards
> > > > > +Date:		August 2014
> > > > > +Contact:	Chao Yu <chao2.yu@samsung.com>
> > > > > +Description:
> > > > > +		The num_discards file is read-only and specifies the number of
> > > > > +		physical blocks which are discarded by this device. These blocks
> > > > > +		are included in discard request which is sended by filesystem as
> > > > > +		the blocks are no longer used.
> > > > > +
> > > > >  What:		/sys/block/zram<id>/max_comp_streams
> > > > >  Date:		February 2014
> > > > >  Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > > > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > > > > index d00831c..904e7a5 100644
> > > > > --- a/drivers/block/zram/zram_drv.c
> > > > > +++ b/drivers/block/zram/zram_drv.c
> > > > > @@ -606,6 +606,7 @@ static void zram_bio_discard(struct zram *zram, u32 index,
> > > > >  		bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> > > > >  		zram_free_page(zram, index);
> > > > >  		bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> > > > > +		atomic64_inc(&zram->stats.num_discards);
> > > > >  		index++;
> > > > >  		n -= PAGE_SIZE;
> > > > >  	}
> > > > > @@ -866,6 +867,7 @@ ZRAM_ATTR_RO(num_reads);
> > > > >  ZRAM_ATTR_RO(num_writes);
> > > > >  ZRAM_ATTR_RO(failed_reads);
> > > > >  ZRAM_ATTR_RO(failed_writes);
> > > > > +ZRAM_ATTR_RO(num_discards);
> > > > >  ZRAM_ATTR_RO(invalid_io);
> > > > >  ZRAM_ATTR_RO(notify_free);
> > > > >  ZRAM_ATTR_RO(zero_pages);
> > > > > @@ -879,6 +881,7 @@ static struct attribute *zram_disk_attrs[] = {
> > > > >  	&dev_attr_num_writes.attr,
> > > > >  	&dev_attr_failed_reads.attr,
> > > > >  	&dev_attr_failed_writes.attr,
> > > > > +	&dev_attr_num_discards.attr,
> > > > >  	&dev_attr_invalid_io.attr,
> > > > >  	&dev_attr_notify_free.attr,
> > > > >  	&dev_attr_zero_pages.attr,
> > > > > diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> > > > > index e0f725c..2994aaf 100644
> > > > > --- a/drivers/block/zram/zram_drv.h
> > > > > +++ b/drivers/block/zram/zram_drv.h
> > > > > @@ -86,6 +86,7 @@ struct zram_stats {
> > > > >  	atomic64_t num_writes;	/* --do-- */
> > > > >  	atomic64_t failed_reads;	/* can happen when memory is too low */
> > > > >  	atomic64_t failed_writes;	/* can happen when memory is too low */
> > > > > +	atomic64_t num_discards;	/* no. of discarded pages */
> > > > >  	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
> > > > >  	atomic64_t notify_free;	/* no. of swap slot free notifications */
> > > > >  	atomic64_t zero_pages;		/* no. of zero filled pages */
> > > > > --
> > > > > 2.0.1.474.g72c7794
> > > > >
> > > > >
> > > >
> > > > --
> > > > 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>
> > >
> >
> > --
> > 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>
> 
> --
> Kind regards,
> Minchan Kim

--
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] zram: add num_discards for discarded pages stat

[Minchan Kim]

Hi Chao,

On Wed, Aug 20, 2014 at 04:20:48PM +0800, Chao Yu wrote:
> Hi Minchan,
> 
> > -----Original Message-----
> > From: Minchan Kim [mailto:minchan@kernel.org]
> > Sent: Wednesday, August 20, 2014 10:09 AM
> > To: Sergey Senozhatsky
> > Cc: Chao Yu; linux-kernel@vger.kernel.org; linux-mm@kvack.org; ngupta@vflare.org; 'Jerome
> > Marchand'; 'Andrew Morton'
> > Subject: Re: [PATCH] zram: add num_discards for discarded pages stat
> > 
> > Hi Sergey,
> > 
> > On Tue, Aug 19, 2014 at 08:25:00PM +0900, Sergey Senozhatsky wrote:
> > > Hello,
> > >
> > > On (08/19/14 13:45), Chao Yu wrote:
> > > > > On (08/15/14 11:27), Chao Yu wrote:
> > > > > > Now we have supported handling discard request which is sended by filesystem,
> > > > > > but no interface could be used to show information of discard.
> > > > > > This patch adds num_discards to stat discarded pages, then export it to sysfs
> > > > > > for displaying.
> > > > > >
> > > > >
> > > > > a side question: we account discarded pages via slot free notify in
> > > > > notify_free and via req_discard in num_discards. how about accounting
> > > > > both of them in num_discards? because, after all, they account a number
> > > > > of discarded pages (zram_free_page()). or there any particular reason we
> > > > > want to distinguish.
> > > >
> > > > Yeah, I agree with you as I have no such reason unless there are our users'
> > > > explicitly requirement for showing notify_free/num_discards separately later.
> > > >
> > > > How do you think of sending another patch to merge these two counts?
> > > >
> > >
> > > Minchan, what do you think? let's account discarded pages in one place.
> > 
> > First of all, I'd like to know why we need num_discards.
> > It should be in description and depends on it whether we should merge both
> > counts or separate.
> 
> Oh, it's my mistaken.
> 
> When commit 	9b9913d80b2896ecd9e0a1a8f167ccad66fac79c (Staging: zram: Update
> zram documentation) and commit e98419c23b1a189c932775f7833e94cb5230a16b (Staging:
> zram: Document sysfs entries) description related to 'discard' stat was designed
> and added to zram.txt and sysfs-block-zram, but without implementation of function
> for handling discard request, description in documents were removed in commit
> 8dd1d3247e6c00b50ef83934ea8b22a1590015de (zram: document failed_reads,
> failed_writes stats)

Thanks for letting me know the history.

> 
> For now, we have already supported discard handling, so it's better to resume
> the stat of discard number, this discard stat supports user one more kind of runtime
> information of zram as other stats supported.
> 
> How do you think?

I'm not strong against the idea but just "resume is better" and
"one more is problem as other stats supported" is not logical
to me.

You should explain why we need such new stat so that user can take
what kinds of benefit from that. Otherwise, we couldn't know the stat
is best or not for the goal.


I might be paranoid about small stuff and I admit I'm not good for it,
too but pz, understand that adding the new feature requires a
good description which should include clear goal.

I hope I'm not discouraging. :)

> 
> > 
> > Thanks.
> > 
> > 
> > 
> > >
> > > > One more thing is that I am missing to update document of zram, sorry about
> > > > that, let me update it in v2.
> > >
> > > thanks.
> > >
> > > 	-ss
> > >
> > > > Thanks,
> > > > Yu
> > > >
> > > > >
> > > > > 	-ss
> > > > >
> > > > > > Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> > > > > > ---
> > > > > >  Documentation/ABI/testing/sysfs-block-zram | 10 ++++++++++
> > > > > >  drivers/block/zram/zram_drv.c              |  3 +++
> > > > > >  drivers/block/zram/zram_drv.h              |  1 +
> > > > > >  3 files changed, 14 insertions(+)
> > > > > >
> > > > > > diff --git a/Documentation/ABI/testing/sysfs-block-zram
> > > > > b/Documentation/ABI/testing/sysfs-block-zram
> > > > > > index 70ec992..fa8936e 100644
> > > > > > --- a/Documentation/ABI/testing/sysfs-block-zram
> > > > > > +++ b/Documentation/ABI/testing/sysfs-block-zram
> > > > > > @@ -57,6 +57,16 @@ Description:
> > > > > >  		The failed_writes file is read-only and specifies the number of
> > > > > >  		failed writes happened on this device.
> > > > > >
> > > > > > +
> > > > > > +What:		/sys/block/zram<id>/num_discards
> > > > > > +Date:		August 2014
> > > > > > +Contact:	Chao Yu <chao2.yu@samsung.com>
> > > > > > +Description:
> > > > > > +		The num_discards file is read-only and specifies the number of
> > > > > > +		physical blocks which are discarded by this device. These blocks
> > > > > > +		are included in discard request which is sended by filesystem as
> > > > > > +		the blocks are no longer used.
> > > > > > +
> > > > > >  What:		/sys/block/zram<id>/max_comp_streams
> > > > > >  Date:		February 2014
> > > > > >  Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > > > > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > > > > > index d00831c..904e7a5 100644
> > > > > > --- a/drivers/block/zram/zram_drv.c
> > > > > > +++ b/drivers/block/zram/zram_drv.c
> > > > > > @@ -606,6 +606,7 @@ static void zram_bio_discard(struct zram *zram, u32 index,
> > > > > >  		bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> > > > > >  		zram_free_page(zram, index);
> > > > > >  		bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> > > > > > +		atomic64_inc(&zram->stats.num_discards);
> > > > > >  		index++;
> > > > > >  		n -= PAGE_SIZE;
> > > > > >  	}
> > > > > > @@ -866,6 +867,7 @@ ZRAM_ATTR_RO(num_reads);
> > > > > >  ZRAM_ATTR_RO(num_writes);
> > > > > >  ZRAM_ATTR_RO(failed_reads);
> > > > > >  ZRAM_ATTR_RO(failed_writes);
> > > > > > +ZRAM_ATTR_RO(num_discards);
> > > > > >  ZRAM_ATTR_RO(invalid_io);
> > > > > >  ZRAM_ATTR_RO(notify_free);
> > > > > >  ZRAM_ATTR_RO(zero_pages);
> > > > > > @@ -879,6 +881,7 @@ static struct attribute *zram_disk_attrs[] = {
> > > > > >  	&dev_attr_num_writes.attr,
> > > > > >  	&dev_attr_failed_reads.attr,
> > > > > >  	&dev_attr_failed_writes.attr,
> > > > > > +	&dev_attr_num_discards.attr,
> > > > > >  	&dev_attr_invalid_io.attr,
> > > > > >  	&dev_attr_notify_free.attr,
> > > > > >  	&dev_attr_zero_pages.attr,
> > > > > > diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> > > > > > index e0f725c..2994aaf 100644
> > > > > > --- a/drivers/block/zram/zram_drv.h
> > > > > > +++ b/drivers/block/zram/zram_drv.h
> > > > > > @@ -86,6 +86,7 @@ struct zram_stats {
> > > > > >  	atomic64_t num_writes;	/* --do-- */
> > > > > >  	atomic64_t failed_reads;	/* can happen when memory is too low */
> > > > > >  	atomic64_t failed_writes;	/* can happen when memory is too low */
> > > > > > +	atomic64_t num_discards;	/* no. of discarded pages */
> > > > > >  	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
> > > > > >  	atomic64_t notify_free;	/* no. of swap slot free notifications */
> > > > > >  	atomic64_t zero_pages;		/* no. of zero filled pages */
> > > > > > --
> > > > > > 2.0.1.474.g72c7794
> > > > > >
> > > > > >
> > > > >
> > > > > --
> > > > > 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>
> > > >
> > >
> > > --
> > > 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>
> > 
> > --
> > Kind regards,
> > Minchan Kim
> 
> --
> 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>

-- 
Kind regards,
Minchan Kim

Re: [PATCH] zram: add num_discards for discarded pages stat

[Minchan Kim]

Hi Chao,

On Wed, Aug 20, 2014 at 04:20:48PM +0800, Chao Yu wrote:
> Hi Minchan,
> 
> > -----Original Message-----
> > From: Minchan Kim [mailto:minchan@kernel.org]
> > Sent: Wednesday, August 20, 2014 10:09 AM
> > To: Sergey Senozhatsky
> > Cc: Chao Yu; linux-kernel@vger.kernel.org; linux-mm@kvack.org; ngupta@vflare.org; 'Jerome
> > Marchand'; 'Andrew Morton'
> > Subject: Re: [PATCH] zram: add num_discards for discarded pages stat
> > 
> > Hi Sergey,
> > 
> > On Tue, Aug 19, 2014 at 08:25:00PM +0900, Sergey Senozhatsky wrote:
> > > Hello,
> > >
> > > On (08/19/14 13:45), Chao Yu wrote:
> > > > > On (08/15/14 11:27), Chao Yu wrote:
> > > > > > Now we have supported handling discard request which is sended by filesystem,
> > > > > > but no interface could be used to show information of discard.
> > > > > > This patch adds num_discards to stat discarded pages, then export it to sysfs
> > > > > > for displaying.
> > > > > >
> > > > >
> > > > > a side question: we account discarded pages via slot free notify in
> > > > > notify_free and via req_discard in num_discards. how about accounting
> > > > > both of them in num_discards? because, after all, they account a number
> > > > > of discarded pages (zram_free_page()). or there any particular reason we
> > > > > want to distinguish.
> > > >
> > > > Yeah, I agree with you as I have no such reason unless there are our users'
> > > > explicitly requirement for showing notify_free/num_discards separately later.
> > > >
> > > > How do you think of sending another patch to merge these two counts?
> > > >
> > >
> > > Minchan, what do you think? let's account discarded pages in one place.
> > 
> > First of all, I'd like to know why we need num_discards.
> > It should be in description and depends on it whether we should merge both
> > counts or separate.
> 
> Oh, it's my mistaken.
> 
> When commit 	9b9913d80b2896ecd9e0a1a8f167ccad66fac79c (Staging: zram: Update
> zram documentation) and commit e98419c23b1a189c932775f7833e94cb5230a16b (Staging:
> zram: Document sysfs entries) description related to 'discard' stat was designed
> and added to zram.txt and sysfs-block-zram, but without implementation of function
> for handling discard request, description in documents were removed in commit
> 8dd1d3247e6c00b50ef83934ea8b22a1590015de (zram: document failed_reads,
> failed_writes stats)

Thanks for letting me know the history.

> 
> For now, we have already supported discard handling, so it's better to resume
> the stat of discard number, this discard stat supports user one more kind of runtime
> information of zram as other stats supported.
> 
> How do you think?

I'm not strong against the idea but just "resume is better" and
"one more is problem as other stats supported" is not logical
to me.

You should explain why we need such new stat so that user can take
what kinds of benefit from that. Otherwise, we couldn't know the stat
is best or not for the goal.


I might be paranoid about small stuff and I admit I'm not good for it,
too but pz, understand that adding the new feature requires a
good description which should include clear goal.

I hope I'm not discouraging. :)

> 
> > 
> > Thanks.
> > 
> > 
> > 
> > >
> > > > One more thing is that I am missing to update document of zram, sorry about
> > > > that, let me update it in v2.
> > >
> > > thanks.
> > >
> > > 	-ss
> > >
> > > > Thanks,
> > > > Yu
> > > >
> > > > >
> > > > > 	-ss
> > > > >
> > > > > > Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> > > > > > ---
> > > > > >  Documentation/ABI/testing/sysfs-block-zram | 10 ++++++++++
> > > > > >  drivers/block/zram/zram_drv.c              |  3 +++
> > > > > >  drivers/block/zram/zram_drv.h              |  1 +
> > > > > >  3 files changed, 14 insertions(+)
> > > > > >
> > > > > > diff --git a/Documentation/ABI/testing/sysfs-block-zram
> > > > > b/Documentation/ABI/testing/sysfs-block-zram
> > > > > > index 70ec992..fa8936e 100644
> > > > > > --- a/Documentation/ABI/testing/sysfs-block-zram
> > > > > > +++ b/Documentation/ABI/testing/sysfs-block-zram
> > > > > > @@ -57,6 +57,16 @@ Description:
> > > > > >  		The failed_writes file is read-only and specifies the number of
> > > > > >  		failed writes happened on this device.
> > > > > >
> > > > > > +
> > > > > > +What:		/sys/block/zram<id>/num_discards
> > > > > > +Date:		August 2014
> > > > > > +Contact:	Chao Yu <chao2.yu@samsung.com>
> > > > > > +Description:
> > > > > > +		The num_discards file is read-only and specifies the number of
> > > > > > +		physical blocks which are discarded by this device. These blocks
> > > > > > +		are included in discard request which is sended by filesystem as
> > > > > > +		the blocks are no longer used.
> > > > > > +
> > > > > >  What:		/sys/block/zram<id>/max_comp_streams
> > > > > >  Date:		February 2014
> > > > > >  Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > > > > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > > > > > index d00831c..904e7a5 100644
> > > > > > --- a/drivers/block/zram/zram_drv.c
> > > > > > +++ b/drivers/block/zram/zram_drv.c
> > > > > > @@ -606,6 +606,7 @@ static void zram_bio_discard(struct zram *zram, u32 index,
> > > > > >  		bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> > > > > >  		zram_free_page(zram, index);
> > > > > >  		bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> > > > > > +		atomic64_inc(&zram->stats.num_discards);
> > > > > >  		index++;
> > > > > >  		n -= PAGE_SIZE;
> > > > > >  	}
> > > > > > @@ -866,6 +867,7 @@ ZRAM_ATTR_RO(num_reads);
> > > > > >  ZRAM_ATTR_RO(num_writes);
> > > > > >  ZRAM_ATTR_RO(failed_reads);
> > > > > >  ZRAM_ATTR_RO(failed_writes);
> > > > > > +ZRAM_ATTR_RO(num_discards);
> > > > > >  ZRAM_ATTR_RO(invalid_io);
> > > > > >  ZRAM_ATTR_RO(notify_free);
> > > > > >  ZRAM_ATTR_RO(zero_pages);
> > > > > > @@ -879,6 +881,7 @@ static struct attribute *zram_disk_attrs[] = {
> > > > > >  	&dev_attr_num_writes.attr,
> > > > > >  	&dev_attr_failed_reads.attr,
> > > > > >  	&dev_attr_failed_writes.attr,
> > > > > > +	&dev_attr_num_discards.attr,
> > > > > >  	&dev_attr_invalid_io.attr,
> > > > > >  	&dev_attr_notify_free.attr,
> > > > > >  	&dev_attr_zero_pages.attr,
> > > > > > diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> > > > > > index e0f725c..2994aaf 100644
> > > > > > --- a/drivers/block/zram/zram_drv.h
> > > > > > +++ b/drivers/block/zram/zram_drv.h
> > > > > > @@ -86,6 +86,7 @@ struct zram_stats {
> > > > > >  	atomic64_t num_writes;	/* --do-- */
> > > > > >  	atomic64_t failed_reads;	/* can happen when memory is too low */
> > > > > >  	atomic64_t failed_writes;	/* can happen when memory is too low */
> > > > > > +	atomic64_t num_discards;	/* no. of discarded pages */
> > > > > >  	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
> > > > > >  	atomic64_t notify_free;	/* no. of swap slot free notifications */
> > > > > >  	atomic64_t zero_pages;		/* no. of zero filled pages */
> > > > > > --
> > > > > > 2.0.1.474.g72c7794
> > > > > >
> > > > > >
> > > > >
> > > > > --
> > > > > 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>
> > > >
> > >
> > > --
> > > 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>
> > 
> > --
> > Kind regards,
> > Minchan Kim
> 
> --
> 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>

-- 
Kind regards,
Minchan Kim

--
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] zram: add num_discards for discarded pages stat

[Chao Yu]

Hi Minchan,

> -----Original Message-----
> From: Minchan Kim [mailto:minchan@kernel.org]
> Sent: Thursday, August 21, 2014 9:19 AM
> To: Chao Yu
> Cc: 'Sergey Senozhatsky'; linux-kernel@vger.kernel.org; linux-mm@kvack.org; ngupta@vflare.org;
> 'Jerome Marchand'; 'Andrew Morton'
> Subject: Re: [PATCH] zram: add num_discards for discarded pages stat
> 
> Hi Chao,
> 
> On Wed, Aug 20, 2014 at 04:20:48PM +0800, Chao Yu wrote:
> > Hi Minchan,
> >
> > > -----Original Message-----
> > > From: Minchan Kim [mailto:minchan@kernel.org]
> > > Sent: Wednesday, August 20, 2014 10:09 AM
> > > To: Sergey Senozhatsky
> > > Cc: Chao Yu; linux-kernel@vger.kernel.org; linux-mm@kvack.org; ngupta@vflare.org; 'Jerome
> > > Marchand'; 'Andrew Morton'
> > > Subject: Re: [PATCH] zram: add num_discards for discarded pages stat
> > >
> > > Hi Sergey,
> > >
> > > On Tue, Aug 19, 2014 at 08:25:00PM +0900, Sergey Senozhatsky wrote:
> > > > Hello,
> > > >
> > > > On (08/19/14 13:45), Chao Yu wrote:
> > > > > > On (08/15/14 11:27), Chao Yu wrote:
> > > > > > > Now we have supported handling discard request which is sended by filesystem,
> > > > > > > but no interface could be used to show information of discard.
> > > > > > > This patch adds num_discards to stat discarded pages, then export it to sysfs
> > > > > > > for displaying.
> > > > > > >
> > > > > >
> > > > > > a side question: we account discarded pages via slot free notify in
> > > > > > notify_free and via req_discard in num_discards. how about accounting
> > > > > > both of them in num_discards? because, after all, they account a number
> > > > > > of discarded pages (zram_free_page()). or there any particular reason we
> > > > > > want to distinguish.
> > > > >
> > > > > Yeah, I agree with you as I have no such reason unless there are our users'
> > > > > explicitly requirement for showing notify_free/num_discards separately later.
> > > > >
> > > > > How do you think of sending another patch to merge these two counts?
> > > > >
> > > >
> > > > Minchan, what do you think? let's account discarded pages in one place.
> > >
> > > First of all, I'd like to know why we need num_discards.
> > > It should be in description and depends on it whether we should merge both
> > > counts or separate.
> >
> > Oh, it's my mistaken.
> >
> > When commit 	9b9913d80b2896ecd9e0a1a8f167ccad66fac79c (Staging: zram: Update
> > zram documentation) and commit e98419c23b1a189c932775f7833e94cb5230a16b (Staging:
> > zram: Document sysfs entries) description related to 'discard' stat was designed
> > and added to zram.txt and sysfs-block-zram, but without implementation of function
> > for handling discard request, description in documents were removed in commit
> > 8dd1d3247e6c00b50ef83934ea8b22a1590015de (zram: document failed_reads,
> > failed_writes stats)
> 
> Thanks for letting me know the history.
> 
> >
> > For now, we have already supported discard handling, so it's better to resume
> > the stat of discard number, this discard stat supports user one more kind of runtime
> > information of zram as other stats supported.
> >
> > How do you think?
> 
> I'm not strong against the idea but just "resume is better" and
> "one more is problem as other stats supported" is not logical
> to me.

OK, I assume maybe to match the principle for adopting and discarding
those stats in original version of zram will do some help, actually I'm wrong.

> 
> You should explain why we need such new stat so that user can take
> what kinds of benefit from that. Otherwise, we couldn't know the stat
> is best or not for the goal.

Alright, it's reasonable from this perspective.

> 
> 
> I might be paranoid about small stuff and I admit I'm not good for it,
> too but pz, understand that adding the new feature requires a
> good description which should include clear goal.

Well, I can understand and accept that.

> 
> I hope I'm not discouraging. :)

Nope, please let me try again, :)

Since we have supported handling discard request in this commit
f4659d8e620d08bd1a84a8aec5d2f5294a242764 (zram: support REQ_DISCARD), zram got
one more chance to free unused memory whenever received discard request. But
without stating for discard request, there is no method for user to know whether
discard request has been handled by zram or how many blocks were discarded by
zram when user wants to know the effect of discard.

In this patch, we add num_discards to stat discarded pages, and export it to
sysfs for users.


Regards,
Yu

> 
> >
> > >
> > > Thanks.
> > >
> > >
> > >
> > > >
> > > > > One more thing is that I am missing to update document of zram, sorry about
> > > > > that, let me update it in v2.
> > > >
> > > > thanks.
> > > >
> > > > 	-ss
> > > >
> > > > > Thanks,
> > > > > Yu
> > > > >
> > > > > >
> > > > > > 	-ss
> > > > > >
> > > > > > > Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> > > > > > > ---
> > > > > > >  Documentation/ABI/testing/sysfs-block-zram | 10 ++++++++++
> > > > > > >  drivers/block/zram/zram_drv.c              |  3 +++
> > > > > > >  drivers/block/zram/zram_drv.h              |  1 +
> > > > > > >  3 files changed, 14 insertions(+)
> > > > > > >
> > > > > > > diff --git a/Documentation/ABI/testing/sysfs-block-zram
> > > > > > b/Documentation/ABI/testing/sysfs-block-zram
> > > > > > > index 70ec992..fa8936e 100644
> > > > > > > --- a/Documentation/ABI/testing/sysfs-block-zram
> > > > > > > +++ b/Documentation/ABI/testing/sysfs-block-zram
> > > > > > > @@ -57,6 +57,16 @@ Description:
> > > > > > >  		The failed_writes file is read-only and specifies the number of
> > > > > > >  		failed writes happened on this device.
> > > > > > >
> > > > > > > +
> > > > > > > +What:		/sys/block/zram<id>/num_discards
> > > > > > > +Date:		August 2014
> > > > > > > +Contact:	Chao Yu <chao2.yu@samsung.com>
> > > > > > > +Description:
> > > > > > > +		The num_discards file is read-only and specifies the number of
> > > > > > > +		physical blocks which are discarded by this device. These blocks
> > > > > > > +		are included in discard request which is sended by filesystem as
> > > > > > > +		the blocks are no longer used.
> > > > > > > +
> > > > > > >  What:		/sys/block/zram<id>/max_comp_streams
> > > > > > >  Date:		February 2014
> > > > > > >  Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > > > > > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > > > > > > index d00831c..904e7a5 100644
> > > > > > > --- a/drivers/block/zram/zram_drv.c
> > > > > > > +++ b/drivers/block/zram/zram_drv.c
> > > > > > > @@ -606,6 +606,7 @@ static void zram_bio_discard(struct zram *zram, u32 index,
> > > > > > >  		bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> > > > > > >  		zram_free_page(zram, index);
> > > > > > >  		bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> > > > > > > +		atomic64_inc(&zram->stats.num_discards);
> > > > > > >  		index++;
> > > > > > >  		n -= PAGE_SIZE;
> > > > > > >  	}
> > > > > > > @@ -866,6 +867,7 @@ ZRAM_ATTR_RO(num_reads);
> > > > > > >  ZRAM_ATTR_RO(num_writes);
> > > > > > >  ZRAM_ATTR_RO(failed_reads);
> > > > > > >  ZRAM_ATTR_RO(failed_writes);
> > > > > > > +ZRAM_ATTR_RO(num_discards);
> > > > > > >  ZRAM_ATTR_RO(invalid_io);
> > > > > > >  ZRAM_ATTR_RO(notify_free);
> > > > > > >  ZRAM_ATTR_RO(zero_pages);
> > > > > > > @@ -879,6 +881,7 @@ static struct attribute *zram_disk_attrs[] = {
> > > > > > >  	&dev_attr_num_writes.attr,
> > > > > > >  	&dev_attr_failed_reads.attr,
> > > > > > >  	&dev_attr_failed_writes.attr,
> > > > > > > +	&dev_attr_num_discards.attr,
> > > > > > >  	&dev_attr_invalid_io.attr,
> > > > > > >  	&dev_attr_notify_free.attr,
> > > > > > >  	&dev_attr_zero_pages.attr,
> > > > > > > diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> > > > > > > index e0f725c..2994aaf 100644
> > > > > > > --- a/drivers/block/zram/zram_drv.h
> > > > > > > +++ b/drivers/block/zram/zram_drv.h
> > > > > > > @@ -86,6 +86,7 @@ struct zram_stats {
> > > > > > >  	atomic64_t num_writes;	/* --do-- */
> > > > > > >  	atomic64_t failed_reads;	/* can happen when memory is too low */
> > > > > > >  	atomic64_t failed_writes;	/* can happen when memory is too low */
> > > > > > > +	atomic64_t num_discards;	/* no. of discarded pages */
> > > > > > >  	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
> > > > > > >  	atomic64_t notify_free;	/* no. of swap slot free notifications */
> > > > > > >  	atomic64_t zero_pages;		/* no. of zero filled pages */
> > > > > > > --
> > > > > > > 2.0.1.474.g72c7794
> > > > > > >
> > > > > > >
> > > > > >
> > > > > > --
> > > > > > 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>
> > > > >
> > > >
> > > > --
> > > > 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>
> > >
> > > --
> > > Kind regards,
> > > Minchan Kim
> >
> > --
> > 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>
> 
> --
> Kind regards,
> Minchan Kim

RE: [PATCH] zram: add num_discards for discarded pages stat

[Chao Yu]

Hi Minchan,

> -----Original Message-----
> From: Minchan Kim [mailto:minchan@kernel.org]
> Sent: Thursday, August 21, 2014 9:19 AM
> To: Chao Yu
> Cc: 'Sergey Senozhatsky'; linux-kernel@vger.kernel.org; linux-mm@kvack.org; ngupta@vflare.org;
> 'Jerome Marchand'; 'Andrew Morton'
> Subject: Re: [PATCH] zram: add num_discards for discarded pages stat
> 
> Hi Chao,
> 
> On Wed, Aug 20, 2014 at 04:20:48PM +0800, Chao Yu wrote:
> > Hi Minchan,
> >
> > > -----Original Message-----
> > > From: Minchan Kim [mailto:minchan@kernel.org]
> > > Sent: Wednesday, August 20, 2014 10:09 AM
> > > To: Sergey Senozhatsky
> > > Cc: Chao Yu; linux-kernel@vger.kernel.org; linux-mm@kvack.org; ngupta@vflare.org; 'Jerome
> > > Marchand'; 'Andrew Morton'
> > > Subject: Re: [PATCH] zram: add num_discards for discarded pages stat
> > >
> > > Hi Sergey,
> > >
> > > On Tue, Aug 19, 2014 at 08:25:00PM +0900, Sergey Senozhatsky wrote:
> > > > Hello,
> > > >
> > > > On (08/19/14 13:45), Chao Yu wrote:
> > > > > > On (08/15/14 11:27), Chao Yu wrote:
> > > > > > > Now we have supported handling discard request which is sended by filesystem,
> > > > > > > but no interface could be used to show information of discard.
> > > > > > > This patch adds num_discards to stat discarded pages, then export it to sysfs
> > > > > > > for displaying.
> > > > > > >
> > > > > >
> > > > > > a side question: we account discarded pages via slot free notify in
> > > > > > notify_free and via req_discard in num_discards. how about accounting
> > > > > > both of them in num_discards? because, after all, they account a number
> > > > > > of discarded pages (zram_free_page()). or there any particular reason we
> > > > > > want to distinguish.
> > > > >
> > > > > Yeah, I agree with you as I have no such reason unless there are our users'
> > > > > explicitly requirement for showing notify_free/num_discards separately later.
> > > > >
> > > > > How do you think of sending another patch to merge these two counts?
> > > > >
> > > >
> > > > Minchan, what do you think? let's account discarded pages in one place.
> > >
> > > First of all, I'd like to know why we need num_discards.
> > > It should be in description and depends on it whether we should merge both
> > > counts or separate.
> >
> > Oh, it's my mistaken.
> >
> > When commit 	9b9913d80b2896ecd9e0a1a8f167ccad66fac79c (Staging: zram: Update
> > zram documentation) and commit e98419c23b1a189c932775f7833e94cb5230a16b (Staging:
> > zram: Document sysfs entries) description related to 'discard' stat was designed
> > and added to zram.txt and sysfs-block-zram, but without implementation of function
> > for handling discard request, description in documents were removed in commit
> > 8dd1d3247e6c00b50ef83934ea8b22a1590015de (zram: document failed_reads,
> > failed_writes stats)
> 
> Thanks for letting me know the history.
> 
> >
> > For now, we have already supported discard handling, so it's better to resume
> > the stat of discard number, this discard stat supports user one more kind of runtime
> > information of zram as other stats supported.
> >
> > How do you think?
> 
> I'm not strong against the idea but just "resume is better" and
> "one more is problem as other stats supported" is not logical
> to me.

OK, I assume maybe to match the principle for adopting and discarding
those stats in original version of zram will do some help, actually I'm wrong.

> 
> You should explain why we need such new stat so that user can take
> what kinds of benefit from that. Otherwise, we couldn't know the stat
> is best or not for the goal.

Alright, it's reasonable from this perspective.

> 
> 
> I might be paranoid about small stuff and I admit I'm not good for it,
> too but pz, understand that adding the new feature requires a
> good description which should include clear goal.

Well, I can understand and accept that.

> 
> I hope I'm not discouraging. :)

Nope, please let me try again, :)

Since we have supported handling discard request in this commit
f4659d8e620d08bd1a84a8aec5d2f5294a242764 (zram: support REQ_DISCARD), zram got
one more chance to free unused memory whenever received discard request. But
without stating for discard request, there is no method for user to know whether
discard request has been handled by zram or how many blocks were discarded by
zram when user wants to know the effect of discard.

In this patch, we add num_discards to stat discarded pages, and export it to
sysfs for users.


Regards,
Yu

> 
> >
> > >
> > > Thanks.
> > >
> > >
> > >
> > > >
> > > > > One more thing is that I am missing to update document of zram, sorry about
> > > > > that, let me update it in v2.
> > > >
> > > > thanks.
> > > >
> > > > 	-ss
> > > >
> > > > > Thanks,
> > > > > Yu
> > > > >
> > > > > >
> > > > > > 	-ss
> > > > > >
> > > > > > > Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> > > > > > > ---
> > > > > > >  Documentation/ABI/testing/sysfs-block-zram | 10 ++++++++++
> > > > > > >  drivers/block/zram/zram_drv.c              |  3 +++
> > > > > > >  drivers/block/zram/zram_drv.h              |  1 +
> > > > > > >  3 files changed, 14 insertions(+)
> > > > > > >
> > > > > > > diff --git a/Documentation/ABI/testing/sysfs-block-zram
> > > > > > b/Documentation/ABI/testing/sysfs-block-zram
> > > > > > > index 70ec992..fa8936e 100644
> > > > > > > --- a/Documentation/ABI/testing/sysfs-block-zram
> > > > > > > +++ b/Documentation/ABI/testing/sysfs-block-zram
> > > > > > > @@ -57,6 +57,16 @@ Description:
> > > > > > >  		The failed_writes file is read-only and specifies the number of
> > > > > > >  		failed writes happened on this device.
> > > > > > >
> > > > > > > +
> > > > > > > +What:		/sys/block/zram<id>/num_discards
> > > > > > > +Date:		August 2014
> > > > > > > +Contact:	Chao Yu <chao2.yu@samsung.com>
> > > > > > > +Description:
> > > > > > > +		The num_discards file is read-only and specifies the number of
> > > > > > > +		physical blocks which are discarded by this device. These blocks
> > > > > > > +		are included in discard request which is sended by filesystem as
> > > > > > > +		the blocks are no longer used.
> > > > > > > +
> > > > > > >  What:		/sys/block/zram<id>/max_comp_streams
> > > > > > >  Date:		February 2014
> > > > > > >  Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > > > > > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > > > > > > index d00831c..904e7a5 100644
> > > > > > > --- a/drivers/block/zram/zram_drv.c
> > > > > > > +++ b/drivers/block/zram/zram_drv.c
> > > > > > > @@ -606,6 +606,7 @@ static void zram_bio_discard(struct zram *zram, u32 index,
> > > > > > >  		bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> > > > > > >  		zram_free_page(zram, index);
> > > > > > >  		bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> > > > > > > +		atomic64_inc(&zram->stats.num_discards);
> > > > > > >  		index++;
> > > > > > >  		n -= PAGE_SIZE;
> > > > > > >  	}
> > > > > > > @@ -866,6 +867,7 @@ ZRAM_ATTR_RO(num_reads);
> > > > > > >  ZRAM_ATTR_RO(num_writes);
> > > > > > >  ZRAM_ATTR_RO(failed_reads);
> > > > > > >  ZRAM_ATTR_RO(failed_writes);
> > > > > > > +ZRAM_ATTR_RO(num_discards);
> > > > > > >  ZRAM_ATTR_RO(invalid_io);
> > > > > > >  ZRAM_ATTR_RO(notify_free);
> > > > > > >  ZRAM_ATTR_RO(zero_pages);
> > > > > > > @@ -879,6 +881,7 @@ static struct attribute *zram_disk_attrs[] = {
> > > > > > >  	&dev_attr_num_writes.attr,
> > > > > > >  	&dev_attr_failed_reads.attr,
> > > > > > >  	&dev_attr_failed_writes.attr,
> > > > > > > +	&dev_attr_num_discards.attr,
> > > > > > >  	&dev_attr_invalid_io.attr,
> > > > > > >  	&dev_attr_notify_free.attr,
> > > > > > >  	&dev_attr_zero_pages.attr,
> > > > > > > diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> > > > > > > index e0f725c..2994aaf 100644
> > > > > > > --- a/drivers/block/zram/zram_drv.h
> > > > > > > +++ b/drivers/block/zram/zram_drv.h
> > > > > > > @@ -86,6 +86,7 @@ struct zram_stats {
> > > > > > >  	atomic64_t num_writes;	/* --do-- */
> > > > > > >  	atomic64_t failed_reads;	/* can happen when memory is too low */
> > > > > > >  	atomic64_t failed_writes;	/* can happen when memory is too low */
> > > > > > > +	atomic64_t num_discards;	/* no. of discarded pages */
> > > > > > >  	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
> > > > > > >  	atomic64_t notify_free;	/* no. of swap slot free notifications */
> > > > > > >  	atomic64_t zero_pages;		/* no. of zero filled pages */
> > > > > > > --
> > > > > > > 2.0.1.474.g72c7794
> > > > > > >
> > > > > > >
> > > > > >
> > > > > > --
> > > > > > 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>
> > > > >
> > > >
> > > > --
> > > > 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>
> > >
> > > --
> > > Kind regards,
> > > Minchan Kim
> >
> > --
> > 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>
> 
> --
> Kind regards,
> Minchan Kim

--
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] zram: add num_discards for discarded pages stat

[Sergey Senozhatsky]

On (08/21/14 17:09), Chao Yu wrote:
[cut]
> > 
> > I hope I'm not discouraging. :)
> 
> Nope, please let me try again, :)
> 
> Since we have supported handling discard request in this commit
> f4659d8e620d08bd1a84a8aec5d2f5294a242764 (zram: support REQ_DISCARD), zram got
> one more chance to free unused memory whenever received discard request. But
> without stating for discard request, there is no method for user to know whether
> discard request has been handled by zram or how many blocks were discarded by
> zram when user wants to know the effect of discard.
> 
> In this patch, we add num_discards to stat discarded pages, and export it to
> sysfs for users.
> 

In other words, here is my proposal:

-----8<-----8<-----

Subject: [PATCH] zram: use notify_free to account all free notifications

notify_free device attribute accounts the number of slot free notifications
and internally represents the number of zram_free_page() calls. Slot free
notifications are sent only when device is used as a swap device, hence
notify_free is used only for swap devices. Since f4659d8e620d08 (zram:
support REQ_DISCARD) ZRAM handles yet another one free notification (also
via zram_free_page() call) -- REQ_DISCARD requests, which are sent by a
filesystem, whenever some data blocks are discarded. However, there is no
way to know the number of notifications in the latter case.

Use notify_free to account the number of pages freed in zram_free_page(),
instead of accounting only swap_slot_free_notify() calls (each
zram_slot_free_notify() call frees one page).

This means that depending on usage scenario notify_free represents:
 a) the number of pages freed because of slot free notifications, which is
   equal to the number of swap_slot_free_notify() calls, so there is no
   behaviour change

 b) the number of pages freed because of REQ_DISCARD notifications

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 Documentation/ABI/testing/sysfs-block-zram | 13 ++++++++-----
 drivers/block/zram/zram_drv.c              |  2 +-
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-block-zram b/Documentation/ABI/testing/sysfs-block-zram
index 70ec992..73ed400 100644
--- a/Documentation/ABI/testing/sysfs-block-zram
+++ b/Documentation/ABI/testing/sysfs-block-zram
@@ -77,11 +77,14 @@ What:		/sys/block/zram<id>/notify_free
 Date:		August 2010
 Contact:	Nitin Gupta <ngupta@vflare.org>
 Description:
-		The notify_free file is read-only and specifies the number of
-		swap slot free notifications received by this device. These
-		notifications are sent to a swap block device when a swap slot
-		is freed. This statistic is applicable only when this disk is
-		being used as a swap disk.
+		The notify_free file is read-only. Depending on device usage
+		scenario it may account a) the number of swap slot free
+		notifications or b) the number of REQ_DISCARD requests sent
+		by bio. The former ones are sent to a swap block device when a
+		swap slot is freed, which implies that this disk is being used
+		as a swap disk. The latter ones are sent by filesystem mounted
+		with discard option, whenever some data blocks are getting
+		discarded.
 
 What:		/sys/block/zram<id>/zero_pages
 Date:		August 2010
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index d00831c..c2e7127 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -344,6 +344,7 @@ static void zram_free_page(struct zram *zram, size_t index)
 	atomic64_sub(zram_get_obj_size(meta, index),
 			&zram->stats.compr_data_size);
 	atomic64_dec(&zram->stats.pages_stored);
+	atomic64_inc(&zram->stats.notify_free);
 
 	meta->table[index].handle = 0;
 	zram_set_obj_size(meta, index, 0);
@@ -843,7 +844,6 @@ static void zram_slot_free_notify(struct block_device *bdev,
 	bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
 	zram_free_page(zram, index);
 	bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
-	atomic64_inc(&zram->stats.notify_free);
 }
 
 static const struct block_device_operations zram_devops = {
-- 
2.1.0.233.g9eef2c8

Re: [PATCH] zram: add num_discards for discarded pages stat

[Sergey Senozhatsky]

On (08/21/14 17:09), Chao Yu wrote:
[cut]
> > 
> > I hope I'm not discouraging. :)
> 
> Nope, please let me try again, :)
> 
> Since we have supported handling discard request in this commit
> f4659d8e620d08bd1a84a8aec5d2f5294a242764 (zram: support REQ_DISCARD), zram got
> one more chance to free unused memory whenever received discard request. But
> without stating for discard request, there is no method for user to know whether
> discard request has been handled by zram or how many blocks were discarded by
> zram when user wants to know the effect of discard.
> 
> In this patch, we add num_discards to stat discarded pages, and export it to
> sysfs for users.
> 

In other words, here is my proposal:

-----8<-----8<-----

Subject: [PATCH] zram: use notify_free to account all free notifications

notify_free device attribute accounts the number of slot free notifications
and internally represents the number of zram_free_page() calls. Slot free
notifications are sent only when device is used as a swap device, hence
notify_free is used only for swap devices. Since f4659d8e620d08 (zram:
support REQ_DISCARD) ZRAM handles yet another one free notification (also
via zram_free_page() call) -- REQ_DISCARD requests, which are sent by a
filesystem, whenever some data blocks are discarded. However, there is no
way to know the number of notifications in the latter case.

Use notify_free to account the number of pages freed in zram_free_page(),
instead of accounting only swap_slot_free_notify() calls (each
zram_slot_free_notify() call frees one page).

This means that depending on usage scenario notify_free represents:
 a) the number of pages freed because of slot free notifications, which is
   equal to the number of swap_slot_free_notify() calls, so there is no
   behaviour change

 b) the number of pages freed because of REQ_DISCARD notifications

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 Documentation/ABI/testing/sysfs-block-zram | 13 ++++++++-----
 drivers/block/zram/zram_drv.c              |  2 +-
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-block-zram b/Documentation/ABI/testing/sysfs-block-zram
index 70ec992..73ed400 100644
--- a/Documentation/ABI/testing/sysfs-block-zram
+++ b/Documentation/ABI/testing/sysfs-block-zram
@@ -77,11 +77,14 @@ What:		/sys/block/zram<id>/notify_free
 Date:		August 2010
 Contact:	Nitin Gupta <ngupta@vflare.org>
 Description:
-		The notify_free file is read-only and specifies the number of
-		swap slot free notifications received by this device. These
-		notifications are sent to a swap block device when a swap slot
-		is freed. This statistic is applicable only when this disk is
-		being used as a swap disk.
+		The notify_free file is read-only. Depending on device usage
+		scenario it may account a) the number of swap slot free
+		notifications or b) the number of REQ_DISCARD requests sent
+		by bio. The former ones are sent to a swap block device when a
+		swap slot is freed, which implies that this disk is being used
+		as a swap disk. The latter ones are sent by filesystem mounted
+		with discard option, whenever some data blocks are getting
+		discarded.
 
 What:		/sys/block/zram<id>/zero_pages
 Date:		August 2010
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index d00831c..c2e7127 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -344,6 +344,7 @@ static void zram_free_page(struct zram *zram, size_t index)
 	atomic64_sub(zram_get_obj_size(meta, index),
 			&zram->stats.compr_data_size);
 	atomic64_dec(&zram->stats.pages_stored);
+	atomic64_inc(&zram->stats.notify_free);
 
 	meta->table[index].handle = 0;
 	zram_set_obj_size(meta, index, 0);
@@ -843,7 +844,6 @@ static void zram_slot_free_notify(struct block_device *bdev,
 	bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
 	zram_free_page(zram, index);
 	bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
-	atomic64_inc(&zram->stats.notify_free);
 }
 
 static const struct block_device_operations zram_devops = {
-- 
2.1.0.233.g9eef2c8

--
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] zram: add num_discards for discarded pages stat

[Minchan Kim]

Hi Sergey,

On Thu, Aug 21, 2014 at 10:05:04PM +0900, Sergey Senozhatsky wrote:
> On (08/21/14 17:09), Chao Yu wrote:
> [cut]
> > > 
> > > I hope I'm not discouraging. :)
> > 
> > Nope, please let me try again, :)
> > 
> > Since we have supported handling discard request in this commit
> > f4659d8e620d08bd1a84a8aec5d2f5294a242764 (zram: support REQ_DISCARD), zram got
> > one more chance to free unused memory whenever received discard request. But
> > without stating for discard request, there is no method for user to know whether
> > discard request has been handled by zram or how many blocks were discarded by
> > zram when user wants to know the effect of discard.
> > 
> > In this patch, we add num_discards to stat discarded pages, and export it to
> > sysfs for users.
> > 
> 
> In other words, here is my proposal:
> 
> -----8<-----8<-----
> 
> Subject: [PATCH] zram: use notify_free to account all free notifications
> 
> notify_free device attribute accounts the number of slot free notifications
> and internally represents the number of zram_free_page() calls. Slot free
> notifications are sent only when device is used as a swap device, hence
> notify_free is used only for swap devices. Since f4659d8e620d08 (zram:
> support REQ_DISCARD) ZRAM handles yet another one free notification (also
> via zram_free_page() call) -- REQ_DISCARD requests, which are sent by a
> filesystem, whenever some data blocks are discarded. However, there is no
> way to know the number of notifications in the latter case.
> 
> Use notify_free to account the number of pages freed in zram_free_page(),
> instead of accounting only swap_slot_free_notify() calls (each
> zram_slot_free_notify() call frees one page).
> 
> This means that depending on usage scenario notify_free represents:
>  a) the number of pages freed because of slot free notifications, which is
>    equal to the number of swap_slot_free_notify() calls, so there is no
>    behaviour change
> 
>  b) the number of pages freed because of REQ_DISCARD notifications

IMO, it would be better to separate it because zram-swap does both
free notify and discard but trigger timing is different so we could
measure which one is better if we want to replace swap_slot_free_notify
with discard. So I'd like to keep both and then we could remove
notify_free later once we prove discard alone is enough.

What do you think about it?

> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
>  Documentation/ABI/testing/sysfs-block-zram | 13 ++++++++-----
>  drivers/block/zram/zram_drv.c              |  2 +-
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-block-zram b/Documentation/ABI/testing/sysfs-block-zram
> index 70ec992..73ed400 100644
> --- a/Documentation/ABI/testing/sysfs-block-zram
> +++ b/Documentation/ABI/testing/sysfs-block-zram
> @@ -77,11 +77,14 @@ What:		/sys/block/zram<id>/notify_free
>  Date:		August 2010
>  Contact:	Nitin Gupta <ngupta@vflare.org>
>  Description:
> -		The notify_free file is read-only and specifies the number of
> -		swap slot free notifications received by this device. These
> -		notifications are sent to a swap block device when a swap slot
> -		is freed. This statistic is applicable only when this disk is
> -		being used as a swap disk.
> +		The notify_free file is read-only. Depending on device usage
> +		scenario it may account a) the number of swap slot free
> +		notifications or b) the number of REQ_DISCARD requests sent
> +		by bio. The former ones are sent to a swap block device when a
> +		swap slot is freed, which implies that this disk is being used
> +		as a swap disk. The latter ones are sent by filesystem mounted
> +		with discard option, whenever some data blocks are getting
> +		discarded.
>  
>  What:		/sys/block/zram<id>/zero_pages
>  Date:		August 2010
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index d00831c..c2e7127 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -344,6 +344,7 @@ static void zram_free_page(struct zram *zram, size_t index)
>  	atomic64_sub(zram_get_obj_size(meta, index),
>  			&zram->stats.compr_data_size);
>  	atomic64_dec(&zram->stats.pages_stored);
> +	atomic64_inc(&zram->stats.notify_free);
>  
>  	meta->table[index].handle = 0;
>  	zram_set_obj_size(meta, index, 0);
> @@ -843,7 +844,6 @@ static void zram_slot_free_notify(struct block_device *bdev,
>  	bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
>  	zram_free_page(zram, index);
>  	bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> -	atomic64_inc(&zram->stats.notify_free);
>  }
>  
>  static const struct block_device_operations zram_devops = {
> -- 
> 2.1.0.233.g9eef2c8
> 
> --
> 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>

-- 
Kind regards,
Minchan Kim

Re: [PATCH] zram: add num_discards for discarded pages stat

[Minchan Kim]

Hi Sergey,

On Thu, Aug 21, 2014 at 10:05:04PM +0900, Sergey Senozhatsky wrote:
> On (08/21/14 17:09), Chao Yu wrote:
> [cut]
> > > 
> > > I hope I'm not discouraging. :)
> > 
> > Nope, please let me try again, :)
> > 
> > Since we have supported handling discard request in this commit
> > f4659d8e620d08bd1a84a8aec5d2f5294a242764 (zram: support REQ_DISCARD), zram got
> > one more chance to free unused memory whenever received discard request. But
> > without stating for discard request, there is no method for user to know whether
> > discard request has been handled by zram or how many blocks were discarded by
> > zram when user wants to know the effect of discard.
> > 
> > In this patch, we add num_discards to stat discarded pages, and export it to
> > sysfs for users.
> > 
> 
> In other words, here is my proposal:
> 
> -----8<-----8<-----
> 
> Subject: [PATCH] zram: use notify_free to account all free notifications
> 
> notify_free device attribute accounts the number of slot free notifications
> and internally represents the number of zram_free_page() calls. Slot free
> notifications are sent only when device is used as a swap device, hence
> notify_free is used only for swap devices. Since f4659d8e620d08 (zram:
> support REQ_DISCARD) ZRAM handles yet another one free notification (also
> via zram_free_page() call) -- REQ_DISCARD requests, which are sent by a
> filesystem, whenever some data blocks are discarded. However, there is no
> way to know the number of notifications in the latter case.
> 
> Use notify_free to account the number of pages freed in zram_free_page(),
> instead of accounting only swap_slot_free_notify() calls (each
> zram_slot_free_notify() call frees one page).
> 
> This means that depending on usage scenario notify_free represents:
>  a) the number of pages freed because of slot free notifications, which is
>    equal to the number of swap_slot_free_notify() calls, so there is no
>    behaviour change
> 
>  b) the number of pages freed because of REQ_DISCARD notifications

IMO, it would be better to separate it because zram-swap does both
free notify and discard but trigger timing is different so we could
measure which one is better if we want to replace swap_slot_free_notify
with discard. So I'd like to keep both and then we could remove
notify_free later once we prove discard alone is enough.

What do you think about it?

> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
>  Documentation/ABI/testing/sysfs-block-zram | 13 ++++++++-----
>  drivers/block/zram/zram_drv.c              |  2 +-
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-block-zram b/Documentation/ABI/testing/sysfs-block-zram
> index 70ec992..73ed400 100644
> --- a/Documentation/ABI/testing/sysfs-block-zram
> +++ b/Documentation/ABI/testing/sysfs-block-zram
> @@ -77,11 +77,14 @@ What:		/sys/block/zram<id>/notify_free
>  Date:		August 2010
>  Contact:	Nitin Gupta <ngupta@vflare.org>
>  Description:
> -		The notify_free file is read-only and specifies the number of
> -		swap slot free notifications received by this device. These
> -		notifications are sent to a swap block device when a swap slot
> -		is freed. This statistic is applicable only when this disk is
> -		being used as a swap disk.
> +		The notify_free file is read-only. Depending on device usage
> +		scenario it may account a) the number of swap slot free
> +		notifications or b) the number of REQ_DISCARD requests sent
> +		by bio. The former ones are sent to a swap block device when a
> +		swap slot is freed, which implies that this disk is being used
> +		as a swap disk. The latter ones are sent by filesystem mounted
> +		with discard option, whenever some data blocks are getting
> +		discarded.
>  
>  What:		/sys/block/zram<id>/zero_pages
>  Date:		August 2010
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index d00831c..c2e7127 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -344,6 +344,7 @@ static void zram_free_page(struct zram *zram, size_t index)
>  	atomic64_sub(zram_get_obj_size(meta, index),
>  			&zram->stats.compr_data_size);
>  	atomic64_dec(&zram->stats.pages_stored);
> +	atomic64_inc(&zram->stats.notify_free);
>  
>  	meta->table[index].handle = 0;
>  	zram_set_obj_size(meta, index, 0);
> @@ -843,7 +844,6 @@ static void zram_slot_free_notify(struct block_device *bdev,
>  	bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
>  	zram_free_page(zram, index);
>  	bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> -	atomic64_inc(&zram->stats.notify_free);
>  }
>  
>  static const struct block_device_operations zram_devops = {
> -- 
> 2.1.0.233.g9eef2c8
> 
> --
> 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>

-- 
Kind regards,
Minchan Kim

--
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] zram: add num_discards for discarded pages stat

[Chao Yu]

Hi Sergey,

> -----Original Message-----
> From: owner-linux-mm@kvack.org [mailto:owner-linux-mm@kvack.org] On Behalf Of Sergey
> Senozhatsky
> Sent: Thursday, August 21, 2014 9:05 PM
> To: Chao Yu
> Cc: 'Minchan Kim'; 'Sergey Senozhatsky'; linux-kernel@vger.kernel.org; linux-mm@kvack.org;
> ngupta@vflare.org; 'Jerome Marchand'; 'Andrew Morton'
> Subject: Re: [PATCH] zram: add num_discards for discarded pages stat
> 
> On (08/21/14 17:09), Chao Yu wrote:
> [cut]
> > >
> > > I hope I'm not discouraging. :)
> >
> > Nope, please let me try again, :)
> >
> > Since we have supported handling discard request in this commit
> > f4659d8e620d08bd1a84a8aec5d2f5294a242764 (zram: support REQ_DISCARD), zram got
> > one more chance to free unused memory whenever received discard request. But
> > without stating for discard request, there is no method for user to know whether
> > discard request has been handled by zram or how many blocks were discarded by
> > zram when user wants to know the effect of discard.
> >
> > In this patch, we add num_discards to stat discarded pages, and export it to
> > sysfs for users.
> >
> 
> In other words, here is my proposal:
> 
> -----8<-----8<-----
> 
> Subject: [PATCH] zram: use notify_free to account all free notifications
> 
> notify_free device attribute accounts the number of slot free notifications
> and internally represents the number of zram_free_page() calls. Slot free
> notifications are sent only when device is used as a swap device, hence
> notify_free is used only for swap devices. Since f4659d8e620d08 (zram:
> support REQ_DISCARD) ZRAM handles yet another one free notification (also
> via zram_free_page() call) -- REQ_DISCARD requests, which are sent by a
> filesystem, whenever some data blocks are discarded. However, there is no
> way to know the number of notifications in the latter case.
> 
> Use notify_free to account the number of pages freed in zram_free_page(),
> instead of accounting only swap_slot_free_notify() calls (each
> zram_slot_free_notify() call frees one page).
> 
> This means that depending on usage scenario notify_free represents:
>  a) the number of pages freed because of slot free notifications, which is
>    equal to the number of swap_slot_free_notify() calls, so there is no
>    behaviour change

As I know, administrator can send discard request by using "swapon -d /dev/zram0"
So after then, notify_free may show the page number mixed with both result of
handling REQ_DISCARD and slot free notifications.

And as I check the code, there is a workqueue "swap_discard_work" inited
in sys_swapon, so maybe there are REQ_DISCARDs sent from background thread,
but not filesystem. Is this right?

Regards,
Yu

> 
>  b) the number of pages freed because of REQ_DISCARD notifications
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
>  Documentation/ABI/testing/sysfs-block-zram | 13 ++++++++-----
>  drivers/block/zram/zram_drv.c              |  2 +-
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-block-zram
> b/Documentation/ABI/testing/sysfs-block-zram
> index 70ec992..73ed400 100644
> --- a/Documentation/ABI/testing/sysfs-block-zram
> +++ b/Documentation/ABI/testing/sysfs-block-zram
> @@ -77,11 +77,14 @@ What:		/sys/block/zram<id>/notify_free
>  Date:		August 2010
>  Contact:	Nitin Gupta <ngupta@vflare.org>
>  Description:
> -		The notify_free file is read-only and specifies the number of
> -		swap slot free notifications received by this device. These
> -		notifications are sent to a swap block device when a swap slot
> -		is freed. This statistic is applicable only when this disk is
> -		being used as a swap disk.
> +		The notify_free file is read-only. Depending on device usage
> +		scenario it may account a) the number of swap slot free
> +		notifications or b) the number of REQ_DISCARD requests sent
> +		by bio. The former ones are sent to a swap block device when a
> +		swap slot is freed, which implies that this disk is being used
> +		as a swap disk. The latter ones are sent by filesystem mounted
> +		with discard option, whenever some data blocks are getting
> +		discarded.
> 
>  What:		/sys/block/zram<id>/zero_pages
>  Date:		August 2010
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index d00831c..c2e7127 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -344,6 +344,7 @@ static void zram_free_page(struct zram *zram, size_t index)
>  	atomic64_sub(zram_get_obj_size(meta, index),
>  			&zram->stats.compr_data_size);
>  	atomic64_dec(&zram->stats.pages_stored);
> +	atomic64_inc(&zram->stats.notify_free);
> 
>  	meta->table[index].handle = 0;
>  	zram_set_obj_size(meta, index, 0);
> @@ -843,7 +844,6 @@ static void zram_slot_free_notify(struct block_device *bdev,
>  	bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
>  	zram_free_page(zram, index);
>  	bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> -	atomic64_inc(&zram->stats.notify_free);
>  }
> 
>  static const struct block_device_operations zram_devops = {
> --
> 2.1.0.233.g9eef2c8
> 
> --
> 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] zram: add num_discards for discarded pages stat

[Chao Yu]

Hi Sergey,

> -----Original Message-----
> From: owner-linux-mm@kvack.org [mailto:owner-linux-mm@kvack.org] On Behalf Of Sergey
> Senozhatsky
> Sent: Thursday, August 21, 2014 9:05 PM
> To: Chao Yu
> Cc: 'Minchan Kim'; 'Sergey Senozhatsky'; linux-kernel@vger.kernel.org; linux-mm@kvack.org;
> ngupta@vflare.org; 'Jerome Marchand'; 'Andrew Morton'
> Subject: Re: [PATCH] zram: add num_discards for discarded pages stat
> 
> On (08/21/14 17:09), Chao Yu wrote:
> [cut]
> > >
> > > I hope I'm not discouraging. :)
> >
> > Nope, please let me try again, :)
> >
> > Since we have supported handling discard request in this commit
> > f4659d8e620d08bd1a84a8aec5d2f5294a242764 (zram: support REQ_DISCARD), zram got
> > one more chance to free unused memory whenever received discard request. But
> > without stating for discard request, there is no method for user to know whether
> > discard request has been handled by zram or how many blocks were discarded by
> > zram when user wants to know the effect of discard.
> >
> > In this patch, we add num_discards to stat discarded pages, and export it to
> > sysfs for users.
> >
> 
> In other words, here is my proposal:
> 
> -----8<-----8<-----
> 
> Subject: [PATCH] zram: use notify_free to account all free notifications
> 
> notify_free device attribute accounts the number of slot free notifications
> and internally represents the number of zram_free_page() calls. Slot free
> notifications are sent only when device is used as a swap device, hence
> notify_free is used only for swap devices. Since f4659d8e620d08 (zram:
> support REQ_DISCARD) ZRAM handles yet another one free notification (also
> via zram_free_page() call) -- REQ_DISCARD requests, which are sent by a
> filesystem, whenever some data blocks are discarded. However, there is no
> way to know the number of notifications in the latter case.
> 
> Use notify_free to account the number of pages freed in zram_free_page(),
> instead of accounting only swap_slot_free_notify() calls (each
> zram_slot_free_notify() call frees one page).
> 
> This means that depending on usage scenario notify_free represents:
>  a) the number of pages freed because of slot free notifications, which is
>    equal to the number of swap_slot_free_notify() calls, so there is no
>    behaviour change

As I know, administrator can send discard request by using "swapon -d /dev/zram0"
So after then, notify_free may show the page number mixed with both result of
handling REQ_DISCARD and slot free notifications.

And as I check the code, there is a workqueue "swap_discard_work" inited
in sys_swapon, so maybe there are REQ_DISCARDs sent from background thread,
but not filesystem. Is this right?

Regards,
Yu

> 
>  b) the number of pages freed because of REQ_DISCARD notifications
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
>  Documentation/ABI/testing/sysfs-block-zram | 13 ++++++++-----
>  drivers/block/zram/zram_drv.c              |  2 +-
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-block-zram
> b/Documentation/ABI/testing/sysfs-block-zram
> index 70ec992..73ed400 100644
> --- a/Documentation/ABI/testing/sysfs-block-zram
> +++ b/Documentation/ABI/testing/sysfs-block-zram
> @@ -77,11 +77,14 @@ What:		/sys/block/zram<id>/notify_free
>  Date:		August 2010
>  Contact:	Nitin Gupta <ngupta@vflare.org>
>  Description:
> -		The notify_free file is read-only and specifies the number of
> -		swap slot free notifications received by this device. These
> -		notifications are sent to a swap block device when a swap slot
> -		is freed. This statistic is applicable only when this disk is
> -		being used as a swap disk.
> +		The notify_free file is read-only. Depending on device usage
> +		scenario it may account a) the number of swap slot free
> +		notifications or b) the number of REQ_DISCARD requests sent
> +		by bio. The former ones are sent to a swap block device when a
> +		swap slot is freed, which implies that this disk is being used
> +		as a swap disk. The latter ones are sent by filesystem mounted
> +		with discard option, whenever some data blocks are getting
> +		discarded.
> 
>  What:		/sys/block/zram<id>/zero_pages
>  Date:		August 2010
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index d00831c..c2e7127 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -344,6 +344,7 @@ static void zram_free_page(struct zram *zram, size_t index)
>  	atomic64_sub(zram_get_obj_size(meta, index),
>  			&zram->stats.compr_data_size);
>  	atomic64_dec(&zram->stats.pages_stored);
> +	atomic64_inc(&zram->stats.notify_free);
> 
>  	meta->table[index].handle = 0;
>  	zram_set_obj_size(meta, index, 0);
> @@ -843,7 +844,6 @@ static void zram_slot_free_notify(struct block_device *bdev,
>  	bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
>  	zram_free_page(zram, index);
>  	bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> -	atomic64_inc(&zram->stats.notify_free);
>  }
> 
>  static const struct block_device_operations zram_devops = {
> --
> 2.1.0.233.g9eef2c8
> 
> --
> 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>

--
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] zram: add num_discards for discarded pages stat

[Sergey Senozhatsky]

Hello,

On (08/26/14 10:43), Chao Yu wrote:
> Hi Sergey,
> > > Nope, please let me try again, :)
> > >
> > > Since we have supported handling discard request in this commit
> > > f4659d8e620d08bd1a84a8aec5d2f5294a242764 (zram: support REQ_DISCARD), zram got
> > > one more chance to free unused memory whenever received discard request. But
> > > without stating for discard request, there is no method for user to know whether
> > > discard request has been handled by zram or how many blocks were discarded by
> > > zram when user wants to know the effect of discard.
> > >
> > > In this patch, we add num_discards to stat discarded pages, and export it to
> > > sysfs for users.
> > >
> > 
> > In other words, here is my proposal:
> > 
> > -----8<-----8<-----
> > 
> > Subject: [PATCH] zram: use notify_free to account all free notifications
> > 
> > notify_free device attribute accounts the number of slot free notifications
> > and internally represents the number of zram_free_page() calls. Slot free
> > notifications are sent only when device is used as a swap device, hence
> > notify_free is used only for swap devices. Since f4659d8e620d08 (zram:
> > support REQ_DISCARD) ZRAM handles yet another one free notification (also
> > via zram_free_page() call) -- REQ_DISCARD requests, which are sent by a
> > filesystem, whenever some data blocks are discarded. However, there is no
> > way to know the number of notifications in the latter case.
> > 
> > Use notify_free to account the number of pages freed in zram_free_page(),
> > instead of accounting only swap_slot_free_notify() calls (each
> > zram_slot_free_notify() call frees one page).
> > 
> > This means that depending on usage scenario notify_free represents:
> >  a) the number of pages freed because of slot free notifications, which is
> >    equal to the number of swap_slot_free_notify() calls, so there is no
> >    behaviour change
> 
> As I know, administrator can send discard request by using "swapon -d /dev/zram0"
> So after then, notify_free may show the page number mixed with both result of
> handling REQ_DISCARD and slot free notifications.
> 
> And as I check the code, there is a workqueue "swap_discard_work" inited
> in sys_swapon, so maybe there are REQ_DISCARDs sent from background thread,
> but not filesystem. Is this right?

yes, correct. I didn't know that at that time. as Minchan noted, swap
device may receive both slot free and REQ_DISCARD.

	-ss

> Regards,
> Yu
> 
> > 
> >  b) the number of pages freed because of REQ_DISCARD notifications
> > 
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > ---
> >  Documentation/ABI/testing/sysfs-block-zram | 13 ++++++++-----
> >  drivers/block/zram/zram_drv.c              |  2 +-
> >  2 files changed, 9 insertions(+), 6 deletions(-)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-block-zram
> > b/Documentation/ABI/testing/sysfs-block-zram
> > index 70ec992..73ed400 100644
> > --- a/Documentation/ABI/testing/sysfs-block-zram
> > +++ b/Documentation/ABI/testing/sysfs-block-zram
> > @@ -77,11 +77,14 @@ What:		/sys/block/zram<id>/notify_free
> >  Date:		August 2010
> >  Contact:	Nitin Gupta <ngupta@vflare.org>
> >  Description:
> > -		The notify_free file is read-only and specifies the number of
> > -		swap slot free notifications received by this device. These
> > -		notifications are sent to a swap block device when a swap slot
> > -		is freed. This statistic is applicable only when this disk is
> > -		being used as a swap disk.
> > +		The notify_free file is read-only. Depending on device usage
> > +		scenario it may account a) the number of swap slot free
> > +		notifications or b) the number of REQ_DISCARD requests sent
> > +		by bio. The former ones are sent to a swap block device when a
> > +		swap slot is freed, which implies that this disk is being used
> > +		as a swap disk. The latter ones are sent by filesystem mounted
> > +		with discard option, whenever some data blocks are getting
> > +		discarded.
> > 
> >  What:		/sys/block/zram<id>/zero_pages
> >  Date:		August 2010
> > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > index d00831c..c2e7127 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -344,6 +344,7 @@ static void zram_free_page(struct zram *zram, size_t index)
> >  	atomic64_sub(zram_get_obj_size(meta, index),
> >  			&zram->stats.compr_data_size);
> >  	atomic64_dec(&zram->stats.pages_stored);
> > +	atomic64_inc(&zram->stats.notify_free);
> > 
> >  	meta->table[index].handle = 0;
> >  	zram_set_obj_size(meta, index, 0);
> > @@ -843,7 +844,6 @@ static void zram_slot_free_notify(struct block_device *bdev,
> >  	bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> >  	zram_free_page(zram, index);
> >  	bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> > -	atomic64_inc(&zram->stats.notify_free);
> >  }
> > 
> >  static const struct block_device_operations zram_devops = {
> > --
> > 2.1.0.233.g9eef2c8
> > 
> > --
> > 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] zram: add num_discards for discarded pages stat

[Sergey Senozhatsky]

Hello,

On (08/26/14 10:43), Chao Yu wrote:
> Hi Sergey,
> > > Nope, please let me try again, :)
> > >
> > > Since we have supported handling discard request in this commit
> > > f4659d8e620d08bd1a84a8aec5d2f5294a242764 (zram: support REQ_DISCARD), zram got
> > > one more chance to free unused memory whenever received discard request. But
> > > without stating for discard request, there is no method for user to know whether
> > > discard request has been handled by zram or how many blocks were discarded by
> > > zram when user wants to know the effect of discard.
> > >
> > > In this patch, we add num_discards to stat discarded pages, and export it to
> > > sysfs for users.
> > >
> > 
> > In other words, here is my proposal:
> > 
> > -----8<-----8<-----
> > 
> > Subject: [PATCH] zram: use notify_free to account all free notifications
> > 
> > notify_free device attribute accounts the number of slot free notifications
> > and internally represents the number of zram_free_page() calls. Slot free
> > notifications are sent only when device is used as a swap device, hence
> > notify_free is used only for swap devices. Since f4659d8e620d08 (zram:
> > support REQ_DISCARD) ZRAM handles yet another one free notification (also
> > via zram_free_page() call) -- REQ_DISCARD requests, which are sent by a
> > filesystem, whenever some data blocks are discarded. However, there is no
> > way to know the number of notifications in the latter case.
> > 
> > Use notify_free to account the number of pages freed in zram_free_page(),
> > instead of accounting only swap_slot_free_notify() calls (each
> > zram_slot_free_notify() call frees one page).
> > 
> > This means that depending on usage scenario notify_free represents:
> >  a) the number of pages freed because of slot free notifications, which is
> >    equal to the number of swap_slot_free_notify() calls, so there is no
> >    behaviour change
> 
> As I know, administrator can send discard request by using "swapon -d /dev/zram0"
> So after then, notify_free may show the page number mixed with both result of
> handling REQ_DISCARD and slot free notifications.
> 
> And as I check the code, there is a workqueue "swap_discard_work" inited
> in sys_swapon, so maybe there are REQ_DISCARDs sent from background thread,
> but not filesystem. Is this right?

yes, correct. I didn't know that at that time. as Minchan noted, swap
device may receive both slot free and REQ_DISCARD.

	-ss

> Regards,
> Yu
> 
> > 
> >  b) the number of pages freed because of REQ_DISCARD notifications
> > 
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > ---
> >  Documentation/ABI/testing/sysfs-block-zram | 13 ++++++++-----
> >  drivers/block/zram/zram_drv.c              |  2 +-
> >  2 files changed, 9 insertions(+), 6 deletions(-)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-block-zram
> > b/Documentation/ABI/testing/sysfs-block-zram
> > index 70ec992..73ed400 100644
> > --- a/Documentation/ABI/testing/sysfs-block-zram
> > +++ b/Documentation/ABI/testing/sysfs-block-zram
> > @@ -77,11 +77,14 @@ What:		/sys/block/zram<id>/notify_free
> >  Date:		August 2010
> >  Contact:	Nitin Gupta <ngupta@vflare.org>
> >  Description:
> > -		The notify_free file is read-only and specifies the number of
> > -		swap slot free notifications received by this device. These
> > -		notifications are sent to a swap block device when a swap slot
> > -		is freed. This statistic is applicable only when this disk is
> > -		being used as a swap disk.
> > +		The notify_free file is read-only. Depending on device usage
> > +		scenario it may account a) the number of swap slot free
> > +		notifications or b) the number of REQ_DISCARD requests sent
> > +		by bio. The former ones are sent to a swap block device when a
> > +		swap slot is freed, which implies that this disk is being used
> > +		as a swap disk. The latter ones are sent by filesystem mounted
> > +		with discard option, whenever some data blocks are getting
> > +		discarded.
> > 
> >  What:		/sys/block/zram<id>/zero_pages
> >  Date:		August 2010
> > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > index d00831c..c2e7127 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -344,6 +344,7 @@ static void zram_free_page(struct zram *zram, size_t index)
> >  	atomic64_sub(zram_get_obj_size(meta, index),
> >  			&zram->stats.compr_data_size);
> >  	atomic64_dec(&zram->stats.pages_stored);
> > +	atomic64_inc(&zram->stats.notify_free);
> > 
> >  	meta->table[index].handle = 0;
> >  	zram_set_obj_size(meta, index, 0);
> > @@ -843,7 +844,6 @@ static void zram_slot_free_notify(struct block_device *bdev,
> >  	bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> >  	zram_free_page(zram, index);
> >  	bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> > -	atomic64_inc(&zram->stats.notify_free);
> >  }
> > 
> >  static const struct block_device_operations zram_devops = {
> > --
> > 2.1.0.233.g9eef2c8
> > 
> > --
> > 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>
> 

--
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] zram: add num_discards for discarded pages stat

[Minchan Kim]

Hello Chao,

On Thu, Aug 21, 2014 at 05:09:19PM +0800, Chao Yu wrote:
> Hi Minchan,
> 
> > -----Original Message-----
> > From: Minchan Kim [mailto:minchan@kernel.org]
> > Sent: Thursday, August 21, 2014 9:19 AM
> > To: Chao Yu
> > Cc: 'Sergey Senozhatsky'; linux-kernel@vger.kernel.org; linux-mm@kvack.org; ngupta@vflare.org;
> > 'Jerome Marchand'; 'Andrew Morton'
> > Subject: Re: [PATCH] zram: add num_discards for discarded pages stat
> > 
> > Hi Chao,
> > 
> > On Wed, Aug 20, 2014 at 04:20:48PM +0800, Chao Yu wrote:
> > > Hi Minchan,
> > >
> > > > -----Original Message-----
> > > > From: Minchan Kim [mailto:minchan@kernel.org]
> > > > Sent: Wednesday, August 20, 2014 10:09 AM
> > > > To: Sergey Senozhatsky
> > > > Cc: Chao Yu; linux-kernel@vger.kernel.org; linux-mm@kvack.org; ngupta@vflare.org; 'Jerome
> > > > Marchand'; 'Andrew Morton'
> > > > Subject: Re: [PATCH] zram: add num_discards for discarded pages stat
> > > >
> > > > Hi Sergey,
> > > >
> > > > On Tue, Aug 19, 2014 at 08:25:00PM +0900, Sergey Senozhatsky wrote:
> > > > > Hello,
> > > > >
> > > > > On (08/19/14 13:45), Chao Yu wrote:
> > > > > > > On (08/15/14 11:27), Chao Yu wrote:
> > > > > > > > Now we have supported handling discard request which is sended by filesystem,
> > > > > > > > but no interface could be used to show information of discard.
> > > > > > > > This patch adds num_discards to stat discarded pages, then export it to sysfs
> > > > > > > > for displaying.
> > > > > > > >
> > > > > > >
> > > > > > > a side question: we account discarded pages via slot free notify in
> > > > > > > notify_free and via req_discard in num_discards. how about accounting
> > > > > > > both of them in num_discards? because, after all, they account a number
> > > > > > > of discarded pages (zram_free_page()). or there any particular reason we
> > > > > > > want to distinguish.
> > > > > >
> > > > > > Yeah, I agree with you as I have no such reason unless there are our users'
> > > > > > explicitly requirement for showing notify_free/num_discards separately later.
> > > > > >
> > > > > > How do you think of sending another patch to merge these two counts?
> > > > > >
> > > > >
> > > > > Minchan, what do you think? let's account discarded pages in one place.
> > > >
> > > > First of all, I'd like to know why we need num_discards.
> > > > It should be in description and depends on it whether we should merge both
> > > > counts or separate.
> > >
> > > Oh, it's my mistaken.
> > >
> > > When commit 	9b9913d80b2896ecd9e0a1a8f167ccad66fac79c (Staging: zram: Update
> > > zram documentation) and commit e98419c23b1a189c932775f7833e94cb5230a16b (Staging:
> > > zram: Document sysfs entries) description related to 'discard' stat was designed
> > > and added to zram.txt and sysfs-block-zram, but without implementation of function
> > > for handling discard request, description in documents were removed in commit
> > > 8dd1d3247e6c00b50ef83934ea8b22a1590015de (zram: document failed_reads,
> > > failed_writes stats)
> > 
> > Thanks for letting me know the history.
> > 
> > >
> > > For now, we have already supported discard handling, so it's better to resume
> > > the stat of discard number, this discard stat supports user one more kind of runtime
> > > information of zram as other stats supported.
> > >
> > > How do you think?
> > 
> > I'm not strong against the idea but just "resume is better" and
> > "one more is problem as other stats supported" is not logical
> > to me.
> 
> OK, I assume maybe to match the principle for adopting and discarding
> those stats in original version of zram will do some help, actually I'm wrong.
> 
> > 
> > You should explain why we need such new stat so that user can take
> > what kinds of benefit from that. Otherwise, we couldn't know the stat
> > is best or not for the goal.
> 
> Alright, it's reasonable from this perspective.
> 
> > 
> > 
> > I might be paranoid about small stuff and I admit I'm not good for it,
> > too but pz, understand that adding the new feature requires a
> > good description which should include clear goal.
> 
> Well, I can understand and accept that.
> 
> > 
> > I hope I'm not discouraging. :)
> 
> Nope, please let me try again, :)
> 
> Since we have supported handling discard request in this commit
> f4659d8e620d08bd1a84a8aec5d2f5294a242764 (zram: support REQ_DISCARD), zram got
> one more chance to free unused memory whenever received discard request. But
> without stating for discard request, there is no method for user to know whether
> discard request has been handled by zram or how many blocks were discarded by
> zram when user wants to know the effect of discard.
> 
> In this patch, we add num_discards to stat discarded pages, and export it to
> sysfs for users.

Yeb, Thanks!

>-- 
Kind regards,
Minchan Kim

Re: [PATCH] zram: add num_discards for discarded pages stat

[Minchan Kim]

Hello Chao,

On Thu, Aug 21, 2014 at 05:09:19PM +0800, Chao Yu wrote:
> Hi Minchan,
> 
> > -----Original Message-----
> > From: Minchan Kim [mailto:minchan@kernel.org]
> > Sent: Thursday, August 21, 2014 9:19 AM
> > To: Chao Yu
> > Cc: 'Sergey Senozhatsky'; linux-kernel@vger.kernel.org; linux-mm@kvack.org; ngupta@vflare.org;
> > 'Jerome Marchand'; 'Andrew Morton'
> > Subject: Re: [PATCH] zram: add num_discards for discarded pages stat
> > 
> > Hi Chao,
> > 
> > On Wed, Aug 20, 2014 at 04:20:48PM +0800, Chao Yu wrote:
> > > Hi Minchan,
> > >
> > > > -----Original Message-----
> > > > From: Minchan Kim [mailto:minchan@kernel.org]
> > > > Sent: Wednesday, August 20, 2014 10:09 AM
> > > > To: Sergey Senozhatsky
> > > > Cc: Chao Yu; linux-kernel@vger.kernel.org; linux-mm@kvack.org; ngupta@vflare.org; 'Jerome
> > > > Marchand'; 'Andrew Morton'
> > > > Subject: Re: [PATCH] zram: add num_discards for discarded pages stat
> > > >
> > > > Hi Sergey,
> > > >
> > > > On Tue, Aug 19, 2014 at 08:25:00PM +0900, Sergey Senozhatsky wrote:
> > > > > Hello,
> > > > >
> > > > > On (08/19/14 13:45), Chao Yu wrote:
> > > > > > > On (08/15/14 11:27), Chao Yu wrote:
> > > > > > > > Now we have supported handling discard request which is sended by filesystem,
> > > > > > > > but no interface could be used to show information of discard.
> > > > > > > > This patch adds num_discards to stat discarded pages, then export it to sysfs
> > > > > > > > for displaying.
> > > > > > > >
> > > > > > >
> > > > > > > a side question: we account discarded pages via slot free notify in
> > > > > > > notify_free and via req_discard in num_discards. how about accounting
> > > > > > > both of them in num_discards? because, after all, they account a number
> > > > > > > of discarded pages (zram_free_page()). or there any particular reason we
> > > > > > > want to distinguish.
> > > > > >
> > > > > > Yeah, I agree with you as I have no such reason unless there are our users'
> > > > > > explicitly requirement for showing notify_free/num_discards separately later.
> > > > > >
> > > > > > How do you think of sending another patch to merge these two counts?
> > > > > >
> > > > >
> > > > > Minchan, what do you think? let's account discarded pages in one place.
> > > >
> > > > First of all, I'd like to know why we need num_discards.
> > > > It should be in description and depends on it whether we should merge both
> > > > counts or separate.
> > >
> > > Oh, it's my mistaken.
> > >
> > > When commit 	9b9913d80b2896ecd9e0a1a8f167ccad66fac79c (Staging: zram: Update
> > > zram documentation) and commit e98419c23b1a189c932775f7833e94cb5230a16b (Staging:
> > > zram: Document sysfs entries) description related to 'discard' stat was designed
> > > and added to zram.txt and sysfs-block-zram, but without implementation of function
> > > for handling discard request, description in documents were removed in commit
> > > 8dd1d3247e6c00b50ef83934ea8b22a1590015de (zram: document failed_reads,
> > > failed_writes stats)
> > 
> > Thanks for letting me know the history.
> > 
> > >
> > > For now, we have already supported discard handling, so it's better to resume
> > > the stat of discard number, this discard stat supports user one more kind of runtime
> > > information of zram as other stats supported.
> > >
> > > How do you think?
> > 
> > I'm not strong against the idea but just "resume is better" and
> > "one more is problem as other stats supported" is not logical
> > to me.
> 
> OK, I assume maybe to match the principle for adopting and discarding
> those stats in original version of zram will do some help, actually I'm wrong.
> 
> > 
> > You should explain why we need such new stat so that user can take
> > what kinds of benefit from that. Otherwise, we couldn't know the stat
> > is best or not for the goal.
> 
> Alright, it's reasonable from this perspective.
> 
> > 
> > 
> > I might be paranoid about small stuff and I admit I'm not good for it,
> > too but pz, understand that adding the new feature requires a
> > good description which should include clear goal.
> 
> Well, I can understand and accept that.
> 
> > 
> > I hope I'm not discouraging. :)
> 
> Nope, please let me try again, :)
> 
> Since we have supported handling discard request in this commit
> f4659d8e620d08bd1a84a8aec5d2f5294a242764 (zram: support REQ_DISCARD), zram got
> one more chance to free unused memory whenever received discard request. But
> without stating for discard request, there is no method for user to know whether
> discard request has been handled by zram or how many blocks were discarded by
> zram when user wants to know the effect of discard.
> 
> In this patch, we add num_discards to stat discarded pages, and export it to
> sysfs for users.

Yeb, Thanks!

>-- 
Kind regards,
Minchan Kim

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