On Sat, Apr 16, 2011 at 02:03:09AM +0300, Zeev Tarantov wrote: > On Sat, Apr 16, 2011 at 01:31, Greg KH <greg@xxxxxxxxx> wrote: > > On Sat, Apr 16, 2011 at 12:45:41AM +0300, Zeev Tarantov wrote: > >> On Fri, Apr 15, 2011 at 05:21, Greg KH <greg@xxxxxxxxx> wrote: > >> > Why is this needed to be added to the kernel? What does it provide that > >> > users or other parts of the kernel needs? > >> > >> It is functionally a general data compression tool that trades off > >> compression ratio for speed. It is optimized for x86-64 and there > >> achieves compression at 250MB/sec and decompression at 500MB/sec > >> (YMMV*). This is a better mousetrap, that can and should replace LZO > >> in every place where the kernel currently uses LZO. > > > > Like where? > > > > Have you done so and found it really to be faster and smaller? If so, > > benchmarks and the numbers will be required for this to be accepted. > > Yes. In the email you replied to, I have linked benchmark results: > https://github.com/zeevt/csnappy/raw/master/zram_benchmark.txt You need it in the patch itself, no one looks at links anymore :) > The benchmark itself is simple: > https://github.com/zeevt/csnappy/raw/master/zramtest2.sh > > > You need to show a solid use case for why to switch to this code in > > order to have it accepted. > > It uses significantly less CPU time than LZO when doing the same work. > Is that a good enough reason to accept the code? To me, yes. > > >> The original code has been used in production by Google since 2005 or > >> so. It is stable and secure. Unless I've ruined it somehow when > >> porting, of course. > > > > It might be stable in userspace, that doesn't mean much for within the > > kernel though, please realize this. > > I realize this. But, the code is not multi threaded, has no floating > point, is reentrant, does not allocate memory, has no complex data > structures and uses kernel primitives for things like unaligned memory > and endianess. For example, a previous version worked great on ppc32 > qemu, which I think is not true for the entirety of the kernel. > > It is almost a drop-in replacement for LZO. The hook up into zram does > not violate any of the preconditions that make LZO safe to use with > zram. > > The code is small, I think. I have removed the streaming support since > the API only allows work with whole arrays. > If it makes review easier, I can cook up a series of patches that > demonstrate the evolution of the code from a simple version that > satisfies the bitstream format to the optimized version Google wrote > and I submitted. > If declaring variables in the middle of code is allowed, I think I can > make the code prettier. No, that's not allowed, sorry. > If CamelCase names for static functions are too irritating, I will change them. It's annoying, but you probably want to keep it close to the original code for a while to handle bugs that are fixed in the upstream version, right? Anyway, I'll let the zram developers review this as it is their code it touches. thanks, greg k-h _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
