From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754732AbbCIO4z (ORCPT ); Mon, 9 Mar 2015 10:56:55 -0400 Received: from mail-pa0-f41.google.com ([209.85.220.41]:32810 "EHLO mail-pa0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752901AbbCIO4w (ORCPT ); Mon, 9 Mar 2015 10:56:52 -0400 Date: Mon, 9 Mar 2015 23:56:39 +0900 From: Minchan Kim To: Sergey Senozhatsky Cc: akpm@linux-foundation.org, ddstreet@ieee.org, gunho.lee@lge.com, iamjoonsoo.kim@lge.com, jmarchan@redhat.com, juno.choi@lge.com, mel@csn.ul.ie, ngupta@vflare.org, semenzato@google.com, sergey.senozhatsky@gmail.com, sjennings@variantweb.net, mm-commits@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: + zram-support-compaction.patch added to -mm tree Message-ID: <20150309145639.GA7860@blaptop> References: <54f780fc.3sOWZKr7rufmI85r%akpm@linux-foundation.org> <20150305052941.GK14927@swordfish> <20150309004859.GB15184@blaptop> <20150309005718.GA794@swordfish> <20150309010522.GD15184@blaptop> <20150309012728.GB794@swordfish> <20150309014702.GE15184@blaptop> <20150309020717.GC794@swordfish> <20150309022127.GF15184@blaptop> <20150309064856.GE794@swordfish> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150309064856.GE794@swordfish> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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/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