From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752292AbbIHBc6 (ORCPT ); Mon, 7 Sep 2015 21:32:58 -0400 Received: from mail-pa0-f45.google.com ([209.85.220.45]:36780 "EHLO mail-pa0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751314AbbIHBcz (ORCPT ); Mon, 7 Sep 2015 21:32:55 -0400 Date: Tue, 8 Sep 2015 10:33:38 +0900 From: Sergey Senozhatsky To: Minchan Kim Cc: Sergey Senozhatsky , Luis Henriques , linux-kernel@vger.kernel.org, Sergey Senozhatsky Subject: Re: [PATCH] zram: don't copy invalid compression algorithms Message-ID: <20150908013338.GF6896@swordfish> References: <1441658910-10226-1-git-send-email-luis.henriques@canonical.com> <20150907235635.GA6896@swordfish> <20150908011443.GB19776@bbox> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150908011443.GB19776@bbox> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On (09/08/15 10:14), Minchan Kim wrote: [..] > > NACK. > > > > This is intentional. We haven't returned 'invalid compression algorithm' > > error from comp_algorithm_store() historically, so someone's script can > > simply ignore it. However, the script will fail to init the device and > > user will be able to figure out the root cause, because zram will report > > to syslog an actually requested alg name. > > > > Example > > > > [ 1669.473296] zram: Cannot initialise llzo compressing backend > > I don't understand your concern. To me, this patch makes sense to me. > Could you explain your point clearly, again? OK. suppose someone landed a typo in a 'zram device management' script echo llzo > /sys/block/zram0/comp_algorithm -bash: echo: write error: Invalid argument but the script ignores 'echo: write error'. Because we added compression algorithm name check recently. then the script does echo 200M > /sys/block/zram0/disksize -bash: echo: write error: Invalid argument doing a simple dmesg reveals the problem [ 7076.657184] zram: Cannot initialise llzo compressing backend note that zram provides 'llzo' here, which is convenient. With this change the semantics is changing and zram now swallows (hides) the user space error. echo llzo > /sys/block/zram0/comp_algorithm -bash: echo: write error: Invalid argument echo 200M > /sys/block/zram0/disksize instead of using a requested 'llzo' zram for some reason switched to 'lzo'. I don't like this behaviour change. User requested to change the 'default' value, and that new value didn't work out. No reason to hide it. -ss