| Message ID | 1472892762-20216-3-git-send-email-dev@kresin.me |
|---|---|
| State | RFC |
| Headers | show |
On 3 September 2016 at 16:52, Mathias Kresin <dev@kresin.me> wrote: > At the moment the padding steps are hardcoded. Especially images for > devices with a 4K sector size can be unnecessarily bloated using the > hardcoded padding steps. > > It has been observed that 192Kb of padding was added to the image of a > 4MB device, albeit due to the 4K sector size the minimum required extra > padding for the jffs2 rootfs_data is 20Kb. > > In worst case it means that the image-size check could fail albeit > there is enough space for all selected packages > > For device build code not exposing the blocksize, use the hardcoded > padding further on. > > Signed-off-by: Mathias Kresin <dev@kresin.me> > --- > include/image-commands.mk | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/image-commands.mk b/include/image-commands.mk > index 3b9ea3c..76e42ab 100644 > --- a/include/image-commands.mk > +++ b/include/image-commands.mk > @@ -131,7 +131,8 @@ define Build/pad-to > endef > > define Build/pad-rootfs > - $(STAGING_DIR_HOST)/bin/padjffs2 $@ $(1) 4 8 16 64 128 256 > + $(STAGING_DIR_HOST)/bin/padjffs2 $@ $(1) \ > + $(if $(BLOCKSIZE),$(BLOCKSIZE:%k=%),4 8 16 64 128 256) > endef > Searching through the code, there are a lot places where BLOCKSIZE is defined with KiB as the unit suffix or without unit at all (in bytes) yousong > define Build/pad-offset > -- > 2.7.4 > > > _______________________________________________ > Lede-dev mailing list > Lede-dev@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/lede-dev
05.09.2016 04:02, Yousong Zhou: > On 3 September 2016 at 16:52, Mathias Kresin <dev@kresin.me> wrote: >> At the moment the padding steps are hardcoded. Especially images for >> devices with a 4K sector size can be unnecessarily bloated using the >> hardcoded padding steps. >> >> It has been observed that 192Kb of padding was added to the image of a >> 4MB device, albeit due to the 4K sector size the minimum required extra >> padding for the jffs2 rootfs_data is 20Kb. >> >> In worst case it means that the image-size check could fail albeit >> there is enough space for all selected packages >> >> For device build code not exposing the blocksize, use the hardcoded >> padding further on. >> >> Signed-off-by: Mathias Kresin <dev@kresin.me> >> --- >> include/image-commands.mk | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/include/image-commands.mk b/include/image-commands.mk >> index 3b9ea3c..76e42ab 100644 >> --- a/include/image-commands.mk >> +++ b/include/image-commands.mk >> @@ -131,7 +131,8 @@ define Build/pad-to >> endef >> >> define Build/pad-rootfs >> - $(STAGING_DIR_HOST)/bin/padjffs2 $@ $(1) 4 8 16 64 128 256 >> + $(STAGING_DIR_HOST)/bin/padjffs2 $@ $(1) \ >> + $(if $(BLOCKSIZE),$(BLOCKSIZE:%k=%),4 8 16 64 128 256) >> endef >> > > Searching through the code, there are a lot places where BLOCKSIZE is > defined with KiB as the unit suffix or without unit at all (in bytes) Hey Yousong, thanks a lot for having a look at this. In all cases the unit suffix is KiB, the BLOCKSIZE is passed to append-ubi (and further to scripts/ubinize-image.sh => ubinize) and never to pad-rootfs. For some reasons ubinize uses the unit suffixes KiB, MiB and GiB (or no unit for bytes). I've already a patch in my local tree to unify the unit suffix used in BLOCKSIZE to k and change it back to KiB in append-ubi. This way the BLOCKSIZE can be reused at a few places. But before pushing this further I would like to ensure that I'm doing it the right way and don't introduce hackish code. The cases where the BLOCKSIZE is used without unit are also unrelated. The block size is passed to dd, genext2fs or ubinize. Mathias
diff --git a/include/image-commands.mk b/include/image-commands.mk index 3b9ea3c..76e42ab 100644 --- a/include/image-commands.mk +++ b/include/image-commands.mk @@ -131,7 +131,8 @@ define Build/pad-to endef define Build/pad-rootfs - $(STAGING_DIR_HOST)/bin/padjffs2 $@ $(1) 4 8 16 64 128 256 + $(STAGING_DIR_HOST)/bin/padjffs2 $@ $(1) \ + $(if $(BLOCKSIZE),$(BLOCKSIZE:%k=%),4 8 16 64 128 256) endef define Build/pad-offset
At the moment the padding steps are hardcoded. Especially images for devices with a 4K sector size can be unnecessarily bloated using the hardcoded padding steps. It has been observed that 192Kb of padding was added to the image of a 4MB device, albeit due to the 4K sector size the minimum required extra padding for the jffs2 rootfs_data is 20Kb. In worst case it means that the image-size check could fail albeit there is enough space for all selected packages For device build code not exposing the blocksize, use the hardcoded padding further on. Signed-off-by: Mathias Kresin <dev@kresin.me> --- include/image-commands.mk | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)