[PATCH MTD-UTILS 0/5] sync with 3.6-rc1 and introduce max_beb_per1024 in UBI_IOCATT

[PATCH MTD-UTILS 0/5] sync with 3.6-rc1 and introduce max_beb_per1024 in UBI_IOCATT

[Richard Genoud]

Hi Artem,
Here are the patches for the introduction of max_beb_per1024 in ubiattach.
The first patch is just a sync of ubi-user.h with the vanilla 3.6-rc1
(So after this patch, the mtd-utils won't build any more)
The second one supresses dtype parameter, as it is obsolete now.
The third syncs with somes defines and struct renames in the ubi-user.h
(Now it compiles again)
The fourth syncs ubi-user.h with the one in linux-ubi/master
And finally the last one instroduce max_beb_per1024.

Best Regards,
Richard.

Richard Genoud (5):
  sync include/mtd/ubi-user.h with kernel v3.6-rc1
  dtype parameter is obsolete
  re-name set volume properties ioctl
  sync include/mtd/ubi-user.h: add max_beb_per1024 parameter
  ubiattach: introduce max_beb_per1024 in UBI_IOCATT

 include/mtd/ubi-user.h             |  152 +++++++++++++++++++-----------------
 mkfs.ubifs/lpt.c                   |   10 +-
 mkfs.ubifs/mkfs.ubifs.c            |   37 ++++-----
 mkfs.ubifs/mkfs.ubifs.h            |    2 +-
 tests/fs-tests/integrity/integck.c |    1 +
 tests/ubi-tests/io_paral.c         |    2 +-
 ubi-utils/include/libubi.h         |    7 +-
 ubi-utils/libubi.c                 |   11 ++-
 ubi-utils/ubiattach.c              |   38 +++++++--
 9 files changed, 145 insertions(+), 115 deletions(-)

-- 
1.7.2.5

[PATCH MTD-UTILS 1/5] sync include/mtd/ubi-user.h with kernel v3.6-rc1

[Richard Genoud]

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 include/mtd/ubi-user.h |  138 +++++++++++++++++++++++-------------------------
 1 files changed, 67 insertions(+), 71 deletions(-)

diff --git a/include/mtd/ubi-user.h b/include/mtd/ubi-user.h
index 296efae..123951f 100644
--- a/include/mtd/ubi-user.h
+++ b/include/mtd/ubi-user.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) International Business Machines Corp., 2006
+ * Copyright © International Business Machines Corp., 2006
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -21,6 +21,8 @@
 #ifndef __UBI_USER_H__
 #define __UBI_USER_H__
 
+#include <linux/types.h>
+
 /*
  * UBI device creation (the same as MTD device attachment)
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -129,7 +131,7 @@
  * ~~~~~~~~~~~~~~~~~~~~~~~~~
  *
  * To set an UBI volume property the %UBI_IOCSETPROP ioctl command should be
- * used. A pointer to a &struct ubi_set_prop_req object is expected to be
+ * used. A pointer to a &struct ubi_set_vol_prop_req object is expected to be
  * passed. The object describes which property should be set, and to which value
  * it should be set.
  */
@@ -152,7 +154,7 @@
 /* Create an UBI volume */
 #define UBI_IOCMKVOL _IOW(UBI_IOC_MAGIC, 0, struct ubi_mkvol_req)
 /* Remove an UBI volume */
-#define UBI_IOCRMVOL _IOW(UBI_IOC_MAGIC, 1, int32_t)
+#define UBI_IOCRMVOL _IOW(UBI_IOC_MAGIC, 1, __s32)
 /* Re-size an UBI volume */
 #define UBI_IOCRSVOL _IOW(UBI_IOC_MAGIC, 2, struct ubi_rsvol_req)
 /* Re-name volumes */
@@ -165,26 +167,27 @@
 /* Attach an MTD device */
 #define UBI_IOCATT _IOW(UBI_CTRL_IOC_MAGIC, 64, struct ubi_attach_req)
 /* Detach an MTD device */
-#define UBI_IOCDET _IOW(UBI_CTRL_IOC_MAGIC, 65, int32_t)
+#define UBI_IOCDET _IOW(UBI_CTRL_IOC_MAGIC, 65, __s32)
 
 /* ioctl commands of UBI volume character devices */
 
 #define UBI_VOL_IOC_MAGIC 'O'
 
 /* Start UBI volume update */
-#define UBI_IOCVOLUP _IOW(UBI_VOL_IOC_MAGIC, 0, int64_t)
+#define UBI_IOCVOLUP _IOW(UBI_VOL_IOC_MAGIC, 0, __s64)
 /* LEB erasure command, used for debugging, disabled by default */
-#define UBI_IOCEBER _IOW(UBI_VOL_IOC_MAGIC, 1, int32_t)
+#define UBI_IOCEBER _IOW(UBI_VOL_IOC_MAGIC, 1, __s32)
 /* Atomic LEB change command */
-#define UBI_IOCEBCH _IOW(UBI_VOL_IOC_MAGIC, 2, int32_t)
+#define UBI_IOCEBCH _IOW(UBI_VOL_IOC_MAGIC, 2, __s32)
 /* Map LEB command */
 #define UBI_IOCEBMAP _IOW(UBI_VOL_IOC_MAGIC, 3, struct ubi_map_req)
 /* Unmap LEB command */
-#define UBI_IOCEBUNMAP _IOW(UBI_VOL_IOC_MAGIC, 4, int32_t)
+#define UBI_IOCEBUNMAP _IOW(UBI_VOL_IOC_MAGIC, 4, __s32)
 /* Check if LEB is mapped command */
-#define UBI_IOCEBISMAP _IOR(UBI_VOL_IOC_MAGIC, 5, int32_t)
+#define UBI_IOCEBISMAP _IOR(UBI_VOL_IOC_MAGIC, 5, __s32)
 /* Set an UBI volume property */
-#define UBI_IOCSETPROP _IOW(UBI_VOL_IOC_MAGIC, 6, struct ubi_set_prop_req)
+#define UBI_IOCSETVOLPROP _IOW(UBI_VOL_IOC_MAGIC, 6, \
+			       struct ubi_set_vol_prop_req)
 
 /* Maximum MTD device name length supported by UBI */
 #define MAX_UBI_MTD_NAME_LEN 127
