From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751741AbbIJF52 (ORCPT ); Thu, 10 Sep 2015 01:57:28 -0400 Received: from mail-pa0-f41.google.com ([209.85.220.41]:34873 "EHLO mail-pa0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750741AbbIJF51 (ORCPT ); Thu, 10 Sep 2015 01:57:27 -0400 Date: Thu, 10 Sep 2015 14:58:14 +0900 From: Minchan Kim To: Sergey Senozhatsky Cc: Luis Henriques , Nitin Gupta , Andrew Morton , linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] zram: introduce comp algorithm fallback functionality Message-ID: <20150910055814.GA30419@bbox> References: <1441737776-25280-1-git-send-email-luis.henriques@canonical.com> <20150910050351.GB9562@bbox> <20150910053319.GB10843@swordfish> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150910053319.GB10843@swordfish> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Sep 10, 2015 at 02:33:19PM +0900, Sergey Senozhatsky wrote: > Hello, > > On (09/10/15 14:03), Minchan Kim wrote: > [..] > > > > I guess most of scripts have checked result of his doing so if we > > removes it, it will break them. > > to be honest, we never documented or required any of those. the only source > of information for the user space -- zram.txt documentation -- simply says > to do 'echo 3 > /sys/block/zram0/max_comp_streams' or any other bunch of > 'echo'-s. so, technically, a user is free to simply copy-paste what we do > in zram.txt to his zram.sh and call it a "recommended way of doing things > in zram". Agree. That's why we spend a lot discussion for this small change. > > besides, zram.txt is outdated. for example there is no mem_used_max > documentation. > > we need to do better job documenting. I'll try to take a look on this later > this week. Thanks. > > > > Given that, every success of "echo zzz > /sys/block/zram0/comp_algorithm" > > makes users to be confused that he might think to success to change algorithm > > in runtime. IOW, it should return error which is more intuitive forme. > > well, not quite like that. we return -EINVAL on invalid output since > d93435c3fba4a47b773693b0c8992470d38510d5. this patch does not change > anything from this pov. it does, however, change the behaviour of > disksize store that follows. I said like that you cut off. "Although we could change ABI of optional parameters into no failure smoothly" ^^^^^^^^^^ > > I'm fine when the motivation for this patch is to introduce the > "fallback" mechanism (like zswap fallbacks to default compressor, f.e.), > but it wasn't the case -- I rewrote the patch slightly, reworded the > commit message and put some reasoning to this patch. > > -ss