diff mbox

[04/19] bcache: fix wrong cache_misses statistics

Message ID 1498855388-16990-4-git-send-email-bcache@lists.ewheeler.net (mailing list archive)
State New, archived
Headers show

Commit Message

Eric Wheeler June 30, 2017, 8:42 p.m. UTC
From: Tang Junhui <tang.junhui@zte.com.cn>

Some missed IOs are not counted into cache_misses, this patch fix this
issue.

Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
Reviewed-by: Eric Wheeler <bcache@linux.ewheeler.net>
Cc: stable@vger.kernel.org
---
 drivers/md/bcache/request.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Coly Li July 1, 2017, 5:58 p.m. UTC | #1
On 2017/7/1 上午4:42, bcache@lists.ewheeler.net wrote:
> From: Tang Junhui <tang.junhui@zte.com.cn>
> 
> Some missed IOs are not counted into cache_misses, this patch fix this
> issue.

Could you please explain more about,
- which kind of missed I/O are mot counted
- where cache_missed is located

This will help the patch to be more understandable.

> 
> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> Reviewed-by: Eric Wheeler <bcache@linux.ewheeler.net>
> Cc: stable@vger.kernel.org

[snip]

> @@ -758,7 +760,7 @@ static void cached_dev_read_done_bh(struct closure *cl)
>  	struct cached_dev *dc = container_of(s->d, struct cached_dev, disk);
>  
>  	bch_mark_cache_accounting(s->iop.c, s->d,
> -				  !s->cache_miss, s->iop.bypass);
> +				  !s->cache_missed, s->iop.bypass);
>  	trace_bcache_read(s->orig_bio, !s->cache_miss, s->iop.bypass);


Should the above line be changed to,
	trace_bcache_read(s->orig_bio, !s->cache_missed, s->iop.bypass);
as well ?


[snip]

Thanks.
Eric Wheeler July 13, 2017, 4:09 a.m. UTC | #2
On Sun, 2 Jul 2017, Coly Li wrote:

> On 2017/7/1 上午4:42, bcache@lists.ewheeler.net wrote:
> > From: Tang Junhui <tang.junhui@zte.com.cn>
> > 
> > Some missed IOs are not counted into cache_misses, this patch fix this
> > issue.
> 
> Could you please explain more about,
> - which kind of missed I/O are mot counted
> - where cache_missed is located
> 
> This will help the patch to be more understandable.

Hi Tang,

I'm waiting to queue this patch pending your response to Coly.  Can you 
update the message send a v2?

--
Eric Wheeler



> 
> > 
> > Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> > Reviewed-by: Eric Wheeler <bcache@linux.ewheeler.net>
> > Cc: stable@vger.kernel.org
> 
> [snip]
> 
> > @@ -758,7 +760,7 @@ static void cached_dev_read_done_bh(struct closure *cl)
> >  	struct cached_dev *dc = container_of(s->d, struct cached_dev, disk);
> >  
> >  	bch_mark_cache_accounting(s->iop.c, s->d,
> > -				  !s->cache_miss, s->iop.bypass);
> > +				  !s->cache_missed, s->iop.bypass);
> >  	trace_bcache_read(s->orig_bio, !s->cache_miss, s->iop.bypass);
> 
> 
> Should the above line be changed to,
> 	trace_bcache_read(s->orig_bio, !s->cache_missed, s->iop.bypass);
> as well ?
> 
> 
> [snip]
> 
> Thanks.
> 
> -- 
> Coly Li
>
Eric Wheeler Oct. 27, 2017, 7:14 p.m. UTC | #3
On Thu, 13 Jul 2017, Eric Wheeler wrote:

> On Sun, 2 Jul 2017, Coly Li wrote:
> 
> > On 2017/7/1 上午4:42, bcache@lists.ewheeler.net wrote:
> > > From: Tang Junhui <tang.junhui@zte.com.cn>
> > > 
> > > Some missed IOs are not counted into cache_misses, this patch fix this
> > > issue.
> > 
> > Could you please explain more about,
> > - which kind of missed I/O are mot counted
> > - where cache_missed is located
> > 
> > This will help the patch to be more understandable.
> 
> Hi Tang,
> 
> I'm waiting to queue this patch pending your response to Coly.  Can you 
> update the message send a v2?

Hi Tang,

Can you to an update message and send this in so we can get the cache miss 
metrics corrected?
 
--
Eric Wheeler

> 
> 
> 
> > 
> > > 
> > > Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> > > Reviewed-by: Eric Wheeler <bcache@linux.ewheeler.net>
> > > Cc: stable@vger.kernel.org
> > 
> > [snip]
> > 
> > > @@ -758,7 +760,7 @@ static void cached_dev_read_done_bh(struct closure *cl)
> > >  	struct cached_dev *dc = container_of(s->d, struct cached_dev, disk);
> > >  
> > >  	bch_mark_cache_accounting(s->iop.c, s->d,
> > > -				  !s->cache_miss, s->iop.bypass);
> > > +				  !s->cache_missed, s->iop.bypass);
> > >  	trace_bcache_read(s->orig_bio, !s->cache_miss, s->iop.bypass);
> > 
> > 
> > Should the above line be changed to,
> > 	trace_bcache_read(s->orig_bio, !s->cache_missed, s->iop.bypass);
> > as well ?
> > 
> > 
> > [snip]
> > 
> > Thanks.
> > 
> > -- 
> > Coly Li
> >
diff mbox

Patch

diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
index 4b413db..d27707d 100644
--- a/drivers/md/bcache/request.c
+++ b/drivers/md/bcache/request.c
@@ -462,6 +462,7 @@  struct search {
 	unsigned		recoverable:1;
 	unsigned		write:1;
 	unsigned		read_dirty_data:1;
+	unsigned		cache_missed:1;
 
 	unsigned long		start_time;
 
@@ -647,6 +648,7 @@  static inline struct search *search_alloc(struct bio *bio,
 
 	s->orig_bio		= bio;
 	s->cache_miss		= NULL;
+	s->cache_missed		= 0;
 	s->d			= d;
 	s->recoverable		= 1;
 	s->write		= op_is_write(bio_op(bio));
@@ -758,7 +760,7 @@  static void cached_dev_read_done_bh(struct closure *cl)
 	struct cached_dev *dc = container_of(s->d, struct cached_dev, disk);
 
 	bch_mark_cache_accounting(s->iop.c, s->d,
-				  !s->cache_miss, s->iop.bypass);
+				  !s->cache_missed, s->iop.bypass);
 	trace_bcache_read(s->orig_bio, !s->cache_miss, s->iop.bypass);
 
 	if (s->iop.status)
@@ -777,6 +779,8 @@  static int cached_dev_cache_miss(struct btree *b, struct search *s,
 	struct cached_dev *dc = container_of(s->d, struct cached_dev, disk);
 	struct bio *miss, *cache_bio;
 
+	s->cache_missed = 1; /* true */
+
 	if (s->cache_miss || s->iop.bypass) {
 		miss = bio_next_split(bio, sectors, GFP_NOIO, s->d->bio_split);
 		ret = miss == bio ? MAP_DONE : MAP_CONTINUE;