Re: [PATCH v4 7/7] zram: writeback throttle

[Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>]

On (12/03/18 14:50), Sergey Senozhatsky wrote:
> On (12/03/18 11:40), Minchan Kim wrote:
> [..]
> > +	down_read(&zram->init_lock);
> > +	atomic64_set(&zram->stats.bd_wb_limit, val);
> > +	if (val == 0)
> > +		zram->stop_writeback = false;
> > +	up_read(&zram->init_lock);
> 
> [..]
> 
> > +		if (zram->stop_writeback) {
> > +			ret = -EIO;
> > +			break;
> > +		}
> > +
> >  		if (!blk_idx) {
> >  			blk_idx = alloc_block_bdev(zram);
> >  			if (!blk_idx) {
> > @@ -694,6 +732,11 @@ static ssize_t writeback_store(struct device *dev,
> >  		zram_set_element(zram, index, blk_idx);
> >  		blk_idx = 0;
> >  		atomic64_inc(&zram->stats.pages_stored);
> > +		if (atomic64_add_unless(&zram->stats.bd_wb_limit,
> > +					-1 << (PAGE_SHIFT - 12), 0)) {
> > +			if (atomic64_read(&zram->stats.bd_wb_limit) == 0)
> > +				zram->stop_writeback = true;
> > +		}
> 
> Do we need ->stop_writeback? It should be identical to
> 
> 	atomic64_read(&zram->stats.bd_wb_limit) == 0

Seems like I misread writeback_limit_store() a bit.

So, if I want to, say, let only 10M of writteback pages, I need to
do

	echo 0 > writeback_limit
	echo 10M > writeback_limit_store	// memparse format is for
						// simplicity only; I know
						// it should be in 4K units.

every day. How about dropping the "echo 0" and ->stop_writeback?
So then we can just do

	echo 10M > writeback_limit_store

every day: if we have ->bd_wb_limit budget then we writeback,
           otherwise we don't.

	-ss