From patchwork Wed Mar 7 17:20:08 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Artem Bityutskiy X-Patchwork-Id: 145320 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from merlin.infradead.org (merlin.infradead.org [IPv6:2001:4978:20e::2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 31342B6F62 for ; Thu, 8 Mar 2012 04:19:00 +1100 (EST) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1S5KUo-00074i-Sw; Wed, 07 Mar 2012 17:17:50 +0000 Received: from mga14.intel.com ([143.182.124.37]) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1S5KUl-000740-JW for linux-mtd@lists.infradead.org; Wed, 07 Mar 2012 17:17:48 +0000 Received: from azsmga001.ch.intel.com ([10.2.17.19]) by azsmga102.ch.intel.com with ESMTP; 07 Mar 2012 09:17:37 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.71,315,1320652800"; d="scan'208,223";a="115964457" Received: from linux.jf.intel.com (HELO linux.intel.com) ([10.23.219.25]) by azsmga001.ch.intel.com with ESMTP; 07 Mar 2012 09:17:37 -0800 Received: from [10.237.72.167] (sauron.fi.intel.com [10.237.72.167]) by linux.intel.com (Postfix) with ESMTP id 1DECE2C8001; Wed, 7 Mar 2012 09:17:33 -0800 (PST) Message-ID: <1331140808.3463.28.camel@sauron.fi.intel.com> Subject: Re: ubi: suspicious calculation in 'ubi_wl_get_peb' From: Artem Bityutskiy To: Shmulik Ladkani , Richard Weinberger Date: Wed, 07 Mar 2012 19:20:08 +0200 In-Reply-To: <20120217153828.71eba4e4@pixies.home.jungo.com> References: <20120217153828.71eba4e4@pixies.home.jungo.com> X-Mailer: Evolution 3.2.3 (3.2.3-1.fc16) Mime-Version: 1.0 X-Spam-Note: CRM114 invocation failed X-Spam-Score: -5.0 (-----) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-5.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/, high trust [143.182.124.37 listed in list.dnswl.org] 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (dedekind1[at]gmail.com) 0.0 DKIM_ADSP_CUSTOM_MED No valid author signature, adsp_override is CUSTOM_MED 0.8 SPF_NEUTRAL SPF: sender does not match SPF record (neutral) 0.2 FREEMAIL_ENVFROM_END_DIGIT Envelope-from freemail username ends in digit (dedekind1[at]gmail.com) -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] 0.9 NML_ADSP_CUSTOM_MED ADSP custom_med hit, and not from a mailing list Cc: linux-mtd@lists.infradead.org X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: dedekind1@gmail.com List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-mtd-bounces@lists.infradead.org Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org On Fri, 2012-02-17 at 15:38 +0200, Shmulik Ladkani wrote: > - e = find_wl_entry(&ubi->free, medium_ec); > + e = find_wl_entry(&ubi->free, WL_FREE_MAX_DIFF/2) > > Did I get something wrong? Yeah, I think you are right. Now I am completely convinced we should remove this "short/long-term" stuff because this did even work correctly :-) CCint Richard - I happened to suggest the removal to him earlier today. What do you think about these untested fixes (also attached): From: Artem Bityutskiy Date: Wed, 7 Mar 2012 18:56:29 +0200 Subject: [PATCH 1/2] UBI: fix documentation and improve readability The "max" parameter of 'find_wl_entry()' was documented incorrectly and it actually means the maximum possible difference, not the maximum absolute value. Rename it to "diff" instead, and amend the documentation. Reported-by: Shmulik Ladkani Signed-off-by: Artem Bityutskiy Reviewed-by: Shmulik Ladkani Reviewed-by: Shmulik Ladkani --- drivers/mtd/ubi/wl.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-) From b874a16e244d21ce2e6b7d1ab81538f5b0a5968d Mon Sep 17 00:00:00 2001 Message-Id: In-Reply-To: <369897e5f49052f78dea2f02a498390cd2459d43.1331140361.git.artem.bityutskiy@linux.intel.com> References: <369897e5f49052f78dea2f02a498390cd2459d43.1331140361.git.artem.bityutskiy@linux.intel.com> From: Artem Bityutskiy Date: Wed, 7 Mar 2012 19:08:36 +0200 Subject: [PATCH 2/2] UBI: fix eraseblock picking criteria The 'find_wl_entry()' function expects the maximum difference as the second argument, not the maximum absolute value. So the "unknown" eraseblock picking was incorrect, as Shmulik Ladkani spotted. This patch fixes the issue. Reported-by: Shmulik Ladkani Signed-off-by: Artem Bityutskiy Cc: stable@kernel.org --- drivers/mtd/ubi/wl.c | 8 +++----- 1 files changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c index 10d7b98..051cb3a 100644 --- a/drivers/mtd/ubi/wl.c +++ b/drivers/mtd/ubi/wl.c @@ -390,7 +390,7 @@ static struct ubi_wl_entry *find_wl_entry(struct rb_root *root, int diff) */ int ubi_wl_get_peb(struct ubi_device *ubi, int dtype) { - int err, medium_ec; + int err; struct ubi_wl_entry *e, *first, *last; ubi_assert(dtype == UBI_LONGTERM || dtype == UBI_SHORTTERM || @@ -437,10 +437,8 @@ retry: if (last->ec - first->ec < WL_FREE_MAX_DIFF) e = rb_entry(ubi->free.rb_node, struct ubi_wl_entry, u.rb); - else { - medium_ec = (first->ec + WL_FREE_MAX_DIFF)/2; - e = find_wl_entry(&ubi->free, medium_ec); - } + else + e = find_wl_entry(&ubi->free, WL_FREE_MAX_DIFF/2); break; case UBI_SHORTTERM: /* -- 1.7.9.1