[PATCH] mkfs.ubifs: remove the check for UBIFS_MAX_LEB_SZ

[PATCH] mkfs.ubifs: remove the check for UBIFS_MAX_LEB_SZ

[Dongsheng Yang]

There is a commit 92ed6c0 to increase UBIFS_MAX_LEB_SZ
to 2MiB. But recently, as the leb size become larger and
larger, 2MiB is not a suitable limit any more.

Then remove this check in mkfs.ubifs

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
NOTE:
	I am not sure the reason why we have to
limit the leb size in mkfs.ubifs. Because
I did not find any reason for it, I send
this patch out. It's very possible I am
mising something.

 mkfs.ubifs/mkfs.ubifs.c | 3 ---
 mkfs.ubifs/ubifs.h      | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/mkfs.ubifs/mkfs.ubifs.c b/mkfs.ubifs/mkfs.ubifs.c
index ca17e2b..8ea0e4c 100644
--- a/mkfs.ubifs/mkfs.ubifs.c
+++ b/mkfs.ubifs/mkfs.ubifs.c
@@ -356,9 +356,6 @@ static int validate_options(void)
 		return err_msg("LEB should be multiple of min. I/O units");
 	if (c->leb_size % 8)
 		return err_msg("LEB size has to be multiple of 8");
-	if (c->leb_size > UBIFS_MAX_LEB_SZ)
-		return err_msg("too large LEB size %d, maximum is %d",
-				c->leb_size, UBIFS_MAX_LEB_SZ);
 	if (c->max_leb_cnt < UBIFS_MIN_LEB_CNT)
 		return err_msg("too low max. count of LEBs, minimum is %d",
 			       UBIFS_MIN_LEB_CNT);
diff --git a/mkfs.ubifs/ubifs.h b/mkfs.ubifs/ubifs.h
index 434b651..f94a52c 100644
--- a/mkfs.ubifs/ubifs.h
+++ b/mkfs.ubifs/ubifs.h
@@ -25,9 +25,6 @@
 #ifndef __UBIFS_H__
 #define __UBIFS_H__
 
-/* Maximum logical eraseblock size in bytes */
-#define UBIFS_MAX_LEB_SZ (2*1024*1024)
-
 /* Minimum amount of data UBIFS writes to the flash */
 #define MIN_WRITE_SZ (UBIFS_DATA_NODE_SZ + 8)
 
-- 
1.8.4.2

Re: [PATCH] mkfs.ubifs: remove the check for UBIFS_MAX_LEB_SZ

[Artem Bityutskiy]

On Tue, 2015-08-18 at 12:52 +0800, Dongsheng Yang wrote:
> There is a commit 92ed6c0 to increase UBIFS_MAX_LEB_SZ
> to 2MiB. But recently, as the leb size become larger and
> larger, 2MiB is not a suitable limit any more.
> 
> Then remove this check in mkfs.ubifs
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
> NOTE:
> 	I am not sure the reason why we have to
> limit the leb size in mkfs.ubifs. Because
> I did not find any reason for it, I send
> this patch out. It's very possible I am
> mising something.

Well, this is sanity check for the user input. If you accidentally
added few zeroes, we want to spot this and inform you, and you may
appreciate that we did not just created a bugus image for you. That's
the idea.

Another point is that UBIFS and UBI reads eraseblocks entirely into
memory from time to time, e.g., when scanning the journal (UBIFS) or
when doing wear-levelling (UBI). Too large eraseblocks will affect the
UBI/UBIFS drivers negatively - the latency may increase significantly
(thing reading eraseblock, modifying, writing it), as well as memory
consumption.

I personally like to be strict, and if I am not sure about something, I
put limits, assuming that others may later change the limits.

That said, I'd prefer to increase the limit instead of removing it
altogether. I suggest to make it reasonably large, so that it suits
your purposes.

Artem.

Re: [PATCH] mkfs.ubifs: remove the check for UBIFS_MAX_LEB_SZ

[Dongsheng Yang]

On 08/18/2015 04:52 PM, Artem Bityutskiy wrote:
> On Tue, 2015-08-18 at 12:52 +0800, Dongsheng Yang wrote:
>> There is a commit 92ed6c0 to increase UBIFS_MAX_LEB_SZ
>> to 2MiB. But recently, as the leb size become larger and
>> larger, 2MiB is not a suitable limit any more.
>>
>> Then remove this check in mkfs.ubifs
>>
>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>> ---
>> NOTE:
>> 	I am not sure the reason why we have to
>> limit the leb size in mkfs.ubifs. Because
>> I did not find any reason for it, I send
>> this patch out. It's very possible I am
>> mising something.
>
> Well, this is sanity check for the user input. If you accidentally
> added few zeroes, we want to spot this and inform you, and you may
> appreciate that we did not just created a bugus image for you. That's
> the idea.

