Re: [PATCHv5 0/4] add compressing abstraction and multi stream support

[Sergey Senozhatsky]

On (02/18/14 15:04), Minchan Kim wrote:
[..]
> 1.8.5.3
> 
> As you can see, your patch made zram too much regressed when it
> used one stream buffer. The reason I guessed is overhead of
> scheduling by sleep/wakeup when others couldn't find a idle stream
> so I had an experiment with below simple patch to restore old behavior
> so it works well again. The reason old behaivor was really good is
> it uses mutex with spin_on_owner so that it could avoid frequent
> sleep/wakeup.
> 
> A solution I could think is that we could grow up the number of
> buffer dynamically up to max_buffers(ex, 2 * number of CPU) so we
> could grab a idle stream easily for each CPU while we could release
> buffers by shrinker when the memory pressure is severe.
> Of course, we should keep one buffer to work.
> 
> Another solution I can imagaine is to define CONFIG_ZRAM_MULTI_STREAM
> and CONFIG_ZRAM_SINGLE_STREAM(ie, default) so we couldn't break good
> performance for old cases and new users who want write parallel
> could use ZRAM_MULTI which should be several streams at a default
> rather than a stream. And we could ehhacne it further by dynamic control
> as I mentioned.
> 
> Thoughts?
>

like the following one (this patch limits the number of streams to
num_online_cpus(). I really don't mind to limit zstrm to
N * num_online_cpus() where N could be 2)

	iozone -t 3 -R -r 16K -s 60M -I +Z

write test:
(old core i5 laptop, 2 cpus + 2 HyperThreading)

                       base            multi buffer

"  Initial write "   574938.14           716623.64
"        Rewrite "   573541.22          1499074.62
"   Random write "   587016.14          1393996.66
"         Pwrite "   595616.45           711028.70
"         Fwrite "  1482843.62          1493398.12


zcomp_strm_get() and zcomp_strm_put() were updated.
zcomp keeps the number allocated and still active (not freed)
streams (atomic_t). zcomp performs zstrm free() only if
current number of streams exceeds the number of online
cpus (cpu went offline).


not a properly `aligned' patch, just to demonstrate the idea.

thoughts?

thanks.

---