Hello,
On Mon, Mar 09, 2015 at 03:48:56PM +0900, Sergey Senozhatsky wrote:
> On (03/09/15 11:21), Minchan Kim wrote:
> > > I was thinking for some time already about splitting stats that we
> > > export in two categories and, thus, two files: IO_stats and MM_stats.
> > >
> > > zram<id>/io_stat
> > >
> > > s*printf( num_reads, num_writes, failed_reads, failed_writes, etc.)
> >
> > Some of it(ie, num_reads, num_writes) was duplicated with
> > /dev/block/zramx/stat?
> > I know /dev/block/zramx/stat doesn't work now and I didn't check why it
> > doesn't
> > work but I hope we make it work so remove duplicate stat, finally. :)
> >
>
> yes, I do recall looking into the issue some months ago. zramX/stat file
> hanled by block
> layer in various places. for example, in:
>
> blk_finish_request(struct request *req, int error)
> blk_account_io_done():
>
> doing
>
> part_stat_inc(cpu, part, ios[rw]);
> part_stat_add(cpu, part, ticks[rw], duration);
> part_round_stats(cpu, part);
> part_dec_in_flight(part, rw);
>
>
> the problem here is that zram has several paths that may issue IO:
> -- usual zram_make_request()
> -- zram_slot_free_notify()
> -- zram_rw_page()
>
> 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.
>
>
> besides, /sys/block/zramX/stat file exports totally different data:
>
> struct disk_stats {
> unsigned long sectors[2]; /* READs and WRITEs */
> unsigned long ios[2];
> unsigned long merges[2];
> unsigned long ticks[2];
> unsigned long io_ticks;
> unsigned long time_in_queue;
> };
>
> Documentation/block/stat.txt
>
> 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.
> (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.
>
> 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. :)
Thanks for looking this, Sergey!
--
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