Sorry, Artem, I was trying to understand it, but I did not got the
point. Could you give me some more information about the idea? Maybe
an example?

Thanx a lot
Yang
>
> Another point is that UBIFS and UBI reads eraseblocks entirely into
> memory from time to time, e.g., when scanning the journal (UBIFS) or
> when doing wear-levelling (UBI). Too large eraseblocks will affect the
> UBI/UBIFS drivers negatively - the latency may increase significantly
> (thing reading eraseblock, modifying, writing it), as well as memory
> consumption.
>
> I personally like to be strict, and if I am not sure about something, I
> put limits, assuming that others may later change the limits.
>
> That said, I'd prefer to increase the limit instead of removing it
> altogether. I suggest to make it reasonably large, so that it suits
> your purposes.
>
> Artem.
> .
>

Re: [PATCH] mkfs.ubifs: remove the check for UBIFS_MAX_LEB_SZ

[Artem Bityutskiy]

On Mon, 2015-08-24 at 09:07 +0800, Dongsheng Yang wrote:
> On 08/18/2015 04:52 PM, Artem Bityutskiy wrote:
> > On Tue, 2015-08-18 at 12:52 +0800, Dongsheng Yang wrote:
> > > There is a commit 92ed6c0 to increase UBIFS_MAX_LEB_SZ
> > > to 2MiB. But recently, as the leb size become larger and
> > > larger, 2MiB is not a suitable limit any more.
> > > 
> > > Then remove this check in mkfs.ubifs
> > > 
> > > Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> > > ---
> > > NOTE:
> > > 	I am not sure the reason why we have to
> > > limit the leb size in mkfs.ubifs. Because
> > > I did not find any reason for it, I send
> > > this patch out. It's very possible I am
> > > mising something.
> > 
> > Well, this is sanity check for the user input. If you accidentally
> > added few zeroes, we want to spot this and inform you, and you may
> > appreciate that we did not just created a bugus image for you.
> > That's
> > the idea.
> 
> Sorry, Artem, I was trying to understand it, but I did not got the
> point. Could you give me some more information about the idea? Maybe
> an example?

Well, this is defensive programming concept. Something goes wrong, due
to bad user input and/or a bug we get to this place with an bogus size.
This check catches it and errors out versus we just go forward,
generate a bugus image, which you then flash and find that things do
not work, and then spend your time for further investigations. Nothing
more than that.

Artem.

Re: [PATCH] mkfs.ubifs: remove the check for UBIFS_MAX_LEB_SZ

[Dongsheng Yang]

On 08/24/2015 02:32 PM, Artem Bityutskiy wrote:
> On Mon, 2015-08-24 at 09:07 +0800, Dongsheng Yang wrote:
>> On 08/18/2015 04:52 PM, Artem Bityutskiy wrote:
>>> On Tue, 2015-08-18 at 12:52 +0800, Dongsheng Yang wrote:
>>>> There is a commit 92ed6c0 to increase UBIFS_MAX_LEB_SZ
>>>> to 2MiB. But recently, as the leb size become larger and
>>>> larger, 2MiB is not a suitable limit any more.
>>>>
>>>> Then remove this check in mkfs.ubifs
>>>>
>>>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>>>> ---
>>>> NOTE:
>>>> 	I am not sure the reason why we have to
>>>> limit the leb size in mkfs.ubifs. Because
>>>> I did not find any reason for it, I send
>>>> this patch out. It's very possible I am
>>>> mising something.
>>>
>>> Well, this is sanity check for the user input. If you accidentally
>>> added few zeroes, we want to spot this and inform you, and you may
>>> appreciate that we did not just created a bugus image for you.
>>> That's
>>> the idea.
>>
>> Sorry, Artem, I was trying to understand it, but I did not got the
>> point. Could you give me some more information about the idea? Maybe
>> an example?
>
> Well, this is defensive programming concept.

Ha, okey, got it. That's good.

