From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752511AbbE0FxC (ORCPT ); Wed, 27 May 2015 01:53:02 -0400 Received: from mail-pd0-f172.google.com ([209.85.192.172]:33253 "EHLO mail-pd0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751585AbbE0FxA (ORCPT ); Wed, 27 May 2015 01:53:00 -0400 Date: Wed, 27 May 2015 14:53:20 +0900 From: Sergey Senozhatsky To: Minchan Kim Cc: Sergey Senozhatsky , Andrew Morton , Marcin Jabrzyk , Nitin Gupta , linux-kernel@vger.kernel.org, Sergey Senozhatsky Subject: Re: [PATCH] zram: check comp algorithm availability earlier Message-ID: <20150527055320.GA3928@swordfish> References: <1432646017-1367-1-git-send-email-sergey.senozhatsky@gmail.com> <20150527035142.GA11609@blaptop> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150527035142.GA11609@blaptop> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On (05/27/15 12:51), Minchan Kim wrote: [..] > > @@ -378,6 +378,12 @@ static ssize_t comp_algorithm_store(struct device *dev, > > if (sz > 0 && zram->compressor[sz - 1] == '\n') > > zram->compressor[sz - 1] = 0x00; > > > > + if (!zcomp_available_algorithm(zram->compressor)) { > > + pr_err("Error: unavailable compression algorithm: %s\n", > > + zram->compressor); > > + len = -EINVAL; > > + } > > + > > I'm not against this patch because it's better than old. > But let's think more about the pr_err part. > > If user try to set wrong algo name, he can see EINVAL. > Isn't it enough? > > I think every sane admin can think he passed wrong argument > if he sees -EINVAL. > So, I don't think we need to emit pr_err in here. > well, it's here simply to make failure investigation easier. one surely will know that supplied string was not recognized as a compression algorithm name, but what was it.. "$3 instead of $2... or, wait, did $i contain something wrong?". zram knew what was wrong. /* and you asked to put this warn here in your previous email. */ sure, can remove it. > The reason I am paranoid about that is that I really don't want > to argue with syslog info which is part of ABI or not in future. > If possible, I don't want to depend on pr_xxx. > just for the record... I don't understand this part. ok. I'll resend later today. -ss