linux-kernel - Re: [PATCH 1/2] UBI: Kill data type hint

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1337015186.2528.87.camel@sauron.fi.intel.com>
Date:	Mon, 14 May 2012 20:06:26 +0300
From:	Artem Bityutskiy <dedekind1@...il.com>
To:	Richard Weinberger <richard@....at>
Cc:	linux-mtd@...ts.infradead.org, tglx@...utronix.de,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] UBI: Kill data type hint

On Mon, 2012-05-14 at 17:55 +0200, Richard Weinberger wrote:
> We do not need this feature and to our shame it even was not working
> and there was a bug found very recently.
> 	-- Artem Bityutskiy
> 
> Without the data type hint UBI2 (fastmap) will be easier to implement.
> 
> Signed-off-by: Richard Weinberger <richard@....at>

Pushed to l2-mtd.git with a minor checkpatch.pl amendment. I've also
found few stale references to "short term" and the like in the comments,
as well as improved "dtype" description in ubi-user.h, so I've pushed
the following patch on top of yours.


Thanks!


From 85ab49c3a38f68a20265c116971863e35d97ae66 Mon Sep 17 00:00:00 2001
From: Artem Bityutskiy <artem.bityutskiy@...ux.intel.com>
Date: Mon, 14 May 2012 19:49:35 +0300
Subject: [PATCH] UBI: amend commentaries WRT dtype

Richard removed the "dtype" hint, but few commentaries were left and this patch
removes them. I've also added a better description about the "dtype" field in
the ubi-user.h for people who may ever wonder what was that dtype thing about.

This patch also adds an important note that it is better to use value "3" for
the "dtype" field.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@...ux.intel.com>
---
 drivers/mtd/ubi/wl.c   |    9 +--------
 fs/ubifs/super.c       |    5 ++---
 include/mtd/ubi-user.h |   11 +++++++++++
 3 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
index f0bc107..64ce993 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -41,12 +41,6 @@
  * physical eraseblocks with low erase counter to free physical eraseblocks
  * with high erase counter.
  *
- * The 'ubi_wl_get_peb()' function accepts data type hints which help to pick
- * an "optimal" physical eraseblock. For example, when it is known that the
- * physical eraseblock will be "put" soon because it contains short-term data,
- * the WL sub-system may pick a free physical eraseblock with low erase
- * counter, and so forth.
- *
  * If the WL sub-system fails to erase a physical eraseblock, it marks it as
  * bad.
  *
@@ -70,8 +64,7 @@
  *    to the user; instead, we first want to let users fill them up with data;
  *
  *  o there is a chance that the user will put the physical eraseblock very
- *    soon, so it makes sense not to move it for some time, but wait; this is
- *    especially important in case of "short term" physical eraseblocks.
+ *    soon, so it makes sense not to move it for some time, but wait.
  *
  * Physical eraseblocks stay protected only for limited time. But the "time" is
  * measured in erase cycles in this case. This is implemented with help of the
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 7a3ce9f..34e903f 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -814,9 +814,8 @@ static int alloc_wbufs(struct ubifs_info *c)
 	}
 
 	/*
-	 * Garbage Collector head likely contains long-term data and
-	 * does not need to be synchronized by timer. Also GC head nodes are
-	 * not grouped.
+	 * Garbage Collector head does not need to be synchronized by timer.
+	 * Also GC head nodes are not grouped.
 	 */
 	c->jheads[GCHD].wbuf.no_timer = 1;
 	c->jheads[GCHD].grouped = 0;
diff --git a/include/mtd/ubi-user.h b/include/mtd/ubi-user.h
index 370c750..8787349 100644
--- a/include/mtd/ubi-user.h
+++ b/include/mtd/ubi-user.h
@@ -358,7 +358,17 @@ struct ubi_rnvol_req {
  *                             requests.
  * @lnum: logical eraseblock number to change
  * @bytes: how many bytes will be written to the logical eraseblock
+ * @dtype: pass "3" for better compatibility with old kernels
  * @padding: reserved for future, not used, has to be zeroed
+ *
+ * The @dtype field used to inform UBI about what kind of data will be written
+ * to the LEB: long term (value 1), short term (value 2), unknown (value 3).
+ * UBI tried to pick a PEB with lower erase counter for short term data and a
+ * PEB with higher erase counter for long term data. But this was not really
+ * used because users usually do not know this and could easily mislead UBI. We
+ * removed this feature in May 2012. UBI currently just ignores the @dtype
+ * field. But for better compatibility with older kernels it is recommended to
+ * set @dtype to 3 (unknown).
  */
 struct ubi_leb_change_req {
 	__s32 lnum;
@@ -369,6 +379,7 @@ struct ubi_leb_change_req {
 
 /**
  * struct ubi_map_req - a data structure used in map LEB requests.
+ * @dtype: pass "3" for better compatibility with old kernels
  * @lnum: logical eraseblock number to unmap
  * @padding: reserved for future, not used, has to be zeroed
  */
-- 
1.7.10

-- 
Best Regards,
Artem Bityutskiy

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)