On (24/05/09 05:43), Christoph Hellwig wrote: > On Wed, May 08, 2024 at 04:41:53PM +0900, Sergey Senozhatsky wrote: > > This patch set moves zram from crypto API to a custom compression > > API which allows us to tune and configure compression algorithms, > > something that crypto API, unfortunately, doesn't support. > > [...] > > > 21 files changed, 1203 insertions(+), 111 deletions(-) > > Why can't it? Well, I asked crypto folks if that's doable and the only reply was "did you try using compression libs directly". And that's not a bad response, I take it. The handling of parameters becomes quite intrusive very quickly. It's not as simple as just passing a new "struct crypto_tfm" to all sort of API abstractions that crypto has, it's a little more than that. Just as an example. For zstd we can work in two modes 1) load the dictionary by_copy 2) load the dictionary by_ref In (2) we need to guarantee that the dictionary memory outlives any comp contexts, so cyrpto_tfm-s now begin to have "external" dependency. But if we load the dictionary by_ref then what we can do is a pre-processing of the dictionary buffer - we get CDict and DDict pointers (specific only to zstd backend) which all contexts now can share (contexts access C/D Dict in read-only mode). For this we need to have a pre-processing stage somewhere in the API and keep the "compression's backend private data" somewhere, then somehow pass it to context cra_init and release that memory when all context were destroyed. In zram I just went with "we do only by_ref" and handle all the dependencies/guarantees, it's very simple because all of this stays in zram. But in general case, a typical crypto API usage tfm = crypto_alloc_comp(comp->name, 0, 0); should become much more complex. I'd say that, probably, developing an entirely new sub-set of API would be simpler. So I implemented a simple zram comp API. I can't tell how much effort it'll be to handle all of this in crypto, I'm not really familiar with crypto, and I'm not sure if crypto API folks are even interested. > This is an awful lot of crazy code duplication just > to pass a few parameters. I see what you mean, but the majority of the code is unique, there isn't too much code duplication in fact. Params handling is unique, dictionary handling is unique, zstd implementation is entirely different and pretty much specific to zram (we don't handle all sort of cases that zstd API support, we focus on things that we need), lz4/lz4hc implementations are also different, etc. etc. Things like lzo/lzorle may count as code duplication, but those are like 20 lines of code or maybe even less (which isn't that crazy).