But I got a device with ped_size=32M. (Although that would affect
ubi/ubifs performance negatively, that's another topic) I am
not sure increasing the MAX_LEB_SIZE to 32M is a good idea, at least
I don't think it's convincing to others that "I have a device with 32M
peb, so I will increase it to 32M". Could you give me some more suggestion?

Thanx

> Something goes wrong, due
> to bad user input and/or a bug we get to this place with an bogus size.
> This check catches it and errors out versus we just go forward,
> generate a bugus image, which you then flash and find that things do
> not work, and then spend your time for further investigations. Nothing
> more than that.
>
> Artem.
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>

Re: [PATCH] mkfs.ubifs: remove the check for UBIFS_MAX_LEB_SZ

[Dongsheng Yang]

On 08/24/2015 03:05 PM, Artem Bityutskiy wrote:
> On Mon, 2015-08-24 at 14:37 +0800, Dongsheng Yang wrote:
>> On 08/24/2015 02:32 PM, Artem Bityutskiy wrote:
>>> On Mon, 2015-08-24 at 09:07 +0800, Dongsheng Yang wrote:
>>>> On 08/18/2015 04:52 PM, Artem Bityutskiy wrote:
>>>>> On Tue, 2015-08-18 at 12:52 +0800, Dongsheng Yang wrote:
>>>>>> There is a commit 92ed6c0 to increase UBIFS_MAX_LEB_SZ
>>>>>> to 2MiB. But recently, as the leb size become larger and
>>>>>> larger, 2MiB is not a suitable limit any more.
>>>>>>
>>>>>> Then remove this check in mkfs.ubifs
>>>>>>
>>>>>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>>>>>> ---
>>>>>> NOTE:
>>>>>> 	I am not sure the reason why we have to
>>>>>> limit the leb size in mkfs.ubifs. Because
>>>>>> I did not find any reason for it, I send
>>>>>> this patch out. It's very possible I am
>>>>>> mising something.
>>>>>
>>>>> Well, this is sanity check for the user input. If you
>>>>> accidentally
>>>>> added few zeroes, we want to spot this and inform you, and you
>>>>> may
>>>>> appreciate that we did not just created a bugus image for you.
>>>>> That's
>>>>> the idea.
>>>>
>>>> Sorry, Artem, I was trying to understand it, but I did not got
>>>> the
>>>> point. Could you give me some more information about the idea?
>>>> Maybe
>>>> an example?
>>>
>>> Well, this is defensive programming concept.
>>
>> Ha, okey, got it. That's good.
>>
>> But I got a device with ped_size=32M. (Although that would affect
>> ubi/ubifs performance negatively, that's another topic) I am
>> not sure increasing the MAX_LEB_SIZE to 32M is a good idea, at least
>> I don't think it's convincing to others that "I have a device with
>> 32M
>> peb, so I will increase it to 32M". Could you give me some more
>> suggestion?
>
> Well, defensive programming is just a generally useful concept, it
> helps to reveal problems, rather than help them hide.
>
> But sometimes it stands on your way, in which case you need to relax
> the defense. I already suggested to change the constant to the size you
> need. I'd appreciate also a comment telling about the system which
> needs 32MiB, if possible.

Got it. Thanx

Yang
>
> Artem.
> .
>

Re: [PATCH] mkfs.ubifs: remove the check for UBIFS_MAX_LEB_SZ

[Artem Bityutskiy]

On Mon, 2015-08-24 at 14:37 +0800, Dongsheng Yang wrote:
> On 08/24/2015 02:32 PM, Artem Bityutskiy wrote:
> > On Mon, 2015-08-24 at 09:07 +0800, Dongsheng Yang wrote:
> > > On 08/18/2015 04:52 PM, Artem Bityutskiy wrote:
> > > > On Tue, 2015-08-18 at 12:52 +0800, Dongsheng Yang wrote:
> > > > > There is a commit 92ed6c0 to increase UBIFS_MAX_LEB_SZ
> > > > > to 2MiB. But recently, as the leb size become larger and
> > > > > larger, 2MiB is not a suitable limit any more.
> > > > > 
> > > > > Then remove this check in mkfs.ubifs
> > > > > 
> > > > > Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> > > > > ---
> > > > > NOTE:
> > > > > 	I am not sure the reason why we have to
> > > > > limit the leb size in mkfs.ubifs. Because
> > > > > I did not find any reason for it, I send
> > > > > this patch out. It's very possible I am
> > > > > mising something.
> > > > 
> > > > Well, this is sanity check for the user input. If you
> > > > accidentally
> > > > added few zeroes, we want to spot this and inform you, and you
> > > > may
> > > > appreciate that we did not just created a bugus image for you.
> > > > That's
> > > > the idea.
> > > 
> > > Sorry, Artem, I was trying to understand it, but I did not got
> > > the
> > > point. Could you give me some more information about the idea?
> > > Maybe
> > > an example?
> > 
> > Well, this is defensive programming concept.
> 
> Ha, okey, got it. That's good.
> 
> But I got a device with ped_size=32M. (Although that would affect
> ubi/ubifs performance negatively, that's another topic) I am
> not sure increasing the MAX_LEB_SIZE to 32M is a good idea, at least
> I don't think it's convincing to others that "I have a device with
> 32M
> peb, so I will increase it to 32M". Could you give me some more
> suggestion?

Well, defensive programming is just a generally useful concept, it
helps to reveal problems, rather than help them hide.

But sometimes it stands on your way, in which case you need to relax
the defense. I already suggested to change the constant to the size you
need. I'd appreciate also a comment telling about the system which
needs 32MiB, if possible.

Artem.