@@ -193,23 +196,6 @@
 #define UBI_MAX_RNVOL 32
 
 /*
- * UBI data type hint constants.
- *
- * UBI_LONGTERM: long-term data
- * UBI_SHORTTERM: short-term data
- * UBI_UNKNOWN: data persistence is unknown
- *
- * These constants are used when data is written to UBI volumes in order to
- * help the UBI wear-leveling unit to find more appropriate physical
- * eraseblocks.
- */
-enum {
-	UBI_LONGTERM  = 1,
-	UBI_SHORTTERM = 2,
-	UBI_UNKNOWN   = 3,
-};
-
-/*
  * UBI volume type constants.
  *
  * @UBI_DYNAMIC_VOLUME: dynamic volume
@@ -221,13 +207,14 @@ enum {
 };
 
 /*
- * UBI set property ioctl constants
+ * UBI set volume property ioctl constants.
  *
- * @UBI_PROP_DIRECT_WRITE: allow / disallow user to directly write and
- *                         erase individual eraseblocks on dynamic volumes
+ * @UBI_VOL_PROP_DIRECT_WRITE: allow (any non-zero value) or disallow (value 0)
+ *                             user to directly write and erase individual
+ *                             eraseblocks on dynamic volumes
  */
 enum {
-       UBI_PROP_DIRECT_WRITE = 1,
+	UBI_VOL_PROP_DIRECT_WRITE = 1,
 };
 
 /**
@@ -260,10 +247,10 @@ enum {
  * sub-page of the first page and add needed padding.
  */
 struct ubi_attach_req {
-	int32_t ubi_num;
-	int32_t mtd_num;
-	int32_t vid_hdr_offset;
-	int8_t padding[12];
+	__s32 ubi_num;
+	__s32 mtd_num;
+	__s32 vid_hdr_offset;
+	__s8 padding[12];
 };
 
 /**
@@ -298,15 +285,15 @@ struct ubi_attach_req {
  * BLOBs, without caring about how to properly align them.
  */
 struct ubi_mkvol_req {
-	int32_t vol_id;
-	int32_t alignment;
-	int64_t bytes;
-	int8_t vol_type;
-	int8_t padding1;
-	int16_t name_len;
-	int8_t padding2[4];
+	__s32 vol_id;
+	__s32 alignment;
+	__s64 bytes;
+	__s8 vol_type;
+	__s8 padding1;
+	__s16 name_len;
+	__s8 padding2[4];
 	char name[UBI_MAX_VOLUME_NAME + 1];
-} __attribute__ ((packed));
+} __attribute__((packed));
 
 /**
  * struct ubi_rsvol_req - a data structure used in volume re-size requests.
@@ -320,9 +307,9 @@ struct ubi_mkvol_req {
  * zero number of bytes).
  */
 struct ubi_rsvol_req {
-	int64_t bytes;
-	int32_t vol_id;
-} __attribute__ ((packed));
+	__s64 bytes;
+	__s32 vol_id;
+} __attribute__((packed));
 
 /**
  * struct ubi_rnvol_req - volumes re-name request.
@@ -356,55 +343,64 @@ struct ubi_rsvol_req {
  * re-name request.
  */
 struct ubi_rnvol_req {
-	int32_t count;
-	int8_t padding1[12];
+	__s32 count;
+	__s8 padding1[12];
 	struct {
-		int32_t vol_id;
-		int16_t name_len;
-		int8_t  padding2[2];
+		__s32 vol_id;
+		__s16 name_len;
+		__s8  padding2[2];
 		char    name[UBI_MAX_VOLUME_NAME + 1];
 	} ents[UBI_MAX_RNVOL];
-} __attribute__ ((packed));
+} __attribute__((packed));
 
 /**
  * struct ubi_leb_change_req - a data structure used in atomic LEB change
  *                             requests.
  * @lnum: logical eraseblock number to change
  * @bytes: how many bytes will be written to the logical eraseblock
- * @dtype: data type (%UBI_LONGTERM, %UBI_SHORTTERM, %UBI_UNKNOWN)
+ * @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 {
-	int32_t lnum;
-	int32_t bytes;
-	int8_t  dtype;
-	int8_t  padding[7];
-} __attribute__ ((packed));
+	__s32 lnum;
+	__s32 bytes;
+	__s8  dtype; /* obsolete, do not use! */
+	__s8  padding[7];
+} __attribute__((packed));
 
 /**
  * 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
- * @dtype: data type (%UBI_LONGTERM, %UBI_SHORTTERM, %UBI_UNKNOWN)
  * @padding: reserved for future, not used, has to be zeroed
  */
 struct ubi_map_req {
-	int32_t lnum;
-	int8_t  dtype;
-	int8_t  padding[3];
-} __attribute__ ((packed));
+	__s32 lnum;
+	__s8  dtype; /* obsolete, do not use! */
+	__s8  padding[3];
+} __attribute__((packed));
 
 
 /**
- * struct ubi_set_prop_req - a data structure used to set an ubi volume
- *                           property.
- * @property: property to set (%UBI_PROP_DIRECT_WRITE)
+ * struct ubi_set_vol_prop_req - a data structure used to set an UBI volume
+ *                               property.
+ * @property: property to set (%UBI_VOL_PROP_DIRECT_WRITE)
  * @padding: reserved for future, not used, has to be zeroed
  * @value: value to set
  */
-struct ubi_set_prop_req {
-       uint8_t  property;
-       uint8_t  padding[7];
-       uint64_t value;
-}  __attribute__ ((packed));
+struct ubi_set_vol_prop_req {
+	__u8  property;
+	__u8  padding[7];
+	__u64 value;
+}  __attribute__((packed));
 
 #endif /* __UBI_USER_H__ */
-- 
1.7.2.5

[PATCH MTD-UTILS 2/5] dtype parameter is obsolete

[Richard Genoud]

kernel commit a65a0eb6d198e058687a9214683bd1c418f20d39 set the dtype
parameter as obsolete.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 mkfs.ubifs/lpt.c           |   10 +++++-----
 mkfs.ubifs/mkfs.ubifs.c    |   35 +++++++++++++++--------------------
 mkfs.ubifs/mkfs.ubifs.h    |    2 +-
 ubi-utils/include/libubi.h |    3 +--
 ubi-utils/libubi.c         |    3 +--
 5 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/mkfs.ubifs/lpt.c b/mkfs.ubifs/lpt.c
index 60002ff..f6d4352 100644
--- a/mkfs.ubifs/lpt.c
+++ b/mkfs.ubifs/lpt.c
@@ -410,7 +410,7 @@ int create_lpt(struct ubifs_info *c)
 			alen = ALIGN(len, c->min_io_size);
 			set_ltab(c, lnum, c->leb_size - alen, alen - len);
 			memset(p, 0xff, alen - len);
-			err = write_leb(lnum++, alen, buf, UBI_SHORTTERM);
+			err = write_leb(lnum++, alen, buf);
 			if (err)
 				goto out;
 			p = buf;
