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. > > > > Nameunits 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 sectorssectors number of sectors read > > read ticks milliseconds total wait time for read requests > > write I/Os requests number of write I/Os processed > > write mergesrequests 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_ticksmilliseconds 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/stat and zram/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/stat -- zram internal IO stats in zram/io_stat (no num_reads, no num_writes) -- zram mm stats in zram/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 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/
Re: + zram-support-compaction.patch added to -mm tree
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
>
> Nameunits 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 sectorssectors number of sectors read
> read ticks milliseconds total wait time for read requests
> write I/Os requests number of write I/Os processed
> write mergesrequests 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_ticksmilliseconds 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/
Re: + zram-support-compaction.patch added to -mm tree
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.
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
Nameunits 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 sectorssectors number of sectors read
read ticks milliseconds total wait time for read requests
write I/Os requests number of write I/Os processed
write mergesrequests 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_ticksmilliseconds 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
(or any significant amount) of our IO stats to that file. that will force users
to gather IO stats accross several files.
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.
> > zram/mm_stat
> > s*printf( orig_data_size, compr_data_size, mem_used_total, num_migrated,
> > etc.)
> >
> >
> >
> > so hoprefully in several years we can entirely remove ZRAM_ATTR_RO
> > functions.
> > probably, first moving them under #ifdef CONFIG_OLD_ZRAM_STATS at some point
> > in the future.
>
> Sounds good so we could warn for 1 or 2 years if users are about to use old
> stat
> and finally removes deprecated stat.
good.
> Please Cc util-linux zram-control peoples when you send patchset.
>
ok.
-ss
--
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/
Re: + zram-support-compaction.patch added to -mm tree
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.
zramid/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
Nameunits 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 sectorssectors number of sectors read
read ticks milliseconds total wait time for read requests
write I/Os requests number of write I/Os processed
write mergesrequests 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_ticksmilliseconds 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/
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. Nameunits 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 sectorssectors number of sectors read read ticks milliseconds total wait time for read requests write I/Os requests number of write I/Os processed write mergesrequests 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_ticksmilliseconds 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 zramid/stat and zramid/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 zramid/stat -- zram internal IO stats in zramid/io_stat (no num_reads, no num_writes) -- zram mm stats in zramid/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 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/
Re: + zram-support-compaction.patch added to -mm tree
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.
zramid/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.
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
Nameunits 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 sectorssectors number of sectors read
read ticks milliseconds total wait time for read requests
write I/Os requests number of write I/Os processed
write mergesrequests 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_ticksmilliseconds 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
(or any significant amount) of our IO stats to that file. that will force users
to gather IO stats accross several files.
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.
zramid/mm_stat
s*printf( orig_data_size, compr_data_size, mem_used_total, num_migrated,
etc.)
so hoprefully in several years we can entirely remove ZRAM_ATTR_RO
functions.
probably, first moving them under #ifdef CONFIG_OLD_ZRAM_STATS at some point
in the future.
Sounds good so we could warn for 1 or 2 years if users are about to use old
stat
and finally removes deprecated stat.
good.
Please Cc util-linux zram-control peoples when you send patchset.
ok.
-ss
--
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/
Re: + zram-support-compaction.patch added to -mm tree
On Mon, Mar 09, 2015 at 11:07:17AM +0900, Sergey Senozhatsky wrote: > On (03/09/15 10:47), Minchan Kim wrote: > > > > > > > > It's not enough. What I want to know is compaction efficiency per > > > > client of > > > > zsmalloc(ie, zram). > > > > > > > > > > so what a typical user can do with this information? isn't it an entirely > > > debug info that makes some hidden sense only to developers? > > > > Absolutely true. > > > > > > > > if you insist on exporting this as a zram stat for everyone how obout > > > starting to move away from per-stat RO sysfs attrs. it seems that we have > > > uncomfortably a lot of sysfs attrs, and that doesn't make life easier in > > > user space. for example, block devices have /sys/block/.../stat file: > > > > > > /sys/block/sda$ cat stat > > >45931 59 2075686 28990655768 9229 1967800 318033 > > > 0 193583 607806 > > > > > > and there are no num_reads, num_writes, num_failed_reads, > > > num_failed_writes, > > > etc., etc. per-stat sysfs attrs force user-space to do lots of syscalls: > > > open(), read(), close() with error control on every step; for every stat. > > > > I absoulte agree with you and I really wanted to tidy it up but was no > > time. Sergey, Could you contribute? If you have no time, I will do by > > myself but it would be low priority now. > > sure. I can handle it. Thanks! > > > 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. :) > > zram/mm_stat > s*printf( orig_data_size, compr_data_size, mem_used_total, num_migrated, etc.) > > > > so hoprefully in several years we can entirely remove ZRAM_ATTR_RO functions. > probably, first moving them under #ifdef CONFIG_OLD_ZRAM_STATS at some point > in the future. Sounds good so we could warn for 1 or 2 years if users are about to use old stat and finally removes deprecated stat. Please Cc util-linux zram-control peoples when you send patchset. > > -ss -- 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/
Re: + zram-support-compaction.patch added to -mm tree
On (03/09/15 10:47), Minchan Kim wrote: > > > > > > It's not enough. What I want to know is compaction efficiency per client > > > of > > > zsmalloc(ie, zram). > > > > > > > so what a typical user can do with this information? isn't it an entirely > > debug info that makes some hidden sense only to developers? > > Absolutely true. > > > > > if you insist on exporting this as a zram stat for everyone how obout > > starting to move away from per-stat RO sysfs attrs. it seems that we have > > uncomfortably a lot of sysfs attrs, and that doesn't make life easier in > > user space. for example, block devices have /sys/block/.../stat file: > > > > /sys/block/sda$ cat stat > >45931 59 2075686 28990655768 9229 1967800 318033 > >0 193583 607806 > > > > and there are no num_reads, num_writes, num_failed_reads, num_failed_writes, > > etc., etc. per-stat sysfs attrs force user-space to do lots of syscalls: > > open(), read(), close() with error control on every step; for every stat. > > I absoulte agree with you and I really wanted to tidy it up but was no > time. Sergey, Could you contribute? If you have no time, I will do by > myself but it would be low priority now. sure. I can handle it. 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.) zram/mm_stat s*printf( orig_data_size, compr_data_size, mem_used_total, num_migrated, etc.) so hoprefully in several years we can entirely remove ZRAM_ATTR_RO functions. probably, first moving them under #ifdef CONFIG_OLD_ZRAM_STATS at some point in the future. -ss -- 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/
Re: + zram-support-compaction.patch added to -mm tree
On Mon, Mar 09, 2015 at 10:27:28AM +0900, Sergey Senozhatsky wrote:
> On (03/09/15 10:05), Minchan Kim wrote:
> > > well, to be fair, compaction is a zsmalloc internal. zram has nothing to
> > > do with
> > > it.
> > >
> > > but do we we even need this stat? it seems that
> > >
> > >mem_total_used (before compaction) - mem_total_user (after comapction)
> > >
> > > will give user an idea on how much memory was compacted.
> >
> > It's not enough. What I want to know is compaction efficiency per client of
> > zsmalloc(ie, zram).
> >
>
> so what a typical user can do with this information? isn't it an entirely
> debug info that makes some hidden sense only to developers?
Absolutely true.
>
> if you insist on exporting this as a zram stat for everyone how obout
> starting to move away from per-stat RO sysfs attrs. it seems that we have
> uncomfortably a lot of sysfs attrs, and that doesn't make life easier in
> user space. for example, block devices have /sys/block/.../stat file:
>
> /sys/block/sda$ cat stat
>45931 59 2075686 28990655768 9229 1967800 318033
> 0 193583 607806
>
> and there are no num_reads, num_writes, num_failed_reads, num_failed_writes,
> etc., etc. per-stat sysfs attrs force user-space to do lots of syscalls:
> open(), read(), close() with error control on every step; for every stat.
I absoulte agree with you and I really wanted to tidy it up but was no
time. Sergey, Could you contribute? If you have no time, I will do by
myself but it would be low priority now.
>
> so how about introducing zram/malloc_stats (or any similar name) and
> provide compaction and all future allocator related stats there
> (via s*printf("%d %d %d", )) ?
>
> -ss
>
> > IOW, (how many of freed pages / how many of objects) per
> > zs_compact.
> >
> > >
> > > -ss
> > >
> > > > That's why it is on debugfs. If we add the stat into zsmalloc, we
> > > > should turn on debugfs
> > > > and CONFIG_ZSMALLOC_STAT to see *a* stat. Even, CONFIG_ZSMALLOC_STAT
> > > > will add
> > > > unncessary overheads to account another stats fo zsmalloc internals.
> > > >
> > > > As well, if we add auto-compacion like stuff in zsmalloc(ie, it will
> > > > trigger
> > > > by itself if fragmention is over to predefined theshold), the stat will
> > > > accumulate stat while someone want to see snapshot compaction effiecieny
> > > > of the moment.
> > > >
> > > > So, I want to keep it in zram now.
> > > >
> > > > --
> > > > Kind regards,
> > > > Minchan Kim
> > > >
> >
> > --
> > Kind regards,
> > Minchan Kim
> >
--
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/
Re: + zram-support-compaction.patch added to -mm tree
On (03/09/15 10:05), Minchan Kim wrote:
> > well, to be fair, compaction is a zsmalloc internal. zram has nothing to do
> > with
> > it.
> >
> > but do we we even need this stat? it seems that
> >
> >mem_total_used (before compaction) - mem_total_user (after comapction)
> >
> > will give user an idea on how much memory was compacted.
>
> It's not enough. What I want to know is compaction efficiency per client of
> zsmalloc(ie, zram).
>
so what a typical user can do with this information? isn't it an entirely
debug info that makes some hidden sense only to developers?
if you insist on exporting this as a zram stat for everyone how obout
starting to move away from per-stat RO sysfs attrs. it seems that we have
uncomfortably a lot of sysfs attrs, and that doesn't make life easier in
user space. for example, block devices have /sys/block/.../stat file:
/sys/block/sda$ cat stat
45931 59 2075686 28990655768 9229 1967800 318033
0 193583 607806
and there are no num_reads, num_writes, num_failed_reads, num_failed_writes,
etc., etc. per-stat sysfs attrs force user-space to do lots of syscalls:
open(), read(), close() with error control on every step; for every stat.
so how about introducing zram/malloc_stats (or any similar name) and
provide compaction and all future allocator related stats there
(via s*printf("%d %d %d", )) ?
-ss
> IOW, (how many of freed pages / how many of objects) per
> zs_compact.
>
> >
> > -ss
> >
> > > That's why it is on debugfs. If we add the stat into zsmalloc, we should
> > > turn on debugfs
> > > and CONFIG_ZSMALLOC_STAT to see *a* stat. Even, CONFIG_ZSMALLOC_STAT will
> > > add
> > > unncessary overheads to account another stats fo zsmalloc internals.
> > >
> > > As well, if we add auto-compacion like stuff in zsmalloc(ie, it will
> > > trigger
> > > by itself if fragmention is over to predefined theshold), the stat will
> > > accumulate stat while someone want to see snapshot compaction effiecieny
> > > of the moment.
> > >
> > > So, I want to keep it in zram now.
> > >
> > > --
> > > Kind regards,
> > > Minchan Kim
> > >
>
> --
> 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/
Re: + zram-support-compaction.patch added to -mm tree
On Mon, Mar 09, 2015 at 09:57:18AM +0900, Sergey Senozhatsky wrote: > On (03/09/15 09:49), Minchan Kim wrote: > > > rather a discussion question. > > > > > > Minchan, do you want to provide num_migrated as part of zsmalloc stats > > > rather > > > than having yet another zram attr? we already provide zsmalloc stats and > > > this > > > type of information seems to belong there. just idea. > > > > Hmm, CONFIG_ZSMALLOC_STAT is actually to show zsmalloc internals. > > well, to be fair, compaction is a zsmalloc internal. zram has nothing to do > with > it. > > but do we we even need this stat? it seems that > >mem_total_used (before compaction) - mem_total_user (after comapction) > > will give user an idea on how much memory was compacted. It's not enough. What I want to know is compaction efficiency per client of zsmalloc(ie, zram). IOW, (how many of freed pages / how many of objects) per zs_compact. > > -ss > > > That's why it is on debugfs. If we add the stat into zsmalloc, we should > > turn on debugfs > > and CONFIG_ZSMALLOC_STAT to see *a* stat. Even, CONFIG_ZSMALLOC_STAT will > > add > > unncessary overheads to account another stats fo zsmalloc internals. > > > > As well, if we add auto-compacion like stuff in zsmalloc(ie, it will trigger > > by itself if fragmention is over to predefined theshold), the stat will > > accumulate stat while someone want to see snapshot compaction effiecieny > > of the moment. > > > > So, I want to keep it in zram now. > > > > -- > > Kind regards, > > Minchan Kim > > -- 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/
Re: + zram-support-compaction.patch added to -mm tree
On (03/09/15 09:49), Minchan Kim wrote: > > rather a discussion question. > > > > Minchan, do you want to provide num_migrated as part of zsmalloc stats > > rather > > than having yet another zram attr? we already provide zsmalloc stats and > > this > > type of information seems to belong there. just idea. > > Hmm, CONFIG_ZSMALLOC_STAT is actually to show zsmalloc internals. well, to be fair, compaction is a zsmalloc internal. zram has nothing to do with it. but do we we even need this stat? it seems that mem_total_used (before compaction) - mem_total_user (after comapction) will give user an idea on how much memory was compacted. -ss > That's why it is on debugfs. If we add the stat into zsmalloc, we should turn > on debugfs > and CONFIG_ZSMALLOC_STAT to see *a* stat. Even, CONFIG_ZSMALLOC_STAT will add > unncessary overheads to account another stats fo zsmalloc internals. > > As well, if we add auto-compacion like stuff in zsmalloc(ie, it will trigger > by itself if fragmention is over to predefined theshold), the stat will > accumulate stat while someone want to see snapshot compaction effiecieny > of the moment. > > So, I want to keep it in zram now. > > -- > 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/
Re: + zram-support-compaction.patch added to -mm tree
Hello Sergey, On Thu, Mar 05, 2015 at 02:29:42PM +0900, Sergey Senozhatsky wrote: > On (03/04/15 14:02), a...@linux-foundation.org wrote: > > +What: /sys/block/zram/compact > > +Date: August 2015 > > +Contact: Minchan Kim > > +Description: > > + The compact file is write-only and trigger compaction for > > + allocator zrm uses. The allocator moves some objects so that > > + it could free fragment space. > > + > > +What: /sys/block/zram/num_migrated > > +Date: August 2015 > > +Contact: Minchan Kim > > +Description: > > + The compact file is read-only and shows how many object > > + migrated by compaction. > > diff -puN drivers/block/zram/zram_drv.c~zram-support-compaction > > drivers/block/zram/zram_drv.c > > --- a/drivers/block/zram/zram_drv.c~zram-support-compaction > > +++ a/drivers/block/zram/zram_drv.c > > @@ -70,6 +70,27 @@ static inline struct zram *dev_to_zram(s > > return (struct zram *)dev_to_disk(dev)->private_data; > > } > > First of all, my apologies to Andrew Morton. if I reply to this email, my > mutt for > some reason replaces akpm at linux-foundation.org with linux-kernel at > vger.kernel.org > (I can't see why this is happening, but this is somehow a `stable > behaviour'). I didn't > spot this, so this is why Andrew was not Cc-d to my previous reply to this > eamil. > > > > > rather a discussion question. > > Minchan, do you want to provide num_migrated as part of zsmalloc stats rather > than having yet another zram attr? we already provide zsmalloc stats and this > type of information seems to belong there. just idea. Hmm, CONFIG_ZSMALLOC_STAT is actually to show zsmalloc internals. That's why it is on debugfs. If we add the stat into zsmalloc, we should turn on debugfs and CONFIG_ZSMALLOC_STAT to see *a* stat. Even, CONFIG_ZSMALLOC_STAT will add unncessary overheads to account another stats fo zsmalloc internals. As well, if we add auto-compacion like stuff in zsmalloc(ie, it will trigger by itself if fragmention is over to predefined theshold), the stat will accumulate stat while someone want to see snapshot compaction effiecieny of the moment. So, I want to keep it in zram now. -- 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/
Re: + zram-support-compaction.patch added to -mm tree
On (03/09/15 10:47), Minchan Kim wrote: It's not enough. What I want to know is compaction efficiency per client of zsmalloc(ie, zram). so what a typical user can do with this information? isn't it an entirely debug info that makes some hidden sense only to developers? Absolutely true. if you insist on exporting this as a zram stat for everyone how obout starting to move away from per-stat RO sysfs attrs. it seems that we have uncomfortably a lot of sysfs attrs, and that doesn't make life easier in user space. for example, block devices have /sys/block/.../stat file: /sys/block/sda$ cat stat 45931 59 2075686 28990655768 9229 1967800 318033 0 193583 607806 and there are no num_reads, num_writes, num_failed_reads, num_failed_writes, etc., etc. per-stat sysfs attrs force user-space to do lots of syscalls: open(), read(), close() with error control on every step; for every stat. I absoulte agree with you and I really wanted to tidy it up but was no time. Sergey, Could you contribute? If you have no time, I will do by myself but it would be low priority now. sure. I can handle it. 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. zramid/io_stat s*printf( num_reads, num_writes, failed_reads, failed_writes, etc.) zramid/mm_stat s*printf( orig_data_size, compr_data_size, mem_used_total, num_migrated, etc.) so hoprefully in several years we can entirely remove ZRAM_ATTR_RO functions. probably, first moving them under #ifdef CONFIG_OLD_ZRAM_STATS at some point in the future. -ss -- 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/
Re: + zram-support-compaction.patch added to -mm tree
Hello Sergey, On Thu, Mar 05, 2015 at 02:29:42PM +0900, Sergey Senozhatsky wrote: On (03/04/15 14:02), a...@linux-foundation.org wrote: +What: /sys/block/zramid/compact +Date: August 2015 +Contact: Minchan Kim minc...@kernel.org +Description: + The compact file is write-only and trigger compaction for + allocator zrm uses. The allocator moves some objects so that + it could free fragment space. + +What: /sys/block/zramid/num_migrated +Date: August 2015 +Contact: Minchan Kim minc...@kernel.org +Description: + The compact file is read-only and shows how many object + migrated by compaction. diff -puN drivers/block/zram/zram_drv.c~zram-support-compaction drivers/block/zram/zram_drv.c --- a/drivers/block/zram/zram_drv.c~zram-support-compaction +++ a/drivers/block/zram/zram_drv.c @@ -70,6 +70,27 @@ static inline struct zram *dev_to_zram(s return (struct zram *)dev_to_disk(dev)-private_data; } First of all, my apologies to Andrew Morton. if I reply to this email, my mutt for some reason replaces akpm at linux-foundation.org with linux-kernel at vger.kernel.org (I can't see why this is happening, but this is somehow a `stable behaviour'). I didn't spot this, so this is why Andrew was not Cc-d to my previous reply to this eamil. rather a discussion question. Minchan, do you want to provide num_migrated as part of zsmalloc stats rather than having yet another zram attr? we already provide zsmalloc stats and this type of information seems to belong there. just idea. Hmm, CONFIG_ZSMALLOC_STAT is actually to show zsmalloc internals. That's why it is on debugfs. If we add the stat into zsmalloc, we should turn on debugfs and CONFIG_ZSMALLOC_STAT to see *a* stat. Even, CONFIG_ZSMALLOC_STAT will add unncessary overheads to account another stats fo zsmalloc internals. As well, if we add auto-compacion like stuff in zsmalloc(ie, it will trigger by itself if fragmention is over to predefined theshold), the stat will accumulate stat while someone want to see snapshot compaction effiecieny of the moment. So, I want to keep it in zram now. -- 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/
Re: + zram-support-compaction.patch added to -mm tree
On (03/09/15 10:05), Minchan Kim wrote: well, to be fair, compaction is a zsmalloc internal. zram has nothing to do with it. but do we we even need this stat? it seems that mem_total_used (before compaction) - mem_total_user (after comapction) will give user an idea on how much memory was compacted. It's not enough. What I want to know is compaction efficiency per client of zsmalloc(ie, zram). so what a typical user can do with this information? isn't it an entirely debug info that makes some hidden sense only to developers? if you insist on exporting this as a zram stat for everyone how obout starting to move away from per-stat RO sysfs attrs. it seems that we have uncomfortably a lot of sysfs attrs, and that doesn't make life easier in user space. for example, block devices have /sys/block/.../stat file: /sys/block/sda$ cat stat 45931 59 2075686 28990655768 9229 1967800 318033 0 193583 607806 and there are no num_reads, num_writes, num_failed_reads, num_failed_writes, etc., etc. per-stat sysfs attrs force user-space to do lots of syscalls: open(), read(), close() with error control on every step; for every stat. so how about introducing zramid/malloc_stats (or any similar name) and provide compaction and all future allocator related stats there (via s*printf(%d %d %d, )) ? -ss IOW, (how many of freed pages / how many of objects) per zs_compact. -ss That's why it is on debugfs. If we add the stat into zsmalloc, we should turn on debugfs and CONFIG_ZSMALLOC_STAT to see *a* stat. Even, CONFIG_ZSMALLOC_STAT will add unncessary overheads to account another stats fo zsmalloc internals. As well, if we add auto-compacion like stuff in zsmalloc(ie, it will trigger by itself if fragmention is over to predefined theshold), the stat will accumulate stat while someone want to see snapshot compaction effiecieny of the moment. So, I want to keep it in zram now. -- Kind regards, Minchan Kim -- 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/
Re: + zram-support-compaction.patch added to -mm tree
On (03/09/15 09:49), Minchan Kim wrote: rather a discussion question. Minchan, do you want to provide num_migrated as part of zsmalloc stats rather than having yet another zram attr? we already provide zsmalloc stats and this type of information seems to belong there. just idea. Hmm, CONFIG_ZSMALLOC_STAT is actually to show zsmalloc internals. well, to be fair, compaction is a zsmalloc internal. zram has nothing to do with it. but do we we even need this stat? it seems that mem_total_used (before compaction) - mem_total_user (after comapction) will give user an idea on how much memory was compacted. -ss That's why it is on debugfs. If we add the stat into zsmalloc, we should turn on debugfs and CONFIG_ZSMALLOC_STAT to see *a* stat. Even, CONFIG_ZSMALLOC_STAT will add unncessary overheads to account another stats fo zsmalloc internals. As well, if we add auto-compacion like stuff in zsmalloc(ie, it will trigger by itself if fragmention is over to predefined theshold), the stat will accumulate stat while someone want to see snapshot compaction effiecieny of the moment. So, I want to keep it in zram now. -- 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/
Re: + zram-support-compaction.patch added to -mm tree
On Mon, Mar 09, 2015 at 11:07:17AM +0900, Sergey Senozhatsky wrote: On (03/09/15 10:47), Minchan Kim wrote: It's not enough. What I want to know is compaction efficiency per client of zsmalloc(ie, zram). so what a typical user can do with this information? isn't it an entirely debug info that makes some hidden sense only to developers? Absolutely true. if you insist on exporting this as a zram stat for everyone how obout starting to move away from per-stat RO sysfs attrs. it seems that we have uncomfortably a lot of sysfs attrs, and that doesn't make life easier in user space. for example, block devices have /sys/block/.../stat file: /sys/block/sda$ cat stat 45931 59 2075686 28990655768 9229 1967800 318033 0 193583 607806 and there are no num_reads, num_writes, num_failed_reads, num_failed_writes, etc., etc. per-stat sysfs attrs force user-space to do lots of syscalls: open(), read(), close() with error control on every step; for every stat. I absoulte agree with you and I really wanted to tidy it up but was no time. Sergey, Could you contribute? If you have no time, I will do by myself but it would be low priority now. sure. I can handle it. Thanks! 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. zramid/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. :) zramid/mm_stat s*printf( orig_data_size, compr_data_size, mem_used_total, num_migrated, etc.) so hoprefully in several years we can entirely remove ZRAM_ATTR_RO functions. probably, first moving them under #ifdef CONFIG_OLD_ZRAM_STATS at some point in the future. Sounds good so we could warn for 1 or 2 years if users are about to use old stat and finally removes deprecated stat. Please Cc util-linux zram-control peoples when you send patchset. -ss -- 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/
Re: + zram-support-compaction.patch added to -mm tree
On Mon, Mar 09, 2015 at 09:57:18AM +0900, Sergey Senozhatsky wrote: On (03/09/15 09:49), Minchan Kim wrote: rather a discussion question. Minchan, do you want to provide num_migrated as part of zsmalloc stats rather than having yet another zram attr? we already provide zsmalloc stats and this type of information seems to belong there. just idea. Hmm, CONFIG_ZSMALLOC_STAT is actually to show zsmalloc internals. well, to be fair, compaction is a zsmalloc internal. zram has nothing to do with it. but do we we even need this stat? it seems that mem_total_used (before compaction) - mem_total_user (after comapction) will give user an idea on how much memory was compacted. It's not enough. What I want to know is compaction efficiency per client of zsmalloc(ie, zram). IOW, (how many of freed pages / how many of objects) per zs_compact. -ss That's why it is on debugfs. If we add the stat into zsmalloc, we should turn on debugfs and CONFIG_ZSMALLOC_STAT to see *a* stat. Even, CONFIG_ZSMALLOC_STAT will add unncessary overheads to account another stats fo zsmalloc internals. As well, if we add auto-compacion like stuff in zsmalloc(ie, it will trigger by itself if fragmention is over to predefined theshold), the stat will accumulate stat while someone want to see snapshot compaction effiecieny of the moment. So, I want to keep it in zram now. -- Kind regards, Minchan Kim -- 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/
Re: + zram-support-compaction.patch added to -mm tree
On Mon, Mar 09, 2015 at 10:27:28AM +0900, Sergey Senozhatsky wrote: On (03/09/15 10:05), Minchan Kim wrote: well, to be fair, compaction is a zsmalloc internal. zram has nothing to do with it. but do we we even need this stat? it seems that mem_total_used (before compaction) - mem_total_user (after comapction) will give user an idea on how much memory was compacted. It's not enough. What I want to know is compaction efficiency per client of zsmalloc(ie, zram). so what a typical user can do with this information? isn't it an entirely debug info that makes some hidden sense only to developers? Absolutely true. if you insist on exporting this as a zram stat for everyone how obout starting to move away from per-stat RO sysfs attrs. it seems that we have uncomfortably a lot of sysfs attrs, and that doesn't make life easier in user space. for example, block devices have /sys/block/.../stat file: /sys/block/sda$ cat stat 45931 59 2075686 28990655768 9229 1967800 318033 0 193583 607806 and there are no num_reads, num_writes, num_failed_reads, num_failed_writes, etc., etc. per-stat sysfs attrs force user-space to do lots of syscalls: open(), read(), close() with error control on every step; for every stat. I absoulte agree with you and I really wanted to tidy it up but was no time. Sergey, Could you contribute? If you have no time, I will do by myself but it would be low priority now. so how about introducing zramid/malloc_stats (or any similar name) and provide compaction and all future allocator related stats there (via s*printf(%d %d %d, )) ? -ss IOW, (how many of freed pages / how many of objects) per zs_compact. -ss That's why it is on debugfs. If we add the stat into zsmalloc, we should turn on debugfs and CONFIG_ZSMALLOC_STAT to see *a* stat. Even, CONFIG_ZSMALLOC_STAT will add unncessary overheads to account another stats fo zsmalloc internals. As well, if we add auto-compacion like stuff in zsmalloc(ie, it will trigger by itself if fragmention is over to predefined theshold), the stat will accumulate stat while someone want to see snapshot compaction effiecieny of the moment. So, I want to keep it in zram now. -- Kind regards, Minchan Kim -- Kind regards, Minchan Kim -- 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/
Re: + zram-support-compaction.patch added to -mm tree
On (03/05/15 14:29), Sergey Senozhatsky wrote: > On (03/04/15 14:02), a...@linux-foundation.org wrote: > > +What: /sys/block/zram/compact > > +Date: August 2015 > > +Contact: Minchan Kim > > +Description: > > + The compact file is write-only and trigger compaction for > > + allocator zrm uses. The allocator moves some objects so that > > + it could free fragment space. > > + > > +What: /sys/block/zram/num_migrated > > +Date: August 2015 > > +Contact: Minchan Kim > > +Description: > > + The compact file is read-only and shows how many object > > + migrated by compaction. > Minchan, do you want to provide num_migrated as part of zsmalloc stats rather > than having yet another zram attr? we already provide zsmalloc stats and this > type of information seems to belong there. just idea. > can compaction memory gain be calculated as, for example, mem_used_total #before_compaction - mem_used_total #after_compaction ? if yes, then do we need this attr at all? the other thing is - why does this attr return the sum of all compactions? wouldn't it be more informative to return the number of bytes gained after the most recent compaction? but, once again, is this attr worth having? -ss -- 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/
Re: + zram-support-compaction.patch added to -mm tree
On (03/05/15 14:29), Sergey Senozhatsky wrote: On (03/04/15 14:02), a...@linux-foundation.org wrote: +What: /sys/block/zramid/compact +Date: August 2015 +Contact: Minchan Kim minc...@kernel.org +Description: + The compact file is write-only and trigger compaction for + allocator zrm uses. The allocator moves some objects so that + it could free fragment space. + +What: /sys/block/zramid/num_migrated +Date: August 2015 +Contact: Minchan Kim minc...@kernel.org +Description: + The compact file is read-only and shows how many object + migrated by compaction. Minchan, do you want to provide num_migrated as part of zsmalloc stats rather than having yet another zram attr? we already provide zsmalloc stats and this type of information seems to belong there. just idea. can compaction memory gain be calculated as, for example, mem_used_total #before_compaction - mem_used_total #after_compaction ? if yes, then do we need this attr at all? the other thing is - why does this attr return the sum of all compactions? wouldn't it be more informative to return the number of bytes gained after the most recent compaction? but, once again, is this attr worth having? -ss -- 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/
Re: + zram-support-compaction.patch added to -mm tree
On (03/04/15 14:02), a...@linux-foundation.org wrote:
> +What:/sys/block/zram/compact
> +Date:August 2015
> +Contact: Minchan Kim
> +Description:
> + The compact file is write-only and trigger compaction for
> + allocator zrm uses. The allocator moves some objects so that
> + it could free fragment space.
> +
> +What:/sys/block/zram/num_migrated
> +Date:August 2015
> +Contact: Minchan Kim
> +Description:
> + The compact file is read-only and shows how many object
> + migrated by compaction.
> diff -puN drivers/block/zram/zram_drv.c~zram-support-compaction
> drivers/block/zram/zram_drv.c
> --- a/drivers/block/zram/zram_drv.c~zram-support-compaction
> +++ a/drivers/block/zram/zram_drv.c
> @@ -70,6 +70,27 @@ static inline struct zram *dev_to_zram(s
> return (struct zram *)dev_to_disk(dev)->private_data;
> }
First of all, my apologies to Andrew Morton. if I reply to this email, my mutt
for
some reason replaces akpm at linux-foundation.org with linux-kernel at
vger.kernel.org
(I can't see why this is happening, but this is somehow a `stable behaviour').
I didn't
spot this, so this is why Andrew was not Cc-d to my previous reply to this
eamil.
rather a discussion question.
Minchan, do you want to provide num_migrated as part of zsmalloc stats rather
than having yet another zram attr? we already provide zsmalloc stats and this
type of information seems to belong there. just idea.
-ss
>
> +static ssize_t compact_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t len)
> +{
> + unsigned long nr_migrated;
> + struct zram *zram = dev_to_zram(dev);
> + struct zram_meta *meta;
> +
> + down_read(>init_lock);
> + if (!init_done(zram)) {
> + up_read(>init_lock);
> + return -EINVAL;
> + }
> +
> + meta = zram->meta;
> + nr_migrated = zs_compact(meta->mem_pool);
> + atomic64_add(nr_migrated, >stats.num_migrated);
> + up_read(>init_lock);
> +
> + return len;
> +}
--
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/
Re: + zram-support-compaction.patch added to -mm tree
Hello Sergey,
On Thu, Mar 05, 2015 at 09:18:45AM +0900, Sergey Senozhatsky wrote:
> Hello,
>
> On (03/04/15 14:02), a...@linux-foundation.org wrote:
> [..]
> > +++ a/drivers/block/zram/zram_drv.c
> > @@ -70,6 +70,27 @@ static inline struct zram *dev_to_zram(s
> > return (struct zram *)dev_to_disk(dev)->private_data;
> > }
> >
> > +static ssize_t compact_store(struct device *dev,
> > + struct device_attribute *attr, const char *buf, size_t len)
> > +{
> > + unsigned long nr_migrated;
> > + struct zram *zram = dev_to_zram(dev);
> > + struct zram_meta *meta;
> > +
> > + down_read(>init_lock);
> > + if (!init_done(zram)) {
> > + up_read(>init_lock);
> > + return -EINVAL;
> > + }
> > +
> > + meta = zram->meta;
> > + nr_migrated = zs_compact(meta->mem_pool);
> > + atomic64_add(nr_migrated, >stats.num_migrated);
> > + up_read(>init_lock);
> > +
> > + return len;
> > +}
> > +
> > /* flag operations require table entry bit_spin_lock() being held */
> > static int zram_test_flag(struct zram_meta *meta, u32 index,
> > enum zram_pageflags flag)
>
>
> let's stick to "helpers, attrs show/store, mm (meta, page), IO, zram control"
> function layout.
>
> so can we please put compact_store() after, say,
> 354 static ssize_t comp_algorithm_store(...)
>
> function?
I will clean it up after Andrew releases new mmotm. :)
Thanks for the review!
--
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/
Re: + zram-support-compaction.patch added to -mm tree
Hello,
On (03/04/15 14:02), a...@linux-foundation.org wrote:
[..]
> +++ a/drivers/block/zram/zram_drv.c
> @@ -70,6 +70,27 @@ static inline struct zram *dev_to_zram(s
> return (struct zram *)dev_to_disk(dev)->private_data;
> }
>
> +static ssize_t compact_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t len)
> +{
> + unsigned long nr_migrated;
> + struct zram *zram = dev_to_zram(dev);
> + struct zram_meta *meta;
> +
> + down_read(>init_lock);
> + if (!init_done(zram)) {
> + up_read(>init_lock);
> + return -EINVAL;
> + }
> +
> + meta = zram->meta;
> + nr_migrated = zs_compact(meta->mem_pool);
> + atomic64_add(nr_migrated, >stats.num_migrated);
> + up_read(>init_lock);
> +
> + return len;
> +}
> +
> /* flag operations require table entry bit_spin_lock() being held */
> static int zram_test_flag(struct zram_meta *meta, u32 index,
> enum zram_pageflags flag)
let's stick to "helpers, attrs show/store, mm (meta, page), IO, zram control"
function layout.
so can we please put compact_store() after, say,
354 static ssize_t comp_algorithm_store(...)
function?
-ss
> @@ -374,6 +395,7 @@ ZRAM_ATTR_RO(invalid_io);
> ZRAM_ATTR_RO(notify_free);
> ZRAM_ATTR_RO(zero_pages);
> ZRAM_ATTR_RO(compr_data_size);
> +ZRAM_ATTR_RO(num_migrated);
>
> static inline bool zram_meta_get(struct zram *zram)
> {
> @@ -1031,6 +1053,7 @@ static const struct block_device_operati
> .owner = THIS_MODULE
> };
>
> +static DEVICE_ATTR_WO(compact);
> static DEVICE_ATTR_RW(disksize);
> static DEVICE_ATTR_RO(initstate);
> static DEVICE_ATTR_WO(reset);
> @@ -1049,6 +1072,8 @@ static struct attribute *zram_disk_attrs
> _attr_num_writes.attr,
> _attr_failed_reads.attr,
> _attr_failed_writes.attr,
> + _attr_num_migrated.attr,
> + _attr_compact.attr,
> _attr_invalid_io.attr,
> _attr_notify_free.attr,
> _attr_zero_pages.attr,
> diff -puN drivers/block/zram/zram_drv.h~zram-support-compaction
> drivers/block/zram/zram_drv.h
> --- a/drivers/block/zram/zram_drv.h~zram-support-compaction
> +++ a/drivers/block/zram/zram_drv.h
> @@ -78,6 +78,7 @@ struct zram_stats {
> atomic64_t compr_data_size; /* compressed size of pages stored */
> atomic64_t num_reads; /* failed + successful */
> atomic64_t num_writes; /* --do-- */
> + atomic64_t num_migrated;/* no. of migrated object */
> atomic64_t failed_reads;/* can happen when memory is too low */
> atomic64_t failed_writes; /* can happen when memory is too low */
> atomic64_t invalid_io; /* non-page-aligned I/O requests */
> _
>
> Patches currently in -mm which might be from minc...@kernel.org are
>
> mm-vmscan-fix-the-page-state-calculation-in-too_many_isolated.patch
> mm-page_isolation-check-pfn-validity-before-access.patch
> mm-support-madvisemadv_free.patch
> mm-support-madvisemadv_free-fix.patch
> x86-add-pmd_-for-thp.patch
> x86-add-pmd_-for-thp-fix.patch
> sparc-add-pmd_-for-thp.patch
> sparc-add-pmd_-for-thp-fix.patch
> powerpc-add-pmd_-for-thp.patch
> arm-add-pmd_mkclean-for-thp.patch
> arm64-add-pmd_-for-thp.patch
> mm-dont-split-thp-page-when-syscall-is-called.patch
> mm-dont-split-thp-page-when-syscall-is-called-fix.patch
> mm-dont-split-thp-page-when-syscall-is-called-fix-2.patch
> zram-cosmetic-zram_attr_ro-code-formatting-tweak.patch
> zram-use-idr-instead-of-zram_devices-array.patch
> zram-factor-out-device-reset-from-reset_store.patch
> zram-reorganize-code-layout.patch
> zram-add-dynamic-device-add-remove-functionality.patch
> zram-remove-max_num_devices-limitation.patch
> zram-report-every-added-and-removed-device.patch
> zram-trivial-correct-flag-operations-comment.patch
> zsmalloc-decouple-handle-and-object.patch
> zsmalloc-factor-out-obj_.patch
> zsmalloc-support-compaction.patch
> zsmalloc-adjust-zs_almost_full.patch
> zram-support-compaction.patch
> zsmalloc-record-handle-in-page-private-for-huge-object.patch
> zsmalloc-add-fullness-into-stat.patch
>
> --
> To unsubscribe from this list: send the line "unsubscribe mm-commits" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
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/
Re: + zram-support-compaction.patch added to -mm tree
Hello,
On (03/04/15 14:02), a...@linux-foundation.org wrote:
[..]
+++ a/drivers/block/zram/zram_drv.c
@@ -70,6 +70,27 @@ static inline struct zram *dev_to_zram(s
return (struct zram *)dev_to_disk(dev)-private_data;
}
+static ssize_t compact_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t len)
+{
+ unsigned long nr_migrated;
+ struct zram *zram = dev_to_zram(dev);
+ struct zram_meta *meta;
+
+ down_read(zram-init_lock);
+ if (!init_done(zram)) {
+ up_read(zram-init_lock);
+ return -EINVAL;
+ }
+
+ meta = zram-meta;
+ nr_migrated = zs_compact(meta-mem_pool);
+ atomic64_add(nr_migrated, zram-stats.num_migrated);
+ up_read(zram-init_lock);
+
+ return len;
+}
+
/* flag operations require table entry bit_spin_lock() being held */
static int zram_test_flag(struct zram_meta *meta, u32 index,
enum zram_pageflags flag)
let's stick to helpers, attrs show/store, mm (meta, page), IO, zram control
function layout.
so can we please put compact_store() after, say,
354 static ssize_t comp_algorithm_store(...)
function?
-ss
@@ -374,6 +395,7 @@ ZRAM_ATTR_RO(invalid_io);
ZRAM_ATTR_RO(notify_free);
ZRAM_ATTR_RO(zero_pages);
ZRAM_ATTR_RO(compr_data_size);
+ZRAM_ATTR_RO(num_migrated);
static inline bool zram_meta_get(struct zram *zram)
{
@@ -1031,6 +1053,7 @@ static const struct block_device_operati
.owner = THIS_MODULE
};
+static DEVICE_ATTR_WO(compact);
static DEVICE_ATTR_RW(disksize);
static DEVICE_ATTR_RO(initstate);
static DEVICE_ATTR_WO(reset);
@@ -1049,6 +1072,8 @@ static struct attribute *zram_disk_attrs
dev_attr_num_writes.attr,
dev_attr_failed_reads.attr,
dev_attr_failed_writes.attr,
+ dev_attr_num_migrated.attr,
+ dev_attr_compact.attr,
dev_attr_invalid_io.attr,
dev_attr_notify_free.attr,
dev_attr_zero_pages.attr,
diff -puN drivers/block/zram/zram_drv.h~zram-support-compaction
drivers/block/zram/zram_drv.h
--- a/drivers/block/zram/zram_drv.h~zram-support-compaction
+++ a/drivers/block/zram/zram_drv.h
@@ -78,6 +78,7 @@ struct zram_stats {
atomic64_t compr_data_size; /* compressed size of pages stored */
atomic64_t num_reads; /* failed + successful */
atomic64_t num_writes; /* --do-- */
+ atomic64_t num_migrated;/* no. of migrated object */
atomic64_t failed_reads;/* can happen when memory is too low */
atomic64_t failed_writes; /* can happen when memory is too low */
atomic64_t invalid_io; /* non-page-aligned I/O requests */
_
Patches currently in -mm which might be from minc...@kernel.org are
mm-vmscan-fix-the-page-state-calculation-in-too_many_isolated.patch
mm-page_isolation-check-pfn-validity-before-access.patch
mm-support-madvisemadv_free.patch
mm-support-madvisemadv_free-fix.patch
x86-add-pmd_-for-thp.patch
x86-add-pmd_-for-thp-fix.patch
sparc-add-pmd_-for-thp.patch
sparc-add-pmd_-for-thp-fix.patch
powerpc-add-pmd_-for-thp.patch
arm-add-pmd_mkclean-for-thp.patch
arm64-add-pmd_-for-thp.patch
mm-dont-split-thp-page-when-syscall-is-called.patch
mm-dont-split-thp-page-when-syscall-is-called-fix.patch
mm-dont-split-thp-page-when-syscall-is-called-fix-2.patch
zram-cosmetic-zram_attr_ro-code-formatting-tweak.patch
zram-use-idr-instead-of-zram_devices-array.patch
zram-factor-out-device-reset-from-reset_store.patch
zram-reorganize-code-layout.patch
zram-add-dynamic-device-add-remove-functionality.patch
zram-remove-max_num_devices-limitation.patch
zram-report-every-added-and-removed-device.patch
zram-trivial-correct-flag-operations-comment.patch
zsmalloc-decouple-handle-and-object.patch
zsmalloc-factor-out-obj_.patch
zsmalloc-support-compaction.patch
zsmalloc-adjust-zs_almost_full.patch
zram-support-compaction.patch
zsmalloc-record-handle-in-page-private-for-huge-object.patch
zsmalloc-add-fullness-into-stat.patch
--
To unsubscribe from this list: send the line unsubscribe mm-commits in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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/
Re: + zram-support-compaction.patch added to -mm tree
On (03/04/15 14:02), a...@linux-foundation.org wrote:
+What:/sys/block/zramid/compact
+Date:August 2015
+Contact: Minchan Kim minc...@kernel.org
+Description:
+ The compact file is write-only and trigger compaction for
+ allocator zrm uses. The allocator moves some objects so that
+ it could free fragment space.
+
+What:/sys/block/zramid/num_migrated
+Date:August 2015
+Contact: Minchan Kim minc...@kernel.org
+Description:
+ The compact file is read-only and shows how many object
+ migrated by compaction.
diff -puN drivers/block/zram/zram_drv.c~zram-support-compaction
drivers/block/zram/zram_drv.c
--- a/drivers/block/zram/zram_drv.c~zram-support-compaction
+++ a/drivers/block/zram/zram_drv.c
@@ -70,6 +70,27 @@ static inline struct zram *dev_to_zram(s
return (struct zram *)dev_to_disk(dev)-private_data;
}
First of all, my apologies to Andrew Morton. if I reply to this email, my mutt
for
some reason replaces akpm at linux-foundation.org with linux-kernel at
vger.kernel.org
(I can't see why this is happening, but this is somehow a `stable behaviour').
I didn't
spot this, so this is why Andrew was not Cc-d to my previous reply to this
eamil.
rather a discussion question.
Minchan, do you want to provide num_migrated as part of zsmalloc stats rather
than having yet another zram attr? we already provide zsmalloc stats and this
type of information seems to belong there. just idea.
-ss
+static ssize_t compact_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t len)
+{
+ unsigned long nr_migrated;
+ struct zram *zram = dev_to_zram(dev);
+ struct zram_meta *meta;
+
+ down_read(zram-init_lock);
+ if (!init_done(zram)) {
+ up_read(zram-init_lock);
+ return -EINVAL;
+ }
+
+ meta = zram-meta;
+ nr_migrated = zs_compact(meta-mem_pool);
+ atomic64_add(nr_migrated, zram-stats.num_migrated);
+ up_read(zram-init_lock);
+
+ return len;
+}
--
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/
Re: + zram-support-compaction.patch added to -mm tree
Hello Sergey,
On Thu, Mar 05, 2015 at 09:18:45AM +0900, Sergey Senozhatsky wrote:
Hello,
On (03/04/15 14:02), a...@linux-foundation.org wrote:
[..]
+++ a/drivers/block/zram/zram_drv.c
@@ -70,6 +70,27 @@ static inline struct zram *dev_to_zram(s
return (struct zram *)dev_to_disk(dev)-private_data;
}
+static ssize_t compact_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t len)
+{
+ unsigned long nr_migrated;
+ struct zram *zram = dev_to_zram(dev);
+ struct zram_meta *meta;
+
+ down_read(zram-init_lock);
+ if (!init_done(zram)) {
+ up_read(zram-init_lock);
+ return -EINVAL;
+ }
+
+ meta = zram-meta;
+ nr_migrated = zs_compact(meta-mem_pool);
+ atomic64_add(nr_migrated, zram-stats.num_migrated);
+ up_read(zram-init_lock);
+
+ return len;
+}
+
/* flag operations require table entry bit_spin_lock() being held */
static int zram_test_flag(struct zram_meta *meta, u32 index,
enum zram_pageflags flag)
let's stick to helpers, attrs show/store, mm (meta, page), IO, zram control
function layout.
so can we please put compact_store() after, say,
354 static ssize_t comp_algorithm_store(...)
function?
I will clean it up after Andrew releases new mmotm. :)
Thanks for the review!
--
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/
