[PATCH MTD-UTILS v2 0/4] introduce max_beb_per1024 in UBI_IOCATT

[PATCH MTD-UTILS v2 0/4] introduce max_beb_per1024 in UBI_IOCATT

[Richard Genoud]

Here is the v2 of the user-space part for max_beb_per1024 parameter.

Changes since v1:
* code factorization between ubi_attach and ubi_attach_mtd 
* ubiattach now fails if -b option is given and the kernel doesn't supports it.

Richard Genoud (4):
  libubi: factorize ubi_attach and ubi_attach_mtd code
  sync include/mtd/ubi-user.h: add max_beb_per1024 parameter
  ubiattach: introduce max_beb_per1024 in UBI_IOCATT
  ubiattach: fail if kernel ignores max_beb_per1024

 include/mtd/ubi-user.h             |   16 ++++++++-
 tests/fs-tests/integrity/integck.c |    1 +
 ubi-utils/include/libubi.h         |   12 +++++--
 ubi-utils/libubi.c                 |   69 +++++++++++++++++++++++------------
 ubi-utils/ubiattach.c              |   47 ++++++++++++++++++++-----
 5 files changed, 108 insertions(+), 37 deletions(-)

-- 
1.7.2.5

[PATCH MTD-UTILS v2 1/4] libubi: factorize ubi_attach and ubi_attach_mtd code

[Richard Genoud]

The req->mtd_num value is now updated with the MTD device number found
by mtd_node_to_num.

If it's not acceptable, I'll do a static __ubi_detach_mtd() function to
prevent that, and still factorize code.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 ubi-utils/include/libubi.h |    5 +++--
 ubi-utils/libubi.c         |   34 ++++++++++------------------------
 2 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/ubi-utils/include/libubi.h b/ubi-utils/include/libubi.h