@@ -452,7 +452,7 @@ int create_lpt(struct ubifs_info *c)
 				set_ltab(c, lnum, c->leb_size - alen,
 					    alen - len);
 				memset(p, 0xff, alen - len);
-				err = write_leb(lnum++, alen, buf, UBI_SHORTTERM);
+				err = write_leb(lnum++, alen, buf);
 				if (err)
 					goto out;
 				p = buf;
@@ -499,7 +499,7 @@ int create_lpt(struct ubifs_info *c)
 			alen = ALIGN(len, c->min_io_size);
 			set_ltab(c, lnum, c->leb_size - alen, alen - len);
 			memset(p, 0xff, alen - len);
-			err = write_leb(lnum++, alen, buf, UBI_SHORTTERM);
+			err = write_leb(lnum++, alen, buf);
 			if (err)
 				goto out;
 			p = buf;
@@ -522,7 +522,7 @@ int create_lpt(struct ubifs_info *c)
 		alen = ALIGN(len, c->min_io_size);
 		set_ltab(c, lnum, c->leb_size - alen, alen - len);
 		memset(p, 0xff, alen - len);
-		err = write_leb(lnum++, alen, buf, UBI_SHORTTERM);
+		err = write_leb(lnum++, alen, buf);
 		if (err)
 			goto out;
 		p = buf;
@@ -542,7 +542,7 @@ int create_lpt(struct ubifs_info *c)
 
 	/* Write remaining buffer */
 	memset(p, 0xff, alen - len);
-	err = write_leb(lnum, alen, buf, UBI_SHORTTERM);
+	err = write_leb(lnum, alen, buf);
 	if (err)
 		goto out;
 
diff --git a/mkfs.ubifs/mkfs.ubifs.c b/mkfs.ubifs/mkfs.ubifs.c
index bb25dc3..751e348 100644
--- a/mkfs.ubifs/mkfs.ubifs.c
+++ b/mkfs.ubifs/mkfs.ubifs.c
@@ -774,16 +774,15 @@ static void prepare_node(void *node, int len)
  * @lnum: LEB number
  * @len: length of data in the buffer
  * @buf: buffer (must be at least c->leb_size bytes)
- * @dtype: expected data type
  */
