* [PATCH] mkfs.ubifs: remove the check for UBIFS_MAX_LEB_SZ @ 2015-08-18 4:52 Dongsheng Yang 2015-08-18 8:52 ` Artem Bityutskiy 0 siblings, 1 reply; 7+ messages in thread From: Dongsheng Yang @ 2015-08-18 4:52 UTC (permalink / raw) To: dedekind1, richard.weinberger, computersforpeace Cc: linux-mtd, 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] mkfs.ubifs: remove the check for UBIFS_MAX_LEB_SZ 2015-08-18 4:52 [PATCH] mkfs.ubifs: remove the check for UBIFS_MAX_LEB_SZ Dongsheng Yang @ 2015-08-18 8:52 ` Artem Bityutskiy 2015-08-24 1:07 ` Dongsheng Yang 0 siblings, 1 reply; 7+ messages in thread From: Artem Bityutskiy @ 2015-08-18 8:52 UTC (permalink / raw) To: Dongsheng Yang, richard.weinberger, computersforpeace; +Cc: linux-mtd 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mkfs.ubifs: remove the check for UBIFS_MAX_LEB_SZ 2015-08-18 8:52 ` Artem Bityutskiy @ 2015-08-24 1:07 ` Dongsheng Yang 2015-08-24 6:32 ` Artem Bityutskiy 0 siblings, 1 reply; 7+ messages in thread From: Dongsheng Yang @ 2015-08-24 1:07 UTC (permalink / raw) To: dedekind1, richard.weinberger, computersforpeace; +Cc: linux-mtd 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. > . > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mkfs.ubifs: remove the check for UBIFS_MAX_LEB_SZ 2015-08-24 1:07 ` Dongsheng Yang @ 2015-08-24 6:32 ` Artem Bityutskiy 2015-08-24 6:37 ` Dongsheng Yang 0 siblings, 1 reply; 7+ messages in thread From: Artem Bityutskiy @ 2015-08-24 6:32 UTC (permalink / raw) To: Dongsheng Yang, richard.weinberger, computersforpeace; +Cc: linux-mtd 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mkfs.ubifs: remove the check for UBIFS_MAX_LEB_SZ 2015-08-24 6:32 ` Artem Bityutskiy @ 2015-08-24 6:37 ` Dongsheng Yang 2015-08-24 7:05 ` Artem Bityutskiy 0 siblings, 1 reply; 7+ messages in thread From: Dongsheng Yang @ 2015-08-24 6:37 UTC (permalink / raw) To: dedekind1, richard.weinberger, computersforpeace; +Cc: linux-mtd 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/ > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mkfs.ubifs: remove the check for UBIFS_MAX_LEB_SZ 2015-08-24 6:37 ` Dongsheng Yang @ 2015-08-24 7:05 ` Artem Bityutskiy 2015-08-24 7:04 ` Dongsheng Yang 0 siblings, 1 reply; 7+ messages in thread From: Artem Bityutskiy @ 2015-08-24 7:05 UTC (permalink / raw) To: Dongsheng Yang, richard.weinberger, computersforpeace; +Cc: linux-mtd 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mkfs.ubifs: remove the check for UBIFS_MAX_LEB_SZ 2015-08-24 7:05 ` Artem Bityutskiy @ 2015-08-24 7:04 ` Dongsheng Yang 0 siblings, 0 replies; 7+ messages in thread From: Dongsheng Yang @ 2015-08-24 7:04 UTC (permalink / raw) To: dedekind1, richard.weinberger, computersforpeace; +Cc: linux-mtd 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. > . > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-08-24 7:11 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-08-18 4:52 [PATCH] mkfs.ubifs: remove the check for UBIFS_MAX_LEB_SZ Dongsheng Yang 2015-08-18 8:52 ` Artem Bityutskiy 2015-08-24 1:07 ` Dongsheng Yang 2015-08-24 6:32 ` Artem Bityutskiy 2015-08-24 6:37 ` Dongsheng Yang 2015-08-24 7:05 ` Artem Bityutskiy 2015-08-24 7:04 ` Dongsheng Yang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).