Re: [PATCH] zram: export the number of available comp streams

[Minchan Kim]

On Fri, Mar 18, 2016 at 10:09:37AM +0900, Sergey Senozhatsky wrote:
> Hello Minchan,
> 
> On (03/18/16 09:32), Minchan Kim wrote:
> [..]
> > > do I need 21? may be no. do I nede 18? if 18 streams are needed only 10%
> > > of the time (I can figure it out by doing repetitive cat zramX/mm_stat),
> > > then I can set max_comp_streams to make 90% of applications happy, e.g.
> > > max_comp_streams to 10, and save some memory.
> > > 
> > 
> > Okay. Let's go back to zcomp design decade. As you remember, the reason
> > we separated single and multi stream code was performance caused by
> > locking scheme(ie, mutex_lock in single stream model was really fast
> > than sleep/wakeup model in multi stream).
> > If we could overcome that problem back then, we should have gone to
> > multi stream code default.
> 
> yes, IIRC I wanted to limit the number of streams by the number of
> online CPUs (or was it 2*num_online_cpus()?), and thus change the
> number of streams dynamically (because CPUs can go on and off line);
> and create at least num_online_cpus() streams during device
> initialization.
> 
> the reason for a single-stream zram IIRC were setups in which
> zram is used as a swap device. streams require some memory, after
> all. and then we discovered that mutex spin on owner boosts single
> stream zram significantly.
> 
> > How about using *per-cpu* streams?
> 
> OK. instead of list of idle streams use per-cpu pointer and process
> CPU_FOO notifications. that can work, sounds good to me.
> 
> 
> > I remember you wanted to create max number of comp streams statically
> > although I didn't want at that time but I change my decision.
> > 
> > Let's allocate comp stream statically but remove max_comp_streams
> > knob. Instead, by default, zram alloctes number of streams according
> > to the number of online CPU.
> 
> OK. removing `max_comp_streams' will take another 2 years. That's
> a major change, we can leave it for longer, just make it nop.
> 
> > So I think we can solve locking scheme issue in single stream
> > , guarantee parallel level as well as enhancing performance with
> > no locking.
> >
> > Downside with the approach is that unnecessary memory space reserve
> > although zram might be used 1% of running system time. But we
> > should give it up for other benefits
> 
> aha, ok.
> 
> > (ie, simple code, removing
> > max_comp_streams knob, no need to this your stat, guarantee parallel
> > level, guarantee consumed memory space).
> 
> I'll take a look and prepare some numbers (most likely next week).

Sounds great to me!

> 
> 
> > What do you think about it?
> 
> so should I ask Andrew to drop this patch?

Yeb.

Thanks!

> 
> 	-ss