-int write_leb(int lnum, int len, void *buf, int dtype)
+int write_leb(int lnum, int len, void *buf)
 {
 	off64_t pos = (off64_t)lnum * c->leb_size;
 
 	dbg_msg(3, "LEB %d len %d", lnum, len);
 	memset(buf + len, 0xff, c->leb_size - len);
 	if (out_ubi)
-		if (ubi_leb_change_start(ubi, out_fd, lnum, c->leb_size, dtype))
+		if (ubi_leb_change_start(ubi, out_fd, lnum, c->leb_size))
 			return sys_err_msg("ubi_leb_change_start failed");
 
 	if (lseek64(out_fd, pos, SEEK_SET) != pos)
@@ -800,11 +799,10 @@ int write_leb(int lnum, int len, void *buf, int dtype)
 /**
  * write_empty_leb - copy the image of an empty LEB to the output target.
  * @lnum: LEB number
- * @dtype: expected data type
  */
-static int write_empty_leb(int lnum, int dtype)
+static int write_empty_leb(int lnum)
 {
-	return write_leb(lnum, 0, leb_buf, dtype);
+	return write_leb(lnum, 0, leb_buf);
 }
 
 /**
@@ -851,9 +849,8 @@ static int do_pad(void *buf, int len)
  * @node: node
  * @len: node length
  * @lnum: LEB number
- * @dtype: expected data type
  */
-static int write_node(void *node, int len, int lnum, int dtype)
+static int write_node(void *node, int len, int lnum)
 {
 	prepare_node(node, len);
 
@@ -861,7 +858,7 @@ static int write_node(void *node, int len, int lnum, int dtype)
 
 	len = do_pad(leb_buf, len);
 
-	return write_leb(lnum, len, leb_buf, dtype);
+	return write_leb(lnum, len, leb_buf);
 }
 
 /**
@@ -973,7 +970,7 @@ static int flush_nodes(void)
 	if (!head_offs)
 		return 0;
 	len = do_pad(leb_buf, head_offs);
-	err = write_leb(head_lnum, len, leb_buf, UBI_UNKNOWN);
+	err = write_leb(head_lnum, len, leb_buf);
 	if (err)
 		return err;
 	set_lprops(head_lnum, head_offs, head_flags);
@@ -1901,7 +1898,7 @@ static int set_gc_lnum(void)
 	int err;
 
 	c->gc_lnum = head_lnum++;
-	err = write_empty_leb(c->gc_lnum, UBI_LONGTERM);
+	err = write_empty_leb(c->gc_lnum);
 	if (err)
 		return err;
 	set_lprops(c->gc_lnum, 0, 0);
@@ -1977,7 +1974,7 @@ static int write_super(void)
 	if (c->space_fixup)
 		sup.flags |= cpu_to_le32(UBIFS_FLG_SPACE_FIXUP);
 
-	return write_node(&sup, UBIFS_SB_NODE_SZ, UBIFS_SB_LNUM, UBI_LONGTERM);
+	return write_node(&sup, UBIFS_SB_NODE_SZ, UBIFS_SB_LNUM);
 }
 
 /**
@@ -2020,13 +2017,11 @@ static int write_master(void)
 	mst.total_dark   = cpu_to_le64(c->lst.total_dark);
 	mst.leb_cnt      = cpu_to_le32(c->leb_cnt);
 
-	err = write_node(&mst, UBIFS_MST_NODE_SZ, UBIFS_MST_LNUM,
-			 UBI_SHORTTERM);
+	err = write_node(&mst, UBIFS_MST_NODE_SZ, UBIFS_MST_LNUM);
 	if (err)
 		return err;
 
-	err = write_node(&mst, UBIFS_MST_NODE_SZ, UBIFS_MST_LNUM + 1,
-			 UBI_SHORTTERM);
+	err = write_node(&mst, UBIFS_MST_NODE_SZ, UBIFS_MST_LNUM + 1);
 	if (err)
 		return err;
 
@@ -2046,14 +2041,14 @@ static int write_log(void)
 	cs.ch.node_type = UBIFS_CS_NODE;
 	cs.cmt_no = cpu_to_le64(0);
 
-	err = write_node(&cs, UBIFS_CS_NODE_SZ, lnum, UBI_UNKNOWN);
+	err = write_node(&cs, UBIFS_CS_NODE_SZ, lnum);
 	if (err)
 		return err;
 
 	lnum += 1;
 
 	for (i = 1; i < c->log_lebs; i++, lnum++) {
-		err = write_empty_leb(lnum, UBI_UNKNOWN);
+		err = write_empty_leb(lnum);
 		if (err)
 			return err;
 	}
@@ -2074,7 +2069,7 @@ static int write_lpt(void)
 
 	lnum = c->nhead_lnum + 1;
 	while (lnum <= c->lpt_last) {
-		err = write_empty_leb(lnum++, UBI_SHORTTERM);
+		err = write_empty_leb(lnum++);
 		if (err)
 			return err;
 	}
@@ -2091,7 +2086,7 @@ static int write_orphan_area(void)
 
 	lnum = UBIFS_LOG_LNUM + c->log_lebs + c->lpt_lebs;
 	for (i = 0; i < c->orph_lebs; i++, lnum++) {
-		err = write_empty_leb(lnum, UBI_SHORTTERM);
+		err = write_empty_leb(lnum);
 		if (err)
 			return err;
 	}
diff --git a/mkfs.ubifs/mkfs.ubifs.h b/mkfs.ubifs/mkfs.ubifs.h
index c00dce0..01161ef 100644
--- a/mkfs.ubifs/mkfs.ubifs.h
+++ b/mkfs.ubifs/mkfs.ubifs.h
@@ -129,7 +129,7 @@ extern struct ubifs_info info_;
 
 struct hashtable_itr;
 
-int write_leb(int lnum, int len, void *buf, int dtype);
+int write_leb(int lnum, int len, void *buf);
 int parse_devtable(const char *tbl_file);
 struct path_htbl_element *devtbl_find_path(const char *path);
 struct name_htbl_element *devtbl_find_name(struct path_htbl_element *ph_elt,
diff --git a/ubi-utils/include/libubi.h b/ubi-utils/include/libubi.h
index dc03d02..45afe87 100644
--- a/ubi-utils/include/libubi.h
+++ b/ubi-utils/include/libubi.h
@@ -426,13 +426,12 @@ int ubi_update_start(libubi_t desc, int fd, long long bytes);
  * @fd: volume character device file descriptor
  * @lnum: LEB number to change
  * @bytes: how many bytes of new data will be written to the LEB
- * @dtype: data type (%UBI_LONGTERM, %UBI_SHORTTERM, %UBI_UNKNOWN)
  *
  * This function initiates atomic LEB change operation and returns %0 in case
  * of success and %-1 in case of error. he caller is assumed to write @bytes
  * data to the volume @fd afterward.
  */
-int ubi_leb_change_start(libubi_t desc, int fd, int lnum, int bytes, int dtype);
+int ubi_leb_change_start(libubi_t desc, int fd, int lnum, int bytes);
 
 /**
  * ubi_set_property - set volume propety.
diff --git a/ubi-utils/libubi.c b/ubi-utils/libubi.c
index c898e36..9b0eab3 100644
--- a/ubi-utils/libubi.c
+++ b/ubi-utils/libubi.c
@@ -1106,7 +1106,7 @@ int ubi_update_start(libubi_t desc, int fd, long long bytes)
 	return 0;
 }
 
-int ubi_leb_change_start(libubi_t desc, int fd, int lnum, int bytes, int dtype)
+int ubi_leb_change_start(libubi_t desc, int fd, int lnum, int bytes)
 {
 	struct ubi_leb_change_req req;
 
@@ -1114,7 +1114,6 @@ int ubi_leb_change_start(libubi_t desc, int fd, int lnum, int bytes, int dtype)
 	memset(&req, 0, sizeof(struct ubi_leb_change_req));
 	req.lnum = lnum;
 	req.bytes = bytes;
-	req.dtype = dtype;
 
 	if (ioctl(fd, UBI_IOCEBCH, &req))
 		return -1;
-- 
1.7.2.5

[PATCH MTD-UTILS 3/5] re-name set volume properties ioctl

[Richard Genoud]

kernel commit 6748482f4153fc0e095aa3dc831d5edac5656a80 renamed:
'UBI_PROP_DIRECT_WRITE' to 'UBI_VOL_PROP_DIRECT_WRITE'

structure from 'struct ubi_set_prop_req' to 'struct
ubi_set_vol_prop_req'.

and 'UBI_IOCSETPROP' to 'UBI_IOCSETVOLPROP'

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 mkfs.ubifs/mkfs.ubifs.c    |    2 +-
 tests/ubi-tests/io_paral.c |    2 +-
 ubi-utils/include/libubi.h |    2 +-
 ubi-utils/libubi.c         |    6 +++---
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/mkfs.ubifs/mkfs.ubifs.c b/mkfs.ubifs/mkfs.ubifs.c
index 751e348..149806b 100644
--- a/mkfs.ubifs/mkfs.ubifs.c
+++ b/mkfs.ubifs/mkfs.ubifs.c
@@ -2132,7 +2132,7 @@ static int open_target(void)
 		if (out_fd == -1)
 			return sys_err_msg("cannot open the UBI volume '%s'",
 					   output);
-		if (ubi_set_property(out_fd, UBI_PROP_DIRECT_WRITE, 1))
+		if (ubi_set_property(out_fd, UBI_VOL_PROP_DIRECT_WRITE, 1))
 			return sys_err_msg("ubi_set_property failed");
 
 		if (check_volume_empty())
diff --git a/tests/ubi-tests/io_paral.c b/tests/ubi-tests/io_paral.c
index 1d9a97c..615c1dd 100644
--- a/tests/ubi-tests/io_paral.c
+++ b/tests/ubi-tests/io_paral.c
@@ -179,7 +179,7 @@ static void *write_thread(void *ptr)
 		return NULL;
 	}
 
-	ret = ubi_set_property(fd, UBI_PROP_DIRECT_WRITE, 1);
+	ret = ubi_set_property(fd, UBI_VOL_PROP_DIRECT_WRITE, 1);
 	if (ret) {
 		failed("ubi_set_property");
 		errmsg("cannot set property for \"%s\"\n", vol_node);
diff --git a/ubi-utils/include/libubi.h b/ubi-utils/include/libubi.h
index 45afe87..11a186b 100644
--- a/ubi-utils/include/libubi.h
+++ b/ubi-utils/include/libubi.h
@@ -436,7 +436,7 @@ int ubi_leb_change_start(libubi_t desc, int fd, int lnum, int bytes);
 /**
  * ubi_set_property - set volume propety.
  * @fd: volume character device file descriptor
- * @property: the property to change (%UBI_PROP_DIRECT_WRITE, etc)
+ * @property: the property to change (%UBI_VOL_PROP_DIRECT_WRITE, etc)
  * @value: new value of the changed property
  *
  * This function changes a property of a volume. Returns zero in case of
diff --git a/ubi-utils/libubi.c b/ubi-utils/libubi.c
index 9b0eab3..ae805c8 100644
--- a/ubi-utils/libubi.c
+++ b/ubi-utils/libubi.c
@@ -1345,13 +1345,13 @@ int ubi_get_vol_info1_nm(libubi_t desc, int dev_num, const char *name,
 
 int ubi_set_property(int fd, uint8_t property, uint64_t value)
 {
-	struct ubi_set_prop_req r;
+	struct ubi_set_vol_prop_req r;
 
-	memset(&r, 0, sizeof(struct ubi_set_prop_req));
+	memset(&r, 0, sizeof(struct ubi_set_vol_prop_req));
 	r.property = property;
 	r.value = value;
 
-	return ioctl(fd, UBI_IOCSETPROP, &r);
+	return ioctl(fd, UBI_IOCSETVOLPROP, &r);
 }
 
 int ubi_leb_unmap(int fd, int lnum)
-- 
1.7.2.5

[PATCH MTD-UTILS 4/5] sync include/mtd/ubi-user.h: add max_beb_per1024 parameter

[Richard Genoud]

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 include/mtd/ubi-user.h |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/include/mtd/ubi-user.h b/include/mtd/ubi-user.h
index 123951f..a12c884 100644
--- a/include/mtd/ubi-user.h
+++ b/include/mtd/ubi-user.h
@@ -222,6 +222,7 @@ enum {
  * @ubi_num: UBI device number to create
  * @mtd_num: MTD device number to attach
  * @vid_hdr_offset: VID header offset (use defaults if %0)
+ * @max_beb_per1024: maximum expected number of bad PEB per 1024 PEBs
  * @padding: reserved for future, not used, has to be zeroed
  *
  * This data structure is used to specify MTD device UBI has to attach and the
@@ -245,12 +246,25 @@ enum {
  * be 2KiB-64 bytes = 1984. Note, that this position is not even 512-bytes
  * aligned, which is OK, as UBI is clever enough to realize this is 4th
  * sub-page of the first page and add needed padding.
+ *
+ * The @max_beb_per1024 is the maximum amount of bad PEBs UBI expects on the
+ * UBI device per 1024 eraseblocks.  This value is often given in an other form
+ * in the NAND datasheet (min NVB i.e. minimal number of valid blocks). The
+ * maximum expected bad eraseblocks per 1024 is then:
+ *    1024 * (1 - MinNVB / MaxNVB)
+ * Which gives 20 for most NAND devices.  This limit is used in order to derive
+ * amount of eraseblock UBI reserves for handling new bad blocks. If the device
+ * has more bad eraseblocks than this limit, UBI does not reserve any physical
+ * eraseblocks for new bad eraseblocks, but attempts to use available
+ * eraseblocks (if any). The accepted range is 0-768. If 0 is given, the
+ * default kernel value of %CONFIG_MTD_UBI_BEB_LIMIT will be used.
  */
 struct ubi_attach_req {
 	__s32 ubi_num;
 	__s32 mtd_num;
 	__s32 vid_hdr_offset;
-	__s8 padding[12];
+	__s16 max_beb_per1024;
+	__s8 padding[10];
 };
 
 /**
-- 
1.7.2.5

[PATCH MTD-UTILS 5/5] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Richard Genoud]

The ioctl UBI_IOCATT has been extended with max_beb_per1024 parameter.

This parameter is used for adjusting the "maximum expected number of
bad blocks per 1024 blocks" for each mtd device.
The number of physical erase blocks (PEB) that UBI will reserve for bad
block handling is now:
whole_flash_chipset__PEB_number * max_beb_per1024 / 1024

This means that for a 4096 PEB NAND device with 3 MTD partitions:
mtd0: 512 PEB
mtd1: 1536 PEB
mtd2: 2048 PEB

the commands:
ubiattach -m 0 -d 0 -b 20 /dev/ubi_ctrl
ubiattach -m 1 -d 1 -b 20 /dev/ubi_ctrl
ubiattach -m 2 -d 2 -b 20 /dev/ubi_ctrl
will attach mtdx to UBIx and reserve:
80 PEB for bad block handling on UBI0
80 PEB for bad block handling on UBI1
80 PEB for bad block handling on UBI2

=> for the whole device, 240 PEB will be reserved for bad block
handling.

This may seems a waste of space, but as far as the bad blocks can appear
every where on a flash device, in the worst case scenario they can
all appear in one MTD partition.
So the maximum number of expected erase blocks given by the NAND
manufacturer should be reserve on each MTD partition.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 tests/fs-tests/integrity/integck.c |    1 +
 ubi-utils/include/libubi.h         |    2 +
 ubi-utils/libubi.c                 |    2 +
 ubi-utils/ubiattach.c              |   38 ++++++++++++++++++++++++++++-------
 4 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/tests/fs-tests/integrity/integck.c b/tests/fs-tests/integrity/integck.c
index 30322cd..f12dfac 100644
--- a/tests/fs-tests/integrity/integck.c
+++ b/tests/fs-tests/integrity/integck.c
@@ -3152,6 +3152,7 @@ static int reattach(void)
 	req.mtd_num = args.mtdn;
 	req.vid_hdr_offset = 0;
 	req.mtd_dev_node = NULL;
+	req.max_beb_per1024 = 0;
 
 	err = ubi_attach(libubi, "/dev/ubi_ctrl", &req);
 	if (err)
diff --git a/ubi-utils/include/libubi.h b/ubi-utils/include/libubi.h
index 11a186b..4e9f3c4 100644
--- a/ubi-utils/include/libubi.h
+++ b/ubi-utils/include/libubi.h
@@ -50,6 +50,7 @@ typedef void * libubi_t;
  * @mtd_dev_node: path to MTD device node to attach
  * @vid_hdr_offset: VID header offset (%0 means default offset and this is what
  *                  most of the users want)
+ * @max_beb_per1024: Maximum expected bad eraseblocks per 1024 eraseblocks
  */
 struct ubi_attach_request
 {
@@ -57,6 +58,7 @@ struct ubi_attach_request
 	int mtd_num;
 	const char *mtd_dev_node;
 	int vid_hdr_offset;
+	int max_beb_per1024;
 };
 
 /**
diff --git a/ubi-utils/libubi.c b/ubi-utils/libubi.c
index ae805c8..1db4a19 100644
--- a/ubi-utils/libubi.c
+++ b/ubi-utils/libubi.c
@@ -719,6 +719,7 @@ int ubi_attach_mtd(libubi_t desc, const char *node,
 	r.ubi_num = req->dev_num;
 	r.mtd_num = req->mtd_num;
 	r.vid_hdr_offset = req->vid_hdr_offset;
+	r.max_beb_per1024 = req->max_beb_per1024;
 
 	ret = do_attach(node, &r);
 	if (ret == 0)
@@ -780,6 +781,7 @@ int ubi_attach(libubi_t desc, const char *node, struct ubi_attach_request *req)
 	memset(&r, 0, sizeof(struct ubi_attach_req));
 	r.ubi_num = req->dev_num;
 	r.vid_hdr_offset = req->vid_hdr_offset;
+	r.max_beb_per1024 = req->max_beb_per1024;
 
 	/*
 	 * User has passed path to device node. Lets find out MTD device number
diff --git a/ubi-utils/ubiattach.c b/ubi-utils/ubiattach.c
index 27e7c09..4521788 100644
--- a/ubi-utils/ubiattach.c
+++ b/ubi-utils/ubiattach.c
@@ -42,6 +42,7 @@ struct args {
 	int vidoffs;
 	const char *node;
 	const char *dev;
+	int max_beb_per1024;
 };
 
 static struct args args = {
@@ -50,6 +51,7 @@ static struct args args = {
 	.vidoffs = 0,
 	.node = NULL,
 	.dev = NULL,
+	.max_beb_per1024 = 0,
 };
 
 static const char doc[] = PROGRAM_NAME " version " VERSION
@@ -63,6 +65,9 @@ static const char optionsstr[] =
 "                      if the character device node does not exist)\n"
 "-O, --vid-hdr-offset  VID header offset (do not specify this unless you really\n"
 "                      know what you are doing, the default should be optimal)\n"
+"-b, --max-beb-per1024 Maximum expected bad block number per 1024 eraseblock.\n"
+"                      The default value is correct for most NAND devices.\n"
+"                      (Range 1-768, 0 for default kernel value).\n"
 "-h, --help            print help message\n"
 "-V, --version         print program version";
 
@@ -71,19 +76,25 @@ static const char usage[] =
 "\t[-m <MTD device number>] [-d <UBI device number>] [-p <path to device>]\n"
 "\t[--mtdn=<MTD device number>] [--devn=<UBI device number>]\n"
 "\t[--dev-path=<path to device>]\n"
+"\t[--max-beb-per1024=<maximum bad block number per 1024 blocks>]\n"
 "UBI control device defaults to " DEFAULT_CTRL_DEV " if not supplied.\n"
 "Example 1: " PROGRAM_NAME " -p /dev/mtd0 - attach /dev/mtd0 to UBI\n"
 "Example 2: " PROGRAM_NAME " -m 0 - attach MTD device 0 (mtd0) to UBI\n"
 "Example 3: " PROGRAM_NAME " -m 0 -d 3 - attach MTD device 0 (mtd0) to UBI\n"
-"           and create UBI device number 3 (ubi3)";
+"           and create UBI device number 3 (ubi3)\n"
+"Example 4: " PROGRAM_NAME " -m 1 -b 25 - attach /dev/mtd1 to UBI and reserve \n"
+"           25*nand_size_in_blocks/1024 erase blocks for bad block handling.\n"
+"           (e.g. if the NAND *chipset* has 4096 PEB, 100 will be reserved \n"
+"           for this UBI device).";
 
 static const struct option long_options[] = {
-	{ .name = "devn",           .has_arg = 1, .flag = NULL, .val = 'd' },
-	{ .name = "dev-path",       .has_arg = 1, .flag = NULL, .val = 'p' },
-	{ .name = "mtdn",           .has_arg = 1, .flag = NULL, .val = 'm' },
-	{ .name = "vid-hdr-offset", .has_arg = 1, .flag = NULL, .val = 'O' },
-	{ .name = "help",           .has_arg = 0, .flag = NULL, .val = 'h' },
-	{ .name = "version",        .has_arg = 0, .flag = NULL, .val = 'V' },
+	{ .name = "devn",            .has_arg = 1, .flag = NULL, .val = 'd' },
+	{ .name = "dev-path",        .has_arg = 1, .flag = NULL, .val = 'p' },
+	{ .name = "mtdn",            .has_arg = 1, .flag = NULL, .val = 'm' },
+	{ .name = "vid-hdr-offset",  .has_arg = 1, .flag = NULL, .val = 'O' },
+	{ .name = "help",            .has_arg = 0, .flag = NULL, .val = 'h' },
+	{ .name = "version",         .has_arg = 0, .flag = NULL, .val = 'V' },
+	{ .name = "max-beb-per1024", .has_arg = 1, .flag = NULL, .val = 'b' },
 	{ NULL, 0, NULL, 0},
 };
 
@@ -92,7 +103,7 @@ static int parse_opt(int argc, char * const argv[])
 	while (1) {
 		int key, error = 0;
 
-		key = getopt_long(argc, argv, "p:m:d:O:hV", long_options, NULL);
+		key = getopt_long(argc, argv, "p:m:d:O:hVb:", long_options, NULL);
 		if (key == -1)
 			break;
 
@@ -134,6 +145,16 @@ static int parse_opt(int argc, char * const argv[])
 		case ':':
 			return errmsg("parameter is missing");
 
+		case 'b':
+			args.max_beb_per1024 = simple_strtoul(optarg, &error);
+			if (error || args.max_beb_per1024 < 0 || args.max_beb_per1024 > 768)
+				return errmsg("bad maximum of expected bad blocks (0-768): \"%s\"", optarg);
+
+			if (args.max_beb_per1024 == 0)
+				warnmsg("default kernel value will be used for maximum expected bad blocks\n");
+
+			break;
+
 		default:
 			fprintf(stderr, "Use -h for help\n");
 			return -1;
@@ -190,6 +211,7 @@ int main(int argc, char * const argv[])
 	req.mtd_num = args.mtdn;
 	req.vid_hdr_offset = args.vidoffs;
 	req.mtd_dev_node = args.dev;
+	req.max_beb_per1024 = args.max_beb_per1024;
 
 	err = ubi_attach(libubi, args.node, &req);
 	if (err) {
-- 
1.7.2.5

Re: [PATCH MTD-UTILS 2/5] dtype parameter is obsolete

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 498 bytes --]

On Wed, 2012-08-22 at 11:27 +0200, Richard Genoud wrote:
> @@ -1114,7 +1114,6 @@ int ubi_leb_change_start(libubi_t desc, int fd,
> int lnum, int bytes, int dtype)
>         memset(&req, 0, sizeof(struct ubi_leb_change_req));
>         req.lnum = lnum;
>         req.bytes = bytes;
> -       req.dtype = dtype; 
I've also changed this:

+         req.dtype = 3

just like comment on top of 'struct ubi_leb_change_req' in ubi-user.h
recommends.

-- 
Best Regards,
Artem Bityutskiy


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH MTD-UTILS 1/5] sync include/mtd/ubi-user.h with kernel v3.6-rc1

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 241 bytes --]

On Wed, 2012-08-22 at 11:27 +0200, Richard Genoud wrote:
> @@ -21,6 +21,8 @@
>  #ifndef __UBI_USER_H__
>  #define __UBI_USER_H__
>  
> +#include <linux/types.h>
> +

And I dropped this chunk.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH MTD-UTILS 0/5] sync with 3.6-rc1 and introduce max_beb_per1024 in UBI_IOCATT

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 879 bytes --]

On Wed, 2012-08-22 at 11:27 +0200, Richard Genoud wrote:
> Hi Artem,
> Here are the patches for the introduction of max_beb_per1024 in ubiattach.
> The first patch is just a sync of ubi-user.h with the vanilla 3.6-rc1
> (So after this patch, the mtd-utils won't build any more)
> The second one supresses dtype parameter, as it is obsolete now.
> The third syncs with somes defines and struct renames in the ubi-user.h
> (Now it compiles again)
> The fourth syncs ubi-user.h with the one in linux-ubi/master
> And finally the last one instroduce max_beb_per1024.

I squashed patches 1-3 and pushed to mtd-utils. I also changed all __s32
and the like back to int32_t, because on older systems these linux types
are not available in the system headers. I also did few minor
modifications, and pushed to mtd-utils.git, thanks!

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH MTD-UTILS 4/5] sync include/mtd/ubi-user.h: add max_beb_per1024 parameter

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 208 bytes --]

On Wed, 2012-08-22 at 11:27 +0200, Richard Genoud wrote:
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>

Pushed this one with minor amendments, thanks!

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH MTD-UTILS 5/5] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 763 bytes --]

On Wed, 2012-08-22 at 11:28 +0200, Richard Genoud wrote:
> +"-b, --max-beb-per1024 Maximum expected bad block number per 1024 eraseblock.\n"
> +"                      The default value is correct for most NAND devices.\n"
> +"                      (Range 1-768, 0 for default kernel value).\n"

If the feature is not supported (the kernel is old), the tool will just
return success and the kernel will just ignore max-beb-per1024, which is
not very nice.

It would be better if ubiattach complained like this instead:

ubiattach error: your UBI driver does not allow changing the reserved
                 PEBs count, probably you run old kernel? The support
                 was added in kernel version 3.7.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH MTD-UTILS 5/5] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Richard Genoud]

2012/8/22 Artem Bityutskiy <dedekind1@gmail.com>:
> If the feature is not supported (the kernel is old), the tool will just
> return success and the kernel will just ignore max-beb-per1024, which is
> not very nice.
>
> It would be better if ubiattach complained like this instead:
>
> ubiattach error: your UBI driver does not allow changing the reserved
>                  PEBs count, probably you run old kernel? The support
>                  was added in kernel version 3.7.
yes, you're right, I'll change that.
Thanks,

Richard.

Re: [PATCH MTD-UTILS 5/5] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 675 bytes --]

On Wed, 2012-08-22 at 13:17 +0200, Richard Genoud wrote:
> 2012/8/22 Artem Bityutskiy <dedekind1@gmail.com>:
> > If the feature is not supported (the kernel is old), the tool will just
> > return success and the kernel will just ignore max-beb-per1024, which is
> > not very nice.
> >
> > It would be better if ubiattach complained like this instead:
> >
> > ubiattach error: your UBI driver does not allow changing the reserved
> >                  PEBs count, probably you run old kernel? The support
> >                  was added in kernel version 3.7.
> yes, you're right, I'll change that.

The question is how :-)

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH MTD-UTILS 5/5] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]

On Wed, 2012-08-22 at 13:17 +0200, Richard Genoud wrote:
> 2012/8/22 Artem Bityutskiy <dedekind1@gmail.com>:
> > If the feature is not supported (the kernel is old), the tool will just
> > return success and the kernel will just ignore max-beb-per1024, which is
> > not very nice.
> >
> > It would be better if ubiattach complained like this instead:
> >
> > ubiattach error: your UBI driver does not allow changing the reserved
> >                  PEBs count, probably you run old kernel? The support
> >                  was added in kernel version 3.7.
> yes, you're right, I'll change that.
> Thanks,

I see 2 ways.

1. Add a 'get device property' ioctl, similar to the 'set volume
property' ioctl we have. And use this ioctl to query the current bad
PEBs limit.

2. Just call the 'attach' ioctl with an insane max_beb_per1024 value
(say, -1) first. If it succeeded, than the driver does not support
max_beb_per1024 at all, and you may exit with a warning that you have
attached it, but the max_beb_per1024 was ignored because the kernel is
old.

If it fails with -EINVAL, this means the kernel checked the value and
the feature is supported, so you can call it for the second time with
the right max_beb_per1024.

The first approach is more complex but probably cleaner. The second is
easier to implement.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH MTD-UTILS 5/5] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 904 bytes --]

On Wed, 2012-08-22 at 14:30 +0300, Artem Bityutskiy wrote:
> 1. Add a 'get device property' ioctl, similar to the 'set volume
> property' ioctl we have. And use this ioctl to query the current bad
> PEBs limit.
> 
> 2. Just call the 'attach' ioctl with an insane max_beb_per1024 value
> (say, -1) first. If it succeeded, than the driver does not support
> max_beb_per1024 at all, and you may exit with a warning that you have
> attached it, but the max_beb_per1024 was ignored because the kernel is
> old.
> 
> If it fails with -EINVAL, this means the kernel checked the value and
> the feature is supported, so you can call it for the second time with
> the right max_beb_per1024.
> 
> The first approach is more complex but probably cleaner. The second is
> easier to implement.

I guess I prefer the second option, though. What do you think?

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH MTD-UTILS 5/5] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Richard Genoud]

2012/8/22 Artem Bityutskiy <dedekind1@gmail.com>:
> I see 2 ways.
>
> 1. Add a 'get device property' ioctl, similar to the 'set volume
> property' ioctl we have. And use this ioctl to query the current bad
> PEBs limit.
>
> 2. Just call the 'attach' ioctl with an insane max_beb_per1024 value
> (say, -1) first. If it succeeded, than the driver does not support
> max_beb_per1024 at all, and you may exit with a warning that you have
> attached it, but the max_beb_per1024 was ignored because the kernel is
> old.
>
> If it fails with -EINVAL, this means the kernel checked the value and
> the feature is supported, so you can call it for the second time with
> the right max_beb_per1024.
>
> The first approach is more complex but probably cleaner. The second is
> easier to implement.
>
> --
> Best Regards,
> Artem Bityutskiy

There's also a 3rd one with an uname system call, checking if kernel
>= 3.7, but that's not very elegant.

-- 
for me, ck means con kolivas and not calvin klein... does it mean I'm a geek ?

Re: [PATCH MTD-UTILS 5/5] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Richard Genoud]

2012/8/22 Artem Bityutskiy <dedekind1@gmail.com>:
> On Wed, 2012-08-22 at 14:30 +0300, Artem Bityutskiy wrote:
>> 1. Add a 'get device property' ioctl, similar to the 'set volume
>> property' ioctl we have. And use this ioctl to query the current bad
>> PEBs limit.
>>
>> 2. Just call the 'attach' ioctl with an insane max_beb_per1024 value
>> (say, -1) first. If it succeeded, than the driver does not support
>> max_beb_per1024 at all, and you may exit with a warning that you have
>> attached it, but the max_beb_per1024 was ignored because the kernel is
>> old.
>>
>> If it fails with -EINVAL, this means the kernel checked the value and
>> the feature is supported, so you can call it for the second time with
>> the right max_beb_per1024.
>>
>> The first approach is more complex but probably cleaner. The second is
>> easier to implement.
>
> I guess I prefer the second option, though. What do you think?
Well, for this one, we can go with the 2nd option, and if one day
there's another parameter, then we could implement the 'get device
property ioctl' for max_beb_per1024 and the new parameter.
How's that sound ?

Re: [PATCH MTD-UTILS 5/5] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Shmulik Ladkani]

Hi Artem,

On Wed, 22 Aug 2012 14:33:17 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > 2. Just call the 'attach' ioctl with an insane max_beb_per1024 value
> > (say, -1) first. If it succeeded, than the driver does not support
> > max_beb_per1024 at all, and you may exit with a warning that you have
> > attached it, but the max_beb_per1024 was ignored because the kernel is
> > old.
> > 
> > If it fails with -EINVAL, this means the kernel checked the value and
> > the feature is supported, so you can call it for the second time with
> > the right max_beb_per1024.

But the ioctl might return -EINVAL due to any of the *other*
ubi_attach_req fields being invalid.
So you can't really determine whether max_beb_per1024 is supported.

> > The first approach is more complex but probably cleaner. The second is
> > easier to implement.
> 
> I guess I prefer the second option, though. What do you think?

I would go with first, due to my slight tendency to over-design
things ;-)
It is cleaner, however bloatier too...

I'll try to think if there's an easy alternative.

Regards,
Shmulik

Re: [PATCH MTD-UTILS 5/5] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 631 bytes --]

On Wed, 2012-08-22 at 13:38 +0200, Richard Genoud wrote:
> > I guess I prefer the second option, though. What do you think?
> Well, for this one, we can go with the 2nd option, and if one day
> there's another parameter, then we could implement the 'get device
> property ioctl' for max_beb_per1024 and the new parameter.
> How's that sound ?

Good. But please, implement this double 'attach' invocation inside
'ubi_attach_mtd()'. Please, introduce a special return code to indicate
the 'attached but max_beb_per1024 was ignored' case (say, 1). Add
comments explaining the trick.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH MTD-UTILS 5/5] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 859 bytes --]

On Wed, 2012-08-22 at 14:47 +0300, Shmulik Ladkani wrote:
> But the ioctl might return -EINVAL due to any of the *other*
> ubi_attach_req fields being invalid.
> So you can't really determine whether max_beb_per1024 is supported.

We call the attach ioctl 2 times.

1. Call it with bogus max_beb_per1024.
   a) Success - print the warning and exit
   b) Failure with non-EINVAL - overall failure.
   c) Failure with -EINVAL, go to step 2.


2. Call with user-supplied max_beb_per1024.
   a) Success - everything is fine, max_beb_per1024 is supported,
      the first failure was because we passed max_beb_per1024=-1
   b) Failure with non-EINVAL - overall failure.
   b) Failure with -EINVAL, return error, the whole operation failed,
      one of the parameters was incorrect.

Where is the flaw?

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH MTD-UTILS 5/5] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Shmulik Ladkani]

On Wed, 22 Aug 2012 15:15:15 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Wed, 2012-08-22 at 14:47 +0300, Shmulik Ladkani wrote:
> > But the ioctl might return -EINVAL due to any of the *other*
> > ubi_attach_req fields being invalid.
> > So you can't really determine whether max_beb_per1024 is supported.
> 
> We call the attach ioctl 2 times.
> 
> 1. Call it with bogus max_beb_per1024.
>    a) Success - print the warning and exit
>    b) Failure with non-EINVAL - overall failure.
>    c) Failure with -EINVAL, go to step 2.
> 
> 
> 2. Call with user-supplied max_beb_per1024.
>    a) Success - everything is fine, max_beb_per1024 is supported,
>       the first failure was because we passed max_beb_per1024=-1
>    b) Failure with non-EINVAL - overall failure.
>    b) Failure with -EINVAL, return error, the whole operation failed,
>       one of the parameters was incorrect.
> 
> Where is the flaw?

Sorry, this seems right. Should have given this more thought... will go
for some cofee.

Regards,
Shmulik

Re: [PATCH MTD-UTILS 5/5] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Richard Genoud]

2012/8/22 Artem Bityutskiy <dedekind1@gmail.com>:
> Good. But please, implement this double 'attach' invocation inside
> 'ubi_attach_mtd()'. Please, introduce a special return code to indicate
> the 'attached but max_beb_per1024 was ignored' case (say, 1). Add
> comments explaining the trick.

one question:
- does the call:
ubiattach -m 2 -b 0
should fail on an old kernel ?
In this case, I'll have to introduce a flag or something to let
ubi_attach_mtd() know that the -b option was given.
Honestly, I don't think it's necessary, as far as -b0 means "the
default kernel value", so on an old kernel the default that's what
will happened.

I'll have also to change the ubi_attach() call.
My though is to factorize code between ubi_attach_mtd() and
ubi_attach(), and then make the change.


Best regards,
Richard.

Re: [PATCH MTD-UTILS 5/5] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 592 bytes --]

On Wed, 2012-08-22 at 15:31 +0200, Richard Genoud wrote:
> 2012/8/22 Artem Bityutskiy <dedekind1@gmail.com>:
> > Good. But please, implement this double 'attach' invocation inside
> > 'ubi_attach_mtd()'. Please, introduce a special return code to indicate
> > the 'attached but max_beb_per1024 was ignored' case (say, 1). Add
> > comments explaining the trick.
> 
> one question:
> - does the call:
> ubiattach -m 2 -b 0
> should fail on an old kernel ?

No, no need to complicate the thing with handling this case.
Let's keep it simple.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]