index 11a186b..bbaa78c 100644
--- a/ubi-utils/include/libubi.h
+++ b/ubi-utils/include/libubi.h
@@ -233,8 +233,9 @@ int ubi_attach_mtd(libubi_t desc, const char *node,
  * device node. Otherwise functionality is similar than in function
  * 'ubi_attach_mtd()' where @req->mtd_num is used.
  *
- * Returns %0 in case of success and %-1 in case of failure (errno is set). The
- * newly created UBI device number is returned in @req->dev_num.
+ * Returns %0 in case of success and %-1 in case of failure (errno is set).
+ * The newly created UBI device number is returned in @req->dev_num.
+ * The MTD device number is returned in @req->mtd_num (-1 if not found)
  */
 int ubi_attach(libubi_t desc, const char *node,
 	       struct ubi_attach_request *req);
diff --git a/ubi-utils/libubi.c b/ubi-utils/libubi.c
index 7736dd4..dec72c7 100644
--- a/ubi-utils/libubi.c
+++ b/ubi-utils/libubi.c
@@ -770,30 +770,16 @@ static int mtd_node_to_num(const char *mtd_dev_node)
 
 int ubi_attach(libubi_t desc, const char *node, struct ubi_attach_request *req)
 {
-	struct ubi_attach_req r;
-	int ret;
-
-	if (!req->mtd_dev_node)
-		/* Fallback to opening by mtd_num */
-		return ubi_attach_mtd(desc, node, req);
-
-	memset(&r, 0, sizeof(struct ubi_attach_req));
-	r.ubi_num = req->dev_num;
-	r.vid_hdr_offset = req->vid_hdr_offset;
-
-	/*
-	 * User has passed path to device node. Lets find out MTD device number
-	 * of the device and pass it to the kernel.
-	 */
-	r.mtd_num = mtd_node_to_num(req->mtd_dev_node);
-	if (r.mtd_num == -1)
-		return -1;
-
-	ret = do_attach(node, &r);
-	if (ret == 0)
-		req->dev_num = r.ubi_num;
-
-	return ret;
+	if (req->mtd_dev_node) {
+		/*
+		 * User has passed path to device node. Lets find out MTD
+		 * device number of the device and update req->mtd_num with it
+		 */
+		req->mtd_num = mtd_node_to_num(req->mtd_dev_node);
+		if (req->mtd_num == -1)
+			return -1;
+	}
+	return ubi_attach_mtd(desc, node, req);
 }
 
 int ubi_detach_mtd(libubi_t desc, const char *node, int mtd_num)
-- 
1.7.2.5

Re: [PATCH MTD-UTILS v2 1/4] libubi: factorize ubi_attach and ubi_attach_mtd code

[Artem Bityutskiy]

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

On Wed, 2012-08-22 at 18:04 +0200, Richard Genoud wrote:
> The req->mtd_num value is now updated with the MTD device number found
> by mtd_node_to_num.
> 
> If it's not acceptable, I'll do a static __ubi_detach_mtd() function to
> prevent that, and still factorize code.

Pushed to mtd-utils, thanks. On top of this pushed a patch which removes
'ubi_attach_mtd()' completely, because we do not have users and
'ubi_attach()' does the job anyway.

Thanks!

-- 
Best Regards,
Artem Bityutskiy

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

[PATCH MTD-UTILS v2 2/4] 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 19762d0..736a828 100644
--- a/include/mtd/ubi-user.h
+++ b/include/mtd/ubi-user.h
@@ -220,6 +220,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
@@ -243,12 +244,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 {
 	int32_t ubi_num;
 	int32_t mtd_num;
 	int32_t vid_hdr_offset;
-	int8_t padding[12];
+	int16_t max_beb_per1024;
+	int8_t padding[10];
 };
 
 /**
-- 
1.7.2.5

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

[Artem Bityutskiy]

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

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

Pushed, thanks!

-- 
Best Regards,
Artem Bityutskiy

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

[PATCH MTD-UTILS v2 3/4] 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                 |    1 +
 ubi-utils/ubiattach.c              |   38 ++++++++++++++++++++++++++++-------
 4 files changed, 34 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 bbaa78c..28c8782 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 dec72c7..1b62e59 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)
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 v2 3/4] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Artem Bityutskiy]

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

On Wed, 2012-08-22 at 18:04 +0200, Richard Genoud wrote:
> 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

Pushed this one with some amendments.

-- 
Best Regards,
Artem Bityutskiy

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

Re: [PATCH MTD-UTILS v2 3/4] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Richard Genoud]

2012/8/23 Artem Bityutskiy <dedekind1@gmail.com>:
> On Wed, 2012-08-22 at 18:04 +0200, Richard Genoud wrote:
>> 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
>
> Pushed this one with some amendments.

Hi Artem,

Could you push your git to mtd-utils so that I can test the final
version, please ?

Best regards,
Richard.

Re: [PATCH MTD-UTILS v2 3/4] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Artem Bityutskiy]

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

On Mon, 2012-08-27 at 09:23 +0200, Richard Genoud wrote:
> Could you push your git to mtd-utils so that I can test the final
> version, please ?

It is pushed, the latest commit is:

commit 9d8751b3f5c6358b6167c38899f1e41498d24a45
Author: Richard Genoud <richard.genoud@gmail.com>
Date:   Wed Aug 22 18:04:37 2012 +0200

    ubiattach: fail if kernel ignores max_beb_per1024
    
    If the kernel doesn't know the max_beb_per1024 parameter in the attach
    ioctl, but the call still succeeded ubi_attach and ubi_attach_mtd will
    return 1 instead of 0.
    
    In this case, the ubiattach command will detach the device and fail with
    an error message.
    
    Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
    Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>


-- 
Best Regards,
Artem Bityutskiy

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

Re: [PATCH MTD-UTILS v2 3/4] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Richard Genoud]

2012/8/27 Artem Bityutskiy <dedekind1@gmail.com>:
> On Mon, 2012-08-27 at 09:23 +0200, Richard Genoud wrote:
>> Could you push your git to mtd-utils so that I can test the final
>> version, please ?
>
> It is pushed, the latest commit is:
>
> commit 9d8751b3f5c6358b6167c38899f1e41498d24a45
> Author: Richard Genoud <richard.genoud@gmail.com>
> Date:   Wed Aug 22 18:04:37 2012 +0200
>
>     ubiattach: fail if kernel ignores max_beb_per1024
>
>     If the kernel doesn't know the max_beb_per1024 parameter in the attach
>     ioctl, but the call still succeeded ubi_attach and ubi_attach_mtd will
>     return 1 instead of 0.
>
>     In this case, the ubiattach command will detach the device and fail with
>     an error message.
>
>     Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>     Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

hum.... I must be looking at the wrong git tree.
I had this one : http://git.infradead.org/mtd-utils.git
with the last commit "UBI: sync ubi-user.h with kernel v3.6-rc1"
312e784fb06eaafff4cfebe29c74b8d0ecc09167

Re: [PATCH MTD-UTILS v2 3/4] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Artem Bityutskiy]

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

On Mon, 2012-08-27 at 12:19 +0200, Richard Genoud wrote:
> hum.... I must be looking at the wrong git tree.
> I had this one : http://git.infradead.org/mtd-utils.git
> with the last commit "UBI: sync ubi-user.h with kernel v3.6-rc1"
> 312e784fb06eaafff4cfebe29c74b8d0ecc09167

Oh, sorry, now it is pushed for real.

-- 
Best Regards,
Artem Bityutskiy

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

[PATCH MTD-UTILS v2 4/4] ubiattach: fail if kernel ignores max_beb_per1024

[Richard Genoud]

If the kernel doesn't know the max_beb_per1024 parameter in the attach
ioctl, but the call still succeeded ubi_attach and ubi_attach_mtd will
return 1 instead of 0.

In this case, the ubiattach command will detach the device and fail with
an error message.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 ubi-utils/include/libubi.h |    7 +++++--
 ubi-utils/libubi.c         |   34 ++++++++++++++++++++++++++++++++++
 ubi-utils/ubiattach.c      |    9 ++++++++-
 3 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/ubi-utils/include/libubi.h b/ubi-utils/include/libubi.h
index 28c8782..da3956d 100644
--- a/ubi-utils/include/libubi.h
+++ b/ubi-utils/include/libubi.h
@@ -218,7 +218,9 @@ int mtd_num2ubi_dev(libubi_t desc, int mtd_num, int *dev_num);
  * @req: MTD attach request.
  *
  * This function creates a new UBI device by attaching an MTD device as
- * described by @req. Returns %0 in case of success and %-1 in case of failure.
+ * described by @req.
+ * Returns %0 in case of success, %-1 in case of failure (errno is set) and %1
+ * if parameter @req->max_beb_per1024 was ignored by kernel.
  * The newly created UBI device number is returned in @req->dev_num.
  */
 int ubi_attach_mtd(libubi_t desc, const char *node,
@@ -235,7 +237,8 @@ int ubi_attach_mtd(libubi_t desc, const char *node,
  * device node. Otherwise functionality is similar than in function
  * 'ubi_attach_mtd()' where @req->mtd_num is used.
  *
- * Returns %0 in case of success and %-1 in case of failure (errno is set).
+ * Returns %0 in case of success, %-1 in case of failure (errno is set) and %1
+ * if parameter @req->max_beb_per1024 was ignored by kernel.
  * The newly created UBI device number is returned in @req->dev_num.
  * The MTD device number is returned in @req->mtd_num (-1 if not found)
  */
diff --git a/ubi-utils/libubi.c b/ubi-utils/libubi.c
index 1b62e59..cc20667 100644
--- a/ubi-utils/libubi.c
+++ b/ubi-utils/libubi.c
@@ -719,6 +719,40 @@ 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;
+
+	if (req->max_beb_per1024) {
+		/*
+		 * max_beb_per1024 was provided by user.
+		 * In this case, we have to check if it's supported by kernel
+		 * or not.
+		 * For that, we're going to make a first call with a wrong
+		 * value.
+		 * If the return value is OK, it means that the kernel doesn't
+		 * support the feature.
+		 * If the return value is not OK nor -EINVAL, another error
+		 * happened.
+		 * If the return value is -EINVAL, we're making a second call,
+		 * with the real value this time.
+		 */
+		r.max_beb_per1024 = -1;
+		ret = do_attach(node, &r);
+		if (ret == 0) {
+			req->dev_num = r.ubi_num;
+			/*
+			 * The call succeeded. It means that the kernel ignored
+			 * max_beb_per1024 parameter. We return 1 to let the
+			 * user know about this.
+			 */
+			return 1;
+		} else {
+			if (errno != EINVAL)
+				return ret;
+			/*
+			 * max_beb_per1024 may be supported,
+			 * let's make the 2nd call.
+			 */
+		}
+	}
 	r.max_beb_per1024 = req->max_beb_per1024;
 
 	ret = do_attach(node, &r);
diff --git a/ubi-utils/ubiattach.c b/ubi-utils/ubiattach.c
index 4521788..1cf0025 100644
--- a/ubi-utils/ubiattach.c
+++ b/ubi-utils/ubiattach.c
@@ -214,12 +214,19 @@ int main(int argc, char * const argv[])
 	req.max_beb_per1024 = args.max_beb_per1024;
 
 	err = ubi_attach(libubi, args.node, &req);
-	if (err) {
+	if (err < 0) {
 		if (args.dev)
 			sys_errmsg("cannot attach \"%s\"", args.dev);
 		else
 			sys_errmsg("cannot attach mtd%d", args.mtdn);
 		goto out_libubi;
+	} else if (err == 1) {
+		/*
+		 * The kernel did not support the max_beb_per1024 parameter.
+		 */
+		(void) ubi_detach_mtd(libubi, args.node, req.mtd_num);
+		errmsg("Your UBI driver does not allow changing the reserved PEBs count, probably you run an old kernel ? The support was added in kernel version 3.7.");
+		goto out_libubi;
 	}
 
 	/* Print some information about the new UBI device */
-- 
1.7.2.5

Re: [PATCH MTD-UTILS v2 4/4] ubiattach: fail if kernel ignores max_beb_per1024

[Artem Bityutskiy]

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

On Wed, 2012-08-22 at 18:04 +0200, Richard Genoud wrote:
> If the kernel doesn't know the max_beb_per1024 parameter in the attach
> ioctl, but the call still succeeded ubi_attach and ubi_attach_mtd will
> return 1 instead of 0.
> 
> In this case, the ubiattach command will detach the device and fail with
> an error message.
> 
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>

Pushed with some amendments, thanks!
> +	} else if (err == 1) {
> +		/*
> +		 * The kernel did not support the max_beb_per1024 parameter.
> +		 */
> +		(void) ubi_detach_mtd(libubi, args.node, req.mtd_num);
> +		errmsg("Your UBI driver does not allow changing the reserved PEBs count, probably you run an old kernel ? The support was added in kernel version 3.7.");
> +		goto out_libubi;

Here I've removed the ubi_detach_mtd() call and the goto. Since we have
already spent energy for attaching, I think it is nicer to just print a
warning and not detach, and let the user decide if he wants to detach.

-- 
Best Regards,
Artem Bityutskiy

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

Re: [PATCH MTD-UTILS v2 4/4] ubiattach: fail if kernel ignores max_beb_per1024

[Richard Genoud]

2012/8/23 Artem Bityutskiy <dedekind1@gmail.com>:
>> +     } else if (err == 1) {
>> +             /*
>> +              * The kernel did not support the max_beb_per1024 parameter.
>> +              */
>> +             (void) ubi_detach_mtd(libubi, args.node, req.mtd_num);
>> +             errmsg("Your UBI driver does not allow changing the reserved PEBs count, probably you run an old kernel ? The support was added in kernel version 3.7.");
>> +             goto out_libubi;
>
> Here I've removed the ubi_detach_mtd() call and the goto. Since we have
> already spent energy for attaching, I think it is nicer to just print a
> warning and not detach, and let the user decide if he wants to detach.
Yes, I hesitate between error and warning.

Thanks !

Richard.