Message-ID: <20150310053752.GA6330@swordfish>
Date:	Tue, 10 Mar 2015 14:37:52 +0900
From:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:	Minchan Kim <minchan@...nel.org>
Cc:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
	akpm@...ux-foundation.org, ddstreet@...e.org, gunho.lee@....com,
	iamjoonsoo.kim@....com, jmarchan@...hat.com, juno.choi@....com,
	mel@....ul.ie, ngupta@...are.org, semenzato@...gle.com,
	sergey.senozhatsky@...il.com, sjennings@...iantweb.net,
	mm-commits@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: + zram-support-compaction.patch added to -mm tree

On (03/09/15 23:56), Minchan Kim wrote:
> > in zram_slot_free_notify() and zram_rw_page() we don't have request queue, request,
> > etc. so it's a bit troubling.
> 
> I skim the code so I might miss something.
> 
> zram_slot_free_notify is just to free allocated space on zsmalloc so
> it's not related to I/O operation so it would be okay if we handle
> make_request and rw_page. Fortunately, they share core function
> called by zram_bvec_rw.  So could we use generic_[start|end]_io_acct
> in there? It seems we don't need request queue.
> 

that will do the trick, I think. thanks. I found these two late last
night.

> > 
> > Name            units         description
> > ----            -----         -----------
> > read I/Os       requests      number of read I/Os processed
> > read merges     requests      number of read I/Os merged with in-queue I/O
> > read sectors    sectors       number of sectors read
> > read ticks      milliseconds  total wait time for read requests
> > write I/Os      requests      number of write I/Os processed
> > write merges    requests      number of write I/Os merged with in-queue I/O
> > write sectors   sectors       number of sectors written
> > write ticks     milliseconds  total wait time for write requests
> > in_flight       requests      number of I/Os currently in flight
> > io_ticks        milliseconds  total time this block device has been active
> > time_in_queue   milliseconds  total wait time for all requests
> > 
> > 
> > the only overlaps are num_read and num_write. so we will not be able to move all
> 
> When I read above, read/write ticks would be useful to us.

yes. somehow I didn't manage to shape my thoughts, I was going to say that this
stat file is surely interesting on his own; and was about to let num_reads and
num_writes to sit in both zram<id>/stat and zram<id>/io_stat files.

> > (or any significant amount) of our IO stats to that file. that will force users
> > to gather IO stats accross several files.
> 
> I'm not saying let's move all of I/O related stuff.
> What I want is to remove duplicated stat if it is and enable zram/stats
> so I hope we could use iostat/nmon to monitor zram I/O.

ok. I did some overlapping (as I mentioned above) -- num_reads and num_writes
present in both ./stat and ./io_stat files. will remove them.
so we end up having:
-- block layer stats in zram<id>/stat
-- zram internal IO stats in zram<id>/io_stat   (no num_reads, no num_writes)
-- zram mm stats in zram<id>/mm_stat   (orig size, compressed size, num_migrated, etc.)

> > 
> > I'll take a look later today/tomorrow if I can do anything about it, but it seems
> > that our own zramX/io_stat file would be simpler solution here. it does sound ugly,
> > but it doesn't look so bad after all.
> 
> If it is really impossible or makes kernel complicated, I will agree with you.
> Otherwise, I really want to see zram in iostat. :)

yes, that's the goal. I found our previous discussion on the topic:
https://lkml.org/lkml/2014/9/4/368

6 months later we are finally on it :)  will send the patches later today.

thanks,

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

Powered by blists - more mailing lists