From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758531AbcFAPEK (ORCPT ); Wed, 1 Jun 2016 11:04:10 -0400 Received: from mail-io0-f194.google.com ([209.85.223.194]:35027 "EHLO mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758324AbcFAPEG (ORCPT ); Wed, 1 Jun 2016 11:04:06 -0400 Subject: Re: [PATCH v2 4/8] zram: use crypto api to check alg availability To: Sergey Senozhatsky , Minchan Kim References: <20160531122017.2878-1-sergey.senozhatsky@gmail.com> <20160531122017.2878-5-sergey.senozhatsky@gmail.com> <20160601000304.GF19976@bbox> <20160601010707.GB461@swordfish> <20160601022708.GL19976@bbox> <20160601025236.GG461@swordfish> <20160601064709.GM19976@bbox> <20160601074845.GA491@swordfish> Cc: Sergey Senozhatsky , Andrew Morton , Joonsoo Kim , linux-kernel@vger.kernel.org From: "Austin S. Hemmelgarn" Message-ID: <69f3a8eb-ef8a-d80c-464a-053fb1acfbfd@gmail.com> Date: Wed, 1 Jun 2016 10:59:48 -0400 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 MIME-Version: 1.0 In-Reply-To: <20160601074845.GA491@swordfish> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016-06-01 03:48, Sergey Senozhatsky wrote: > On (06/01/16 15:47), Minchan Kim wrote: > [..] >>> so both BUILTIN and BUILT-AS-A-MODULE cases are handled at compile >>> time now and we can avoid crypto_has_comp() checks for most of the >>> comp_algorithm calls, except for the case when someone requests an >>> out-of-tree module. >> >> Hmm, isn't it problem, either? >> >> That module was built but not installed. In that case, setting the >> algorithm will be failed. IOW, we are lying to user. > > have you ever seen this? really, why should we even bother? > if there is no requested algorithm we will fallback to LZO. > > and how is that different from: user enabled LZO in .config (because it's > a prerequisite for zram) but forgot to install the module? do we have to > "fix" this as well?... implement our own LZO compression in zram? > or `cp lib/lzo/* drivers/block/zram/'? Ideally, it should fall back to whatever algorithm it can find that is supported, preferably in the following order: lzo lz4 lz4hc deflate 842 LZO first will keep backwards compatibility, while the rest is roughly in decreasing order of performance on most hardware (except on PPC systems which have hardware support for 842 compression). Handling not being able to find any algorithm gets trickier, and the choices are pretty much use a null algorithm and just store flat data, or refuse to store anything. > >> For solving the problem, if we check it with crypto_has_comp, again, >> it will load module into memory. :( > > this will require a *VERY* non-standard behaviour from user > > cat /sys/block/zram0/comp_algorithm > [lzo] lz4 > # um... > echo 842 > /sys/block/zram0/comp_algorithm > > and I'm quite confident that anyone who does this actually want > to init the device with the requested out-of-tree module right > after `echo FOO > comp_algorithm', rather than anything else. Just from the perspective of a system administrator, most people probably aren't going to be directly touching the sysfs entries themselves except possibly for testing, and I don't think I've ever seen anything that actually reads zram/comp_algorithm except to verify that it's set to the requested algorithm. Given that, the behavior I'd expect from zram/comp_algorithm as an administrator would be: 1. List the currently used algorithm together with all algorithm's supported in the mainline kernel. 2. If somebody writes an algorithm we don't know about, check it with crypto_has_comp, and switch to it if successful. 3. Cache positive lookups of unknown algorithms so that you don't have to check again on other devices, and list those algorithms in zram/comp_algorithm even if they're not being used. This would provide relative compatibility with the current behavior, while still allowing people using unknown compression modules to use them.