All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] initramfs: Fix disabling of initramfs (and its compression)
@ 2017-05-21  3:33 Florian Fainelli
  2017-05-22  2:46 ` Nicholas Piggin
  0 siblings, 1 reply; 10+ messages in thread
From: Florian Fainelli @ 2017-05-21  3:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: npiggin, ppandit, pebolle, mmarek, Florian Fainelli,
	Andrew Morton, Francisco Blas Izquierdo Riera (klondike),
	Daniel Thompson

Commit db2aa7fd15e8 ("initramfs: allow again choice of the embedded
initram compression algorithm") introduced the possibility to select the
initramfs compression algorithm from Kconfig and while this is a nice
feature it broke the use case described below.

Here is what my build system does:

- kernel is initially configured not to have an initramfs included
- build the user space root file system
- re-configure the kernel to have an initramfs included
(CONFIG_INITRAMFS_SOURCE="/path/to/romfs") and set relevant
CONFIG_INITRAMFS options, in my case, no compression option
(CONFIG_INITRAMFS_COMPRESSION_NONE)
- kernel is re-built with these options -> kernel+initramfs image is
  copied
- kernel is re-built again without these options -> kernel image is
  copied

Building a kernel without an initramfs means setting this option:

CONFIG_INITRAMFS_SOURCE="" (and this one only)

whereas building a kernel with an initramfs means setting these options:

CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
/home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
CONFIG_INITRAMFS_ROOT_UID=1000
CONFIG_INITRAMFS_ROOT_GID=1000
CONFIG_INITRAMFS_COMPRESSION_NONE=y
CONFIG_INITRAMFS_COMPRESSION=""

Commit db2aa7fd15e857891cefbada8348c8d938c7a2bc ("initramfs: allow again
choice of the embedded initram compression algorithm") is problematic
because CONFIG_INITRAMFS_COMPRESSION which is used to determine the
initramfs_data.cpio extension/compression is a string, and due to how
Kconfig works it will evaluate in order, how to assign it.

Setting CONFIG_INITRAMFS_COMPRESSION_NONE with
CONFIG_INITRAMFS_SOURCE="" cannot possibly work (because of the depends
on INITRAMFS_SOURCE!="" imposed on CONFIG_INITRAMFS_COMPRESSION ) yet we
still get CONFIG_INITRAMFS_COMPRESSION assigned to ".gz" because
CONFIG_RD_GZIP=y is set in my kernel, even when there is no initramfs
being built.

So we basically end-up generating two initramfs_data.cpio* files, one
without extension, and one with .gz. This causes usr/Makefile to track
usr/initramfs_data.cpio.gz, and not usr/initramfs_data.cpio anymore,
that is also largely problematic after
9e3596b0c6539e28546ff7c72a06576627068353 ("kbuild: initramfs cleanup,
set target from Kconfig") because we used to track all possible
initramfs_data files in the $(targets) variable before that commit.

The end result is that the kernel with an initramfs clearly does not
contain what we expect it to, it has a stale initramfs_data.cpio file
built into it, and we keep re-generating an initramfs_data.cpio.gz file
which is not the one that we want to include in the kernel image proper.

The fix consists in hiding CONFIG_INITRAMFS_COMPRESSION when
CONFIG_INITRAMFS_SOURCE="". This puts us back in a state to the pre-4.10
behavior where we can properly disable and re-enable initramfs within
the same kernel .config file, and be in control of what
CONFIG_INITRAMFS_COMPRESSION is set to.

Fixes: db2aa7fd15e8 ("initramfs: allow again choice of the embedded initram compression algorithm")
Fixes: 9e3596b0c653 ("kbuild: initramfs cleanup, set target from Kconfig")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 usr/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/usr/Kconfig b/usr/Kconfig
index c0c48507e44e..ad0543e21760 100644
--- a/usr/Kconfig
+++ b/usr/Kconfig
@@ -220,6 +220,7 @@ config INITRAMFS_COMPRESSION_LZ4
 endchoice
 
 config INITRAMFS_COMPRESSION
+	depends on INITRAMFS_SOURCE!=""
 	string
 	default ""      if INITRAMFS_COMPRESSION_NONE
 	default ".gz"   if INITRAMFS_COMPRESSION_GZIP
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] initramfs: Fix disabling of initramfs (and its compression)
  2017-05-21  3:33 [PATCH] initramfs: Fix disabling of initramfs (and its compression) Florian Fainelli
@ 2017-05-22  2:46 ` Nicholas Piggin
  2017-08-28 20:03   ` Florian Fainelli
  0 siblings, 1 reply; 10+ messages in thread
From: Nicholas Piggin @ 2017-05-22  2:46 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-kernel, ppandit, pebolle, mmarek, Andrew Morton,
	Francisco Blas Izquierdo Riera (klondike),
	Daniel Thompson

On Sat, 20 May 2017 20:33:35 -0700
Florian Fainelli <f.fainelli@gmail.com> wrote:

> Commit db2aa7fd15e8 ("initramfs: allow again choice of the embedded
> initram compression algorithm") introduced the possibility to select the
> initramfs compression algorithm from Kconfig and while this is a nice
> feature it broke the use case described below.
> 
> Here is what my build system does:
> 
> - kernel is initially configured not to have an initramfs included
> - build the user space root file system
> - re-configure the kernel to have an initramfs included
> (CONFIG_INITRAMFS_SOURCE="/path/to/romfs") and set relevant
> CONFIG_INITRAMFS options, in my case, no compression option
> (CONFIG_INITRAMFS_COMPRESSION_NONE)
> - kernel is re-built with these options -> kernel+initramfs image is
>   copied
> - kernel is re-built again without these options -> kernel image is
>   copied
> 
> Building a kernel without an initramfs means setting this option:
> 
> CONFIG_INITRAMFS_SOURCE="" (and this one only)
> 
> whereas building a kernel with an initramfs means setting these options:
> 
> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
> CONFIG_INITRAMFS_ROOT_UID=1000
> CONFIG_INITRAMFS_ROOT_GID=1000
> CONFIG_INITRAMFS_COMPRESSION_NONE=y
> CONFIG_INITRAMFS_COMPRESSION=""
> 
> Commit db2aa7fd15e857891cefbada8348c8d938c7a2bc ("initramfs: allow again
> choice of the embedded initram compression algorithm") is problematic
> because CONFIG_INITRAMFS_COMPRESSION which is used to determine the
> initramfs_data.cpio extension/compression is a string, and due to how
> Kconfig works it will evaluate in order, how to assign it.
> 
> Setting CONFIG_INITRAMFS_COMPRESSION_NONE with
> CONFIG_INITRAMFS_SOURCE="" cannot possibly work (because of the depends
> on INITRAMFS_SOURCE!="" imposed on CONFIG_INITRAMFS_COMPRESSION ) yet we
> still get CONFIG_INITRAMFS_COMPRESSION assigned to ".gz" because
> CONFIG_RD_GZIP=y is set in my kernel, even when there is no initramfs
> being built.
> 
> So we basically end-up generating two initramfs_data.cpio* files, one
> without extension, and one with .gz. This causes usr/Makefile to track
> usr/initramfs_data.cpio.gz, and not usr/initramfs_data.cpio anymore,
> that is also largely problematic after
> 9e3596b0c6539e28546ff7c72a06576627068353 ("kbuild: initramfs cleanup,
> set target from Kconfig") because we used to track all possible
> initramfs_data files in the $(targets) variable before that commit.
> 
> The end result is that the kernel with an initramfs clearly does not
> contain what we expect it to, it has a stale initramfs_data.cpio file
> built into it, and we keep re-generating an initramfs_data.cpio.gz file
> which is not the one that we want to include in the kernel image proper.
> 
> The fix consists in hiding CONFIG_INITRAMFS_COMPRESSION when
> CONFIG_INITRAMFS_SOURCE="". This puts us back in a state to the pre-4.10
> behavior where we can properly disable and re-enable initramfs within
> the same kernel .config file, and be in control of what
> CONFIG_INITRAMFS_COMPRESSION is set to.
> 
> Fixes: db2aa7fd15e8 ("initramfs: allow again choice of the embedded initram compression algorithm")
> Fixes: 9e3596b0c653 ("kbuild: initramfs cleanup, set target from Kconfig")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

This is very thorough, thank you for tracking it down and fixing it.

I can't say I've worked through the problem in the code, but your
changelog and the proposed fix seem reasonable to me. So for what
it's worth:

Acked-by: Nicholas Piggin <npiggin@gmail.com>

> ---
>  usr/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/usr/Kconfig b/usr/Kconfig
> index c0c48507e44e..ad0543e21760 100644
> --- a/usr/Kconfig
> +++ b/usr/Kconfig
> @@ -220,6 +220,7 @@ config INITRAMFS_COMPRESSION_LZ4
>  endchoice
>  
>  config INITRAMFS_COMPRESSION
> +	depends on INITRAMFS_SOURCE!=""
>  	string
>  	default ""      if INITRAMFS_COMPRESSION_NONE
>  	default ".gz"   if INITRAMFS_COMPRESSION_GZIP

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] initramfs: Fix disabling of initramfs (and its compression)
  2017-05-22  2:46 ` Nicholas Piggin
@ 2017-08-28 20:03   ` Florian Fainelli
  2017-08-29  3:09     ` Nicholas Piggin
  0 siblings, 1 reply; 10+ messages in thread
From: Florian Fainelli @ 2017-08-28 20:03 UTC (permalink / raw)
  To: Nicholas Piggin, Francisco Blas Izquierdo Riera (klondike)
  Cc: linux-kernel, ppandit, pebolle, mmarek, Andrew Morton, Daniel Thompson

On 05/21/2017 07:46 PM, Nicholas Piggin wrote:
> On Sat, 20 May 2017 20:33:35 -0700
> Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
>> Commit db2aa7fd15e8 ("initramfs: allow again choice of the embedded
>> initram compression algorithm") introduced the possibility to select the
>> initramfs compression algorithm from Kconfig and while this is a nice
>> feature it broke the use case described below.
>>
>> Here is what my build system does:
>>
>> - kernel is initially configured not to have an initramfs included
>> - build the user space root file system
>> - re-configure the kernel to have an initramfs included
>> (CONFIG_INITRAMFS_SOURCE="/path/to/romfs") and set relevant
>> CONFIG_INITRAMFS options, in my case, no compression option
>> (CONFIG_INITRAMFS_COMPRESSION_NONE)
>> - kernel is re-built with these options -> kernel+initramfs image is
>>   copied
>> - kernel is re-built again without these options -> kernel image is
>>   copied
>>
>> Building a kernel without an initramfs means setting this option:
>>
>> CONFIG_INITRAMFS_SOURCE="" (and this one only)
>>
>> whereas building a kernel with an initramfs means setting these options:
>>
>> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
>> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
>> CONFIG_INITRAMFS_ROOT_UID=1000
>> CONFIG_INITRAMFS_ROOT_GID=1000
>> CONFIG_INITRAMFS_COMPRESSION_NONE=y
>> CONFIG_INITRAMFS_COMPRESSION=""
>>
>> Commit db2aa7fd15e857891cefbada8348c8d938c7a2bc ("initramfs: allow again
>> choice of the embedded initram compression algorithm") is problematic
>> because CONFIG_INITRAMFS_COMPRESSION which is used to determine the
>> initramfs_data.cpio extension/compression is a string, and due to how
>> Kconfig works it will evaluate in order, how to assign it.
>>
>> Setting CONFIG_INITRAMFS_COMPRESSION_NONE with
>> CONFIG_INITRAMFS_SOURCE="" cannot possibly work (because of the depends
>> on INITRAMFS_SOURCE!="" imposed on CONFIG_INITRAMFS_COMPRESSION ) yet we
>> still get CONFIG_INITRAMFS_COMPRESSION assigned to ".gz" because
>> CONFIG_RD_GZIP=y is set in my kernel, even when there is no initramfs
>> being built.
>>
>> So we basically end-up generating two initramfs_data.cpio* files, one
>> without extension, and one with .gz. This causes usr/Makefile to track
>> usr/initramfs_data.cpio.gz, and not usr/initramfs_data.cpio anymore,
>> that is also largely problematic after
>> 9e3596b0c6539e28546ff7c72a06576627068353 ("kbuild: initramfs cleanup,
>> set target from Kconfig") because we used to track all possible
>> initramfs_data files in the $(targets) variable before that commit.
>>
>> The end result is that the kernel with an initramfs clearly does not
>> contain what we expect it to, it has a stale initramfs_data.cpio file
>> built into it, and we keep re-generating an initramfs_data.cpio.gz file
>> which is not the one that we want to include in the kernel image proper.
>>
>> The fix consists in hiding CONFIG_INITRAMFS_COMPRESSION when
>> CONFIG_INITRAMFS_SOURCE="". This puts us back in a state to the pre-4.10
>> behavior where we can properly disable and re-enable initramfs within
>> the same kernel .config file, and be in control of what
>> CONFIG_INITRAMFS_COMPRESSION is set to.
>>
>> Fixes: db2aa7fd15e8 ("initramfs: allow again choice of the embedded initram compression algorithm")
>> Fixes: 9e3596b0c653 ("kbuild: initramfs cleanup, set target from Kconfig")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> This is very thorough, thank you for tracking it down and fixing it.
> 
> I can't say I've worked through the problem in the code, but your
> changelog and the proposed fix seem reasonable to me. So for what
> it's worth:
> 
> Acked-by: Nicholas Piggin <npiggin@gmail.com>

Well, I am looking at this again, and it's still broken, the same test
case is involved, except this time, I am switching beween no-initramfs
and initramfs with gzip compression (the key thing is using a
compression of some sort). The end result is the following:

- change stuff in the rootfs
- build the kernel with initramfs, CONFIG_INITRAMFS_COMPRESSION_GZIP=y,
usr/initramfs_data.cpio.gz gets generated correctly the first time
- build the kernel without initramfs,
CONFIG_INITRAMFS_COMPRESSION_NONE=y, usr/initramfs_data.cpio gets generated

Now back to step 1 add some files, and we can see that
usr/initramfs_data.cpio.gz is now stale from before...

So while my earlier fix switched the initramfs w/o compression to no
initramfs rebuild, now this does not work because we still have two
files left to be tracked:

usr/initramfs_data.cpio (no compression, or when initramfs is disabled)
and usr/initramfs_data.cpio.$(suffix_y)

How would you go about solving this?



> 
>> ---
>>  usr/Kconfig | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/usr/Kconfig b/usr/Kconfig
>> index c0c48507e44e..ad0543e21760 100644
>> --- a/usr/Kconfig
>> +++ b/usr/Kconfig
>> @@ -220,6 +220,7 @@ config INITRAMFS_COMPRESSION_LZ4
>>  endchoice
>>  
>>  config INITRAMFS_COMPRESSION
>> +	depends on INITRAMFS_SOURCE!=""
>>  	string
>>  	default ""      if INITRAMFS_COMPRESSION_NONE
>>  	default ".gz"   if INITRAMFS_COMPRESSION_GZIP
> 


-- 
Florian

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] initramfs: Fix disabling of initramfs (and its compression)
  2017-08-28 20:03   ` Florian Fainelli
@ 2017-08-29  3:09     ` Nicholas Piggin
  2017-08-29  3:52       ` Florian Fainelli
  2017-09-07 12:50       ` Florian Fainelli
  0 siblings, 2 replies; 10+ messages in thread
From: Nicholas Piggin @ 2017-08-29  3:09 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Francisco Blas Izquierdo Riera (klondike),
	linux-kernel, ppandit, pebolle, mmarek, Andrew Morton,
	Daniel Thompson

On Mon, 28 Aug 2017 13:03:31 -0700
Florian Fainelli <f.fainelli@gmail.com> wrote:

> On 05/21/2017 07:46 PM, Nicholas Piggin wrote:
> > On Sat, 20 May 2017 20:33:35 -0700
> > Florian Fainelli <f.fainelli@gmail.com> wrote:
> >   
> >> Commit db2aa7fd15e8 ("initramfs: allow again choice of the embedded
> >> initram compression algorithm") introduced the possibility to select the
> >> initramfs compression algorithm from Kconfig and while this is a nice
> >> feature it broke the use case described below.
> >>
> >> Here is what my build system does:
> >>
> >> - kernel is initially configured not to have an initramfs included
> >> - build the user space root file system
> >> - re-configure the kernel to have an initramfs included
> >> (CONFIG_INITRAMFS_SOURCE="/path/to/romfs") and set relevant
> >> CONFIG_INITRAMFS options, in my case, no compression option
> >> (CONFIG_INITRAMFS_COMPRESSION_NONE)
> >> - kernel is re-built with these options -> kernel+initramfs image is
> >>   copied
> >> - kernel is re-built again without these options -> kernel image is
> >>   copied
> >>
> >> Building a kernel without an initramfs means setting this option:
> >>
> >> CONFIG_INITRAMFS_SOURCE="" (and this one only)
> >>
> >> whereas building a kernel with an initramfs means setting these options:
> >>
> >> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
> >> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
> >> CONFIG_INITRAMFS_ROOT_UID=1000
> >> CONFIG_INITRAMFS_ROOT_GID=1000
> >> CONFIG_INITRAMFS_COMPRESSION_NONE=y
> >> CONFIG_INITRAMFS_COMPRESSION=""
> >>
> >> Commit db2aa7fd15e857891cefbada8348c8d938c7a2bc ("initramfs: allow again
> >> choice of the embedded initram compression algorithm") is problematic
> >> because CONFIG_INITRAMFS_COMPRESSION which is used to determine the
> >> initramfs_data.cpio extension/compression is a string, and due to how
> >> Kconfig works it will evaluate in order, how to assign it.
> >>
> >> Setting CONFIG_INITRAMFS_COMPRESSION_NONE with
> >> CONFIG_INITRAMFS_SOURCE="" cannot possibly work (because of the depends
> >> on INITRAMFS_SOURCE!="" imposed on CONFIG_INITRAMFS_COMPRESSION ) yet we
> >> still get CONFIG_INITRAMFS_COMPRESSION assigned to ".gz" because
> >> CONFIG_RD_GZIP=y is set in my kernel, even when there is no initramfs
> >> being built.
> >>
> >> So we basically end-up generating two initramfs_data.cpio* files, one
> >> without extension, and one with .gz. This causes usr/Makefile to track
> >> usr/initramfs_data.cpio.gz, and not usr/initramfs_data.cpio anymore,
> >> that is also largely problematic after
> >> 9e3596b0c6539e28546ff7c72a06576627068353 ("kbuild: initramfs cleanup,
> >> set target from Kconfig") because we used to track all possible
> >> initramfs_data files in the $(targets) variable before that commit.
> >>
> >> The end result is that the kernel with an initramfs clearly does not
> >> contain what we expect it to, it has a stale initramfs_data.cpio file
> >> built into it, and we keep re-generating an initramfs_data.cpio.gz file
> >> which is not the one that we want to include in the kernel image proper.
> >>
> >> The fix consists in hiding CONFIG_INITRAMFS_COMPRESSION when
> >> CONFIG_INITRAMFS_SOURCE="". This puts us back in a state to the pre-4.10
> >> behavior where we can properly disable and re-enable initramfs within
> >> the same kernel .config file, and be in control of what
> >> CONFIG_INITRAMFS_COMPRESSION is set to.
> >>
> >> Fixes: db2aa7fd15e8 ("initramfs: allow again choice of the embedded initram compression algorithm")
> >> Fixes: 9e3596b0c653 ("kbuild: initramfs cleanup, set target from Kconfig")
> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>  
> > 
> > This is very thorough, thank you for tracking it down and fixing it.
> > 
> > I can't say I've worked through the problem in the code, but your
> > changelog and the proposed fix seem reasonable to me. So for what
> > it's worth:
> > 
> > Acked-by: Nicholas Piggin <npiggin@gmail.com>  
> 
> Well, I am looking at this again, and it's still broken, the same test
> case is involved, except this time, I am switching beween no-initramfs
> and initramfs with gzip compression (the key thing is using a
> compression of some sort). The end result is the following:
> 
> - change stuff in the rootfs
> - build the kernel with initramfs, CONFIG_INITRAMFS_COMPRESSION_GZIP=y,
> usr/initramfs_data.cpio.gz gets generated correctly the first time
> - build the kernel without initramfs,
> CONFIG_INITRAMFS_COMPRESSION_NONE=y, usr/initramfs_data.cpio gets generated
> 
> Now back to step 1 add some files, and we can see that
> usr/initramfs_data.cpio.gz is now stale from before...
> 
> So while my earlier fix switched the initramfs w/o compression to no
> initramfs rebuild, now this does not work because we still have two
> files left to be tracked:
> 
> usr/initramfs_data.cpio (no compression, or when initramfs is disabled)
> and usr/initramfs_data.cpio.$(suffix_y)
> 
> How would you go about solving this?

I don't see the problem. When I change back to .gz, modify the source
directory, and rebuild, it rebuilds a new initramfs_data.cpio.gz here.

Thanks,
Nick

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] initramfs: Fix disabling of initramfs (and its compression)
  2017-08-29  3:09     ` Nicholas Piggin
@ 2017-08-29  3:52       ` Florian Fainelli
  2017-09-07 12:50       ` Florian Fainelli
  1 sibling, 0 replies; 10+ messages in thread
From: Florian Fainelli @ 2017-08-29  3:52 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: Francisco Blas Izquierdo Riera (klondike),
	linux-kernel, ppandit, pebolle, mmarek, Andrew Morton,
	Daniel Thompson



On 08/28/2017 08:09 PM, Nicholas Piggin wrote:
> On Mon, 28 Aug 2017 13:03:31 -0700
> Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
>> On 05/21/2017 07:46 PM, Nicholas Piggin wrote:
>>> On Sat, 20 May 2017 20:33:35 -0700
>>> Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>   
>>>> Commit db2aa7fd15e8 ("initramfs: allow again choice of the embedded
>>>> initram compression algorithm") introduced the possibility to select the
>>>> initramfs compression algorithm from Kconfig and while this is a nice
>>>> feature it broke the use case described below.
>>>>
>>>> Here is what my build system does:
>>>>
>>>> - kernel is initially configured not to have an initramfs included
>>>> - build the user space root file system
>>>> - re-configure the kernel to have an initramfs included
>>>> (CONFIG_INITRAMFS_SOURCE="/path/to/romfs") and set relevant
>>>> CONFIG_INITRAMFS options, in my case, no compression option
>>>> (CONFIG_INITRAMFS_COMPRESSION_NONE)
>>>> - kernel is re-built with these options -> kernel+initramfs image is
>>>>   copied
>>>> - kernel is re-built again without these options -> kernel image is
>>>>   copied
>>>>
>>>> Building a kernel without an initramfs means setting this option:
>>>>
>>>> CONFIG_INITRAMFS_SOURCE="" (and this one only)
>>>>
>>>> whereas building a kernel with an initramfs means setting these options:
>>>>
>>>> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
>>>> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
>>>> CONFIG_INITRAMFS_ROOT_UID=1000
>>>> CONFIG_INITRAMFS_ROOT_GID=1000
>>>> CONFIG_INITRAMFS_COMPRESSION_NONE=y
>>>> CONFIG_INITRAMFS_COMPRESSION=""
>>>>
>>>> Commit db2aa7fd15e857891cefbada8348c8d938c7a2bc ("initramfs: allow again
>>>> choice of the embedded initram compression algorithm") is problematic
>>>> because CONFIG_INITRAMFS_COMPRESSION which is used to determine the
>>>> initramfs_data.cpio extension/compression is a string, and due to how
>>>> Kconfig works it will evaluate in order, how to assign it.
>>>>
>>>> Setting CONFIG_INITRAMFS_COMPRESSION_NONE with
>>>> CONFIG_INITRAMFS_SOURCE="" cannot possibly work (because of the depends
>>>> on INITRAMFS_SOURCE!="" imposed on CONFIG_INITRAMFS_COMPRESSION ) yet we
>>>> still get CONFIG_INITRAMFS_COMPRESSION assigned to ".gz" because
>>>> CONFIG_RD_GZIP=y is set in my kernel, even when there is no initramfs
>>>> being built.
>>>>
>>>> So we basically end-up generating two initramfs_data.cpio* files, one
>>>> without extension, and one with .gz. This causes usr/Makefile to track
>>>> usr/initramfs_data.cpio.gz, and not usr/initramfs_data.cpio anymore,
>>>> that is also largely problematic after
>>>> 9e3596b0c6539e28546ff7c72a06576627068353 ("kbuild: initramfs cleanup,
>>>> set target from Kconfig") because we used to track all possible
>>>> initramfs_data files in the $(targets) variable before that commit.
>>>>
>>>> The end result is that the kernel with an initramfs clearly does not
>>>> contain what we expect it to, it has a stale initramfs_data.cpio file
>>>> built into it, and we keep re-generating an initramfs_data.cpio.gz file
>>>> which is not the one that we want to include in the kernel image proper.
>>>>
>>>> The fix consists in hiding CONFIG_INITRAMFS_COMPRESSION when
>>>> CONFIG_INITRAMFS_SOURCE="". This puts us back in a state to the pre-4.10
>>>> behavior where we can properly disable and re-enable initramfs within
>>>> the same kernel .config file, and be in control of what
>>>> CONFIG_INITRAMFS_COMPRESSION is set to.
>>>>
>>>> Fixes: db2aa7fd15e8 ("initramfs: allow again choice of the embedded initram compression algorithm")
>>>> Fixes: 9e3596b0c653 ("kbuild: initramfs cleanup, set target from Kconfig")
>>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>  
>>>
>>> This is very thorough, thank you for tracking it down and fixing it.
>>>
>>> I can't say I've worked through the problem in the code, but your
>>> changelog and the proposed fix seem reasonable to me. So for what
>>> it's worth:
>>>
>>> Acked-by: Nicholas Piggin <npiggin@gmail.com>  
>>
>> Well, I am looking at this again, and it's still broken, the same test
>> case is involved, except this time, I am switching beween no-initramfs
>> and initramfs with gzip compression (the key thing is using a
>> compression of some sort). The end result is the following:
>>
>> - change stuff in the rootfs
>> - build the kernel with initramfs, CONFIG_INITRAMFS_COMPRESSION_GZIP=y,
>> usr/initramfs_data.cpio.gz gets generated correctly the first time
>> - build the kernel without initramfs,
>> CONFIG_INITRAMFS_COMPRESSION_NONE=y, usr/initramfs_data.cpio gets generated
>>
>> Now back to step 1 add some files, and we can see that
>> usr/initramfs_data.cpio.gz is now stale from before...
>>
>> So while my earlier fix switched the initramfs w/o compression to no
>> initramfs rebuild, now this does not work because we still have two
>> files left to be tracked:
>>
>> usr/initramfs_data.cpio (no compression, or when initramfs is disabled)
>> and usr/initramfs_data.cpio.$(suffix_y)
>>
>> How would you go about solving this?
> 
> I don't see the problem. When I change back to .gz, modify the source
> directory, and rebuild, it rebuilds a new initramfs_data.cpio.gz here.

Can you share the different steps involved here? Here is what the build
system I am using is doing.

initramfs:
https://github.com/Broadcom/stblinux-4.9/blob/master/rootfs/Makefile#L169
no-initramfs:
https://github.com/Broadcom/stblinux-4.9/blob/master/rootfs/Makefile#L189

The script that does the config option switching is here:

https://github.com/Broadcom/stblinux-4.9/blob/master/rootfs/bin/config.pl#L1072

When you say change the source directory, you mean setting
CONFIG_INITRAMFS_SOURCE to one directory that contains your rootfs, and
then setting it to empty to disable that, right?

Here is what I have so far, only caveat (still triple checking) is that
we seem to be always re-generating usr/initramfs_cpio.$(suffix_y) which
is still an improvement over the current situation where we have a stale
file after the first creation.

diff --git a/usr/Makefile b/usr/Makefile
index 0b87e71c00fc..2d7d7f4b91f1 100644
--- a/usr/Makefile
+++ b/usr/Makefile
@@ -7,6 +7,7 @@ PHONY += klibcdirs

 suffix_y = $(subst $\",,$(CONFIG_INITRAMFS_COMPRESSION))
 datafile_y = initramfs_data.cpio$(suffix_y)
+datafile_d_y = .$(datafile_y).d
 AFLAGS_initramfs_data.o += -DINITRAMFS_IMAGE="usr/$(datafile_y)"


@@ -29,12 +30,12 @@ ramfs-args  := \
         $(if $(CONFIG_INITRAMFS_ROOT_UID), -u
$(CONFIG_INITRAMFS_ROOT_UID)) \
         $(if $(CONFIG_INITRAMFS_ROOT_GID), -g $(CONFIG_INITRAMFS_ROOT_GID))

-# .initramfs_data.cpio.d is used to identify all files included
+# $(datafile_d_y) is used to identify all files included
 # in initramfs and to detect if any files are added/removed.
 # Removed files are identified by directory timestamp being updated
 # The dependency list is generated by gen_initramfs.sh -l
-ifneq ($(wildcard $(obj)/.initramfs_data.cpio.d),)
-       include $(obj)/.initramfs_data.cpio.d
+ifneq ($(wildcard $(obj)/$(datafile_d_y)),)
+       include $(obj)/$(datafile_d_y)
 endif

 quiet_cmd_initfs = GEN     $@
@@ -52,5 +53,5 @@ $(deps_initramfs): klibcdirs
 # 3) If gen_init_cpio are newer than initramfs_data.cpio
 # 4) arguments to gen_initramfs.sh changes
 $(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
-       $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/.initramfs_data.cpio.d
+       $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/$(datafile_d_y)
        $(call if_changed,initfs)
-- 
Florian

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] initramfs: Fix disabling of initramfs (and its compression)
  2017-08-29  3:09     ` Nicholas Piggin
  2017-08-29  3:52       ` Florian Fainelli
@ 2017-09-07 12:50       ` Florian Fainelli
  2017-09-07 13:19         ` Nicholas Piggin
  1 sibling, 1 reply; 10+ messages in thread
From: Florian Fainelli @ 2017-09-07 12:50 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: Francisco Blas Izquierdo Riera (klondike),
	linux-kernel, ppandit, pebolle, mmarek, Andrew Morton,
	Daniel Thompson

On 08/28/2017 08:09 PM, Nicholas Piggin wrote:
> On Mon, 28 Aug 2017 13:03:31 -0700
> Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
>> On 05/21/2017 07:46 PM, Nicholas Piggin wrote:
>>> On Sat, 20 May 2017 20:33:35 -0700
>>> Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>   
>>>> Commit db2aa7fd15e8 ("initramfs: allow again choice of the embedded
>>>> initram compression algorithm") introduced the possibility to select the
>>>> initramfs compression algorithm from Kconfig and while this is a nice
>>>> feature it broke the use case described below.
>>>>
>>>> Here is what my build system does:
>>>>
>>>> - kernel is initially configured not to have an initramfs included
>>>> - build the user space root file system
>>>> - re-configure the kernel to have an initramfs included
>>>> (CONFIG_INITRAMFS_SOURCE="/path/to/romfs") and set relevant
>>>> CONFIG_INITRAMFS options, in my case, no compression option
>>>> (CONFIG_INITRAMFS_COMPRESSION_NONE)
>>>> - kernel is re-built with these options -> kernel+initramfs image is
>>>>   copied
>>>> - kernel is re-built again without these options -> kernel image is
>>>>   copied
>>>>
>>>> Building a kernel without an initramfs means setting this option:
>>>>
>>>> CONFIG_INITRAMFS_SOURCE="" (and this one only)
>>>>
>>>> whereas building a kernel with an initramfs means setting these options:
>>>>
>>>> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
>>>> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
>>>> CONFIG_INITRAMFS_ROOT_UID=1000
>>>> CONFIG_INITRAMFS_ROOT_GID=1000
>>>> CONFIG_INITRAMFS_COMPRESSION_NONE=y
>>>> CONFIG_INITRAMFS_COMPRESSION=""
>>>>
>>>> Commit db2aa7fd15e857891cefbada8348c8d938c7a2bc ("initramfs: allow again
>>>> choice of the embedded initram compression algorithm") is problematic
>>>> because CONFIG_INITRAMFS_COMPRESSION which is used to determine the
>>>> initramfs_data.cpio extension/compression is a string, and due to how
>>>> Kconfig works it will evaluate in order, how to assign it.
>>>>
>>>> Setting CONFIG_INITRAMFS_COMPRESSION_NONE with
>>>> CONFIG_INITRAMFS_SOURCE="" cannot possibly work (because of the depends
>>>> on INITRAMFS_SOURCE!="" imposed on CONFIG_INITRAMFS_COMPRESSION ) yet we
>>>> still get CONFIG_INITRAMFS_COMPRESSION assigned to ".gz" because
>>>> CONFIG_RD_GZIP=y is set in my kernel, even when there is no initramfs
>>>> being built.
>>>>
>>>> So we basically end-up generating two initramfs_data.cpio* files, one
>>>> without extension, and one with .gz. This causes usr/Makefile to track
>>>> usr/initramfs_data.cpio.gz, and not usr/initramfs_data.cpio anymore,
>>>> that is also largely problematic after
>>>> 9e3596b0c6539e28546ff7c72a06576627068353 ("kbuild: initramfs cleanup,
>>>> set target from Kconfig") because we used to track all possible
>>>> initramfs_data files in the $(targets) variable before that commit.
>>>>
>>>> The end result is that the kernel with an initramfs clearly does not
>>>> contain what we expect it to, it has a stale initramfs_data.cpio file
>>>> built into it, and we keep re-generating an initramfs_data.cpio.gz file
>>>> which is not the one that we want to include in the kernel image proper.
>>>>
>>>> The fix consists in hiding CONFIG_INITRAMFS_COMPRESSION when
>>>> CONFIG_INITRAMFS_SOURCE="". This puts us back in a state to the pre-4.10
>>>> behavior where we can properly disable and re-enable initramfs within
>>>> the same kernel .config file, and be in control of what
>>>> CONFIG_INITRAMFS_COMPRESSION is set to.
>>>>
>>>> Fixes: db2aa7fd15e8 ("initramfs: allow again choice of the embedded initram compression algorithm")
>>>> Fixes: 9e3596b0c653 ("kbuild: initramfs cleanup, set target from Kconfig")
>>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>  
>>>
>>> This is very thorough, thank you for tracking it down and fixing it.
>>>
>>> I can't say I've worked through the problem in the code, but your
>>> changelog and the proposed fix seem reasonable to me. So for what
>>> it's worth:
>>>
>>> Acked-by: Nicholas Piggin <npiggin@gmail.com>  
>>
>> Well, I am looking at this again, and it's still broken, the same test
>> case is involved, except this time, I am switching beween no-initramfs
>> and initramfs with gzip compression (the key thing is using a
>> compression of some sort). The end result is the following:
>>
>> - change stuff in the rootfs
>> - build the kernel with initramfs, CONFIG_INITRAMFS_COMPRESSION_GZIP=y,
>> usr/initramfs_data.cpio.gz gets generated correctly the first time
>> - build the kernel without initramfs,
>> CONFIG_INITRAMFS_COMPRESSION_NONE=y, usr/initramfs_data.cpio gets generated
>>
>> Now back to step 1 add some files, and we can see that
>> usr/initramfs_data.cpio.gz is now stale from before...
>>
>> So while my earlier fix switched the initramfs w/o compression to no
>> initramfs rebuild, now this does not work because we still have two
>> files left to be tracked:
>>
>> usr/initramfs_data.cpio (no compression, or when initramfs is disabled)
>> and usr/initramfs_data.cpio.$(suffix_y)
>>
>> How would you go about solving this?
> 
> I don't see the problem. When I change back to .gz, modify the source
> directory, and rebuild, it rebuilds a new initramfs_data.cpio.gz here.

This is really puzzling me, so here is what is going on my side:

First time build w/ initramfs enabled:

grep INITRAMFS .config
CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
/home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
CONFIG_INITRAMFS_ROOT_UID=1000
CONFIG_INITRAMFS_ROOT_GID=1000
# CONFIG_INITRAMFS_COMPRESSION_NONE is not set
CONFIG_INITRAMFS_COMPRESSION_GZIP=y
# CONFIG_INITRAMFS_COMPRESSION_BZIP2 is not set
# CONFIG_INITRAMFS_COMPRESSION_LZMA is not set
# CONFIG_INITRAMFS_COMPRESSION_XZ is not set
# CONFIG_INITRAMFS_COMPRESSION_LZO is not set
# CONFIG_INITRAMFS_COMPRESSION_LZ4 is not set
CONFIG_INITRAMFS_COMPRESSION=".gz"

fainelli@fainelli-laptop:[~/../linux]$ ls -la usr
total 6.8M
drwxrwxr-x  2 fainelli fainelli 4.0K Sep  7 05:37 ./
drwxrwxr-x 27 fainelli fainelli  16K Sep  7 05:38 ../
-rw-rw-r--  1 fainelli fainelli  146 Sep  7 05:37 built-in.o
-rw-rw-r--  1 fainelli fainelli  112 Sep  7 05:37 .built-in.o.cmd
-rwxrwxr-x  1 fainelli fainelli  23K Sep  7 05:36 gen_init_cpio*
-rw-rw-r--  1 fainelli fainelli  13K Oct 13  2016 gen_init_cpio.c
-rw-rw-r--  1 fainelli fainelli 3.3K Sep  7 05:36 .gen_init_cpio.cmd
-rw-rw-r--  1 fainelli fainelli  151 Aug 28  2016 .gitignore
-rw-rw-r--  1 fainelli fainelli 9.6K Sep  7 05:36 .initramfs_data.cpio.d
-rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.cpio.gz
-rw-rw-r--  1 fainelli fainelli  220 Sep  7 05:37
.initramfs_data.cpio.gz.cmd
-rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.o
-rw-rw-r--  1 fainelli fainelli 2.7K Sep  7 05:37 .initramfs_data.o.cmd
-rw-rw-r--  1 fainelli fainelli 1.3K Aug 28  2016 initramfs_data.S
-rw-rw-r--  1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
-rw-rw-r--  1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
-rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:34 modules.builtin
-rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:35 modules.order

Now let's reconfigure this kernel w/o initramfs and rebuild:

grep INITRAMFS .config
CONFIG_INITRAMFS_SOURCE=""

fainelli@fainelli-laptop:[~/../linux]$ ls -la usr
total 3.6M
drwxrwxr-x  2 fainelli fainelli 4.0K Sep  7 05:44 ./
drwxrwxr-x 27 fainelli fainelli  16K Sep  7 05:44 ../
-rw-rw-r--  1 fainelli fainelli  146 Sep  7 05:44 built-in.o
-rw-rw-r--  1 fainelli fainelli  112 Sep  7 05:44 .built-in.o.cmd
-rwxrwxr-x  1 fainelli fainelli  23K Sep  7 05:36 gen_init_cpio*
-rw-rw-r--  1 fainelli fainelli  13K Oct 13  2016 gen_init_cpio.c
-rw-rw-r--  1 fainelli fainelli 3.3K Sep  7 05:36 .gen_init_cpio.cmd
-rw-rw-r--  1 fainelli fainelli  151 Aug 28  2016 .gitignore
-rw-rw-r--  1 fainelli fainelli  512 Sep  7 05:44 initramfs_data.cpio
-rw-rw-r--  1 fainelli fainelli  105 Sep  7 05:44 .initramfs_data.cpio.cmd
-rw-rw-r--  1 fainelli fainelli   52 Sep  7 05:44 .initramfs_data.cpio.d
-rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.cpio.gz
-rw-rw-r--  1 fainelli fainelli  220 Sep  7 05:37
.initramfs_data.cpio.gz.cmd
-rw-rw-r--  1 fainelli fainelli 1.3K Sep  7 05:44 initramfs_data.o
-rw-rw-r--  1 fainelli fainelli 2.7K Sep  7 05:44 .initramfs_data.o.cmd
-rw-rw-r--  1 fainelli fainelli 1.3K Aug 28  2016 initramfs_data.S
-rw-rw-r--  1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
-rw-rw-r--  1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
-rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:34 modules.builtin
-rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:35 modules.order

Now let's rebuild with a new application added to the root filesystem:

grep INITRAMFS .config
CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
/home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
CONFIG_INITRAMFS_ROOT_UID=1000
CONFIG_INITRAMFS_ROOT_GID=1000
# CONFIG_INITRAMFS_COMPRESSION_NONE is not set
CONFIG_INITRAMFS_COMPRESSION_GZIP=y
# CONFIG_INITRAMFS_COMPRESSION_BZIP2 is not set
# CONFIG_INITRAMFS_COMPRESSION_LZMA is not set
# CONFIG_INITRAMFS_COMPRESSION_XZ is not set
# CONFIG_INITRAMFS_COMPRESSION_LZO is not set
# CONFIG_INITRAMFS_COMPRESSION_LZ4 is not set
CONFIG_INITRAMFS_COMPRESSION=".gz"

We are not fine here:
scripts/kconfig/conf  --silentoldconfig Kconfig
#
# configuration written to .config
#
  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     scripts/mod/devicetable-offsets.h
  CHK     include/generated/utsrelease.h
  CHK     include/generated/timeconst.h
  CHK     include/generated/bounds.h
  CHK     include/generated/asm-offsets.h
  CALL    scripts/checksyscalls.sh
  CHK     include/generated/compile.h
  AS      usr/initramfs_data.o

^===== we should have called gzip to re-generate the initramfs, we did not

  AR      usr/built-in.o
  GEN     .version
  CHK     include/generated/compile.h
  UPD     include/generated/compile.h
  CC      init/version.o
  AR      init/built-in.o
  AR      built-in.o
  LD      vmlinux.o
  MODPOST vmlinux.o

And the timestamps here confirm that:

fainelli@fainelli-laptop:[~/../linux]$ ls -la usr
total 6.9M
drwxrwxr-x  2 fainelli fainelli 4.0K Sep  7 05:46 ./
drwxrwxr-x 27 fainelli fainelli  16K Sep  7 05:46 ../
-rw-rw-r--  1 fainelli fainelli  146 Sep  7 05:46 built-in.o
-rw-rw-r--  1 fainelli fainelli  112 Sep  7 05:46 .built-in.o.cmd
-rwxrwxr-x  1 fainelli fainelli  23K Sep  7 05:36 gen_init_cpio*
-rw-rw-r--  1 fainelli fainelli  13K Oct 13  2016 gen_init_cpio.c
-rw-rw-r--  1 fainelli fainelli 3.3K Sep  7 05:36 .gen_init_cpio.cmd
-rw-rw-r--  1 fainelli fainelli  151 Aug 28  2016 .gitignore
-rw-rw-r--  1 fainelli fainelli  512 Sep  7 05:44 initramfs_data.cpio
-rw-rw-r--  1 fainelli fainelli  105 Sep  7 05:44 .initramfs_data.cpio.cmd
-rw-rw-r--  1 fainelli fainelli  11K Sep  7 05:46 .initramfs_data.cpio.d
-rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.cpio.gz
-rw-rw-r--  1 fainelli fainelli  220 Sep  7 05:37
.initramfs_data.cpio.gz.cmd
-rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:46 initramfs_data.o
-rw-rw-r--  1 fainelli fainelli 2.7K Sep  7 05:46 .initramfs_data.o.cmd
-rw-rw-r--  1 fainelli fainelli 1.3K Aug 28  2016 initramfs_data.S
-rw-rw-r--  1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
-rw-rw-r--  1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
-rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:46 modules.builtin
-rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:46 modules.order

Worse yet, usr/.initramfs_data.cpio.d does list the binary that was just
added, but for some reason, this did not trigger a rebuild of
initramfs_data.cpio.gz do you see any reasons why?

Thanks!
-- 
Florian

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] initramfs: Fix disabling of initramfs (and its compression)
  2017-09-07 12:50       ` Florian Fainelli
@ 2017-09-07 13:19         ` Nicholas Piggin
  2017-09-08  4:36           ` Florian Fainelli
  0 siblings, 1 reply; 10+ messages in thread
From: Nicholas Piggin @ 2017-09-07 13:19 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Francisco Blas Izquierdo Riera (klondike),
	linux-kernel, ppandit, pebolle, mmarek, Andrew Morton,
	Daniel Thompson

On Thu, 7 Sep 2017 05:50:30 -0700
Florian Fainelli <f.fainelli@gmail.com> wrote:

> On 08/28/2017 08:09 PM, Nicholas Piggin wrote:
> > On Mon, 28 Aug 2017 13:03:31 -0700
> > Florian Fainelli <f.fainelli@gmail.com> wrote:
> >   
> >> On 05/21/2017 07:46 PM, Nicholas Piggin wrote:  
> >>> On Sat, 20 May 2017 20:33:35 -0700
> >>> Florian Fainelli <f.fainelli@gmail.com> wrote:
> >>>     
> >>>> Commit db2aa7fd15e8 ("initramfs: allow again choice of the embedded
> >>>> initram compression algorithm") introduced the possibility to select the
> >>>> initramfs compression algorithm from Kconfig and while this is a nice
> >>>> feature it broke the use case described below.
> >>>>
> >>>> Here is what my build system does:
> >>>>
> >>>> - kernel is initially configured not to have an initramfs included
> >>>> - build the user space root file system
> >>>> - re-configure the kernel to have an initramfs included
> >>>> (CONFIG_INITRAMFS_SOURCE="/path/to/romfs") and set relevant
> >>>> CONFIG_INITRAMFS options, in my case, no compression option
> >>>> (CONFIG_INITRAMFS_COMPRESSION_NONE)
> >>>> - kernel is re-built with these options -> kernel+initramfs image is
> >>>>   copied
> >>>> - kernel is re-built again without these options -> kernel image is
> >>>>   copied
> >>>>
> >>>> Building a kernel without an initramfs means setting this option:
> >>>>
> >>>> CONFIG_INITRAMFS_SOURCE="" (and this one only)
> >>>>
> >>>> whereas building a kernel with an initramfs means setting these options:
> >>>>
> >>>> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
> >>>> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
> >>>> CONFIG_INITRAMFS_ROOT_UID=1000
> >>>> CONFIG_INITRAMFS_ROOT_GID=1000
> >>>> CONFIG_INITRAMFS_COMPRESSION_NONE=y
> >>>> CONFIG_INITRAMFS_COMPRESSION=""
> >>>>
> >>>> Commit db2aa7fd15e857891cefbada8348c8d938c7a2bc ("initramfs: allow again
> >>>> choice of the embedded initram compression algorithm") is problematic
> >>>> because CONFIG_INITRAMFS_COMPRESSION which is used to determine the
> >>>> initramfs_data.cpio extension/compression is a string, and due to how
> >>>> Kconfig works it will evaluate in order, how to assign it.
> >>>>
> >>>> Setting CONFIG_INITRAMFS_COMPRESSION_NONE with
> >>>> CONFIG_INITRAMFS_SOURCE="" cannot possibly work (because of the depends
> >>>> on INITRAMFS_SOURCE!="" imposed on CONFIG_INITRAMFS_COMPRESSION ) yet we
> >>>> still get CONFIG_INITRAMFS_COMPRESSION assigned to ".gz" because
> >>>> CONFIG_RD_GZIP=y is set in my kernel, even when there is no initramfs
> >>>> being built.
> >>>>
> >>>> So we basically end-up generating two initramfs_data.cpio* files, one
> >>>> without extension, and one with .gz. This causes usr/Makefile to track
> >>>> usr/initramfs_data.cpio.gz, and not usr/initramfs_data.cpio anymore,
> >>>> that is also largely problematic after
> >>>> 9e3596b0c6539e28546ff7c72a06576627068353 ("kbuild: initramfs cleanup,
> >>>> set target from Kconfig") because we used to track all possible
> >>>> initramfs_data files in the $(targets) variable before that commit.
> >>>>
> >>>> The end result is that the kernel with an initramfs clearly does not
> >>>> contain what we expect it to, it has a stale initramfs_data.cpio file
> >>>> built into it, and we keep re-generating an initramfs_data.cpio.gz file
> >>>> which is not the one that we want to include in the kernel image proper.
> >>>>
> >>>> The fix consists in hiding CONFIG_INITRAMFS_COMPRESSION when
> >>>> CONFIG_INITRAMFS_SOURCE="". This puts us back in a state to the pre-4.10
> >>>> behavior where we can properly disable and re-enable initramfs within
> >>>> the same kernel .config file, and be in control of what
> >>>> CONFIG_INITRAMFS_COMPRESSION is set to.
> >>>>
> >>>> Fixes: db2aa7fd15e8 ("initramfs: allow again choice of the embedded initram compression algorithm")
> >>>> Fixes: 9e3596b0c653 ("kbuild: initramfs cleanup, set target from Kconfig")
> >>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>    
> >>>
> >>> This is very thorough, thank you for tracking it down and fixing it.
> >>>
> >>> I can't say I've worked through the problem in the code, but your
> >>> changelog and the proposed fix seem reasonable to me. So for what
> >>> it's worth:
> >>>
> >>> Acked-by: Nicholas Piggin <npiggin@gmail.com>    
> >>
> >> Well, I am looking at this again, and it's still broken, the same test
> >> case is involved, except this time, I am switching beween no-initramfs
> >> and initramfs with gzip compression (the key thing is using a
> >> compression of some sort). The end result is the following:
> >>
> >> - change stuff in the rootfs
> >> - build the kernel with initramfs, CONFIG_INITRAMFS_COMPRESSION_GZIP=y,
> >> usr/initramfs_data.cpio.gz gets generated correctly the first time
> >> - build the kernel without initramfs,
> >> CONFIG_INITRAMFS_COMPRESSION_NONE=y, usr/initramfs_data.cpio gets generated
> >>
> >> Now back to step 1 add some files, and we can see that
> >> usr/initramfs_data.cpio.gz is now stale from before...
> >>
> >> So while my earlier fix switched the initramfs w/o compression to no
> >> initramfs rebuild, now this does not work because we still have two
> >> files left to be tracked:
> >>
> >> usr/initramfs_data.cpio (no compression, or when initramfs is disabled)
> >> and usr/initramfs_data.cpio.$(suffix_y)
> >>
> >> How would you go about solving this?  
> > 
> > I don't see the problem. When I change back to .gz, modify the source
> > directory, and rebuild, it rebuilds a new initramfs_data.cpio.gz here.  
> 
> This is really puzzling me, so here is what is going on my side:
> 
> First time build w/ initramfs enabled:
> 
> grep INITRAMFS .config
> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
> CONFIG_INITRAMFS_ROOT_UID=1000
> CONFIG_INITRAMFS_ROOT_GID=1000
> # CONFIG_INITRAMFS_COMPRESSION_NONE is not set
> CONFIG_INITRAMFS_COMPRESSION_GZIP=y
> # CONFIG_INITRAMFS_COMPRESSION_BZIP2 is not set
> # CONFIG_INITRAMFS_COMPRESSION_LZMA is not set
> # CONFIG_INITRAMFS_COMPRESSION_XZ is not set
> # CONFIG_INITRAMFS_COMPRESSION_LZO is not set
> # CONFIG_INITRAMFS_COMPRESSION_LZ4 is not set
> CONFIG_INITRAMFS_COMPRESSION=".gz"
> 
> fainelli@fainelli-laptop:[~/../linux]$ ls -la usr
> total 6.8M
> drwxrwxr-x  2 fainelli fainelli 4.0K Sep  7 05:37 ./
> drwxrwxr-x 27 fainelli fainelli  16K Sep  7 05:38 ../
> -rw-rw-r--  1 fainelli fainelli  146 Sep  7 05:37 built-in.o
> -rw-rw-r--  1 fainelli fainelli  112 Sep  7 05:37 .built-in.o.cmd
> -rwxrwxr-x  1 fainelli fainelli  23K Sep  7 05:36 gen_init_cpio*
> -rw-rw-r--  1 fainelli fainelli  13K Oct 13  2016 gen_init_cpio.c
> -rw-rw-r--  1 fainelli fainelli 3.3K Sep  7 05:36 .gen_init_cpio.cmd
> -rw-rw-r--  1 fainelli fainelli  151 Aug 28  2016 .gitignore
> -rw-rw-r--  1 fainelli fainelli 9.6K Sep  7 05:36 .initramfs_data.cpio.d
> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.cpio.gz
> -rw-rw-r--  1 fainelli fainelli  220 Sep  7 05:37
> .initramfs_data.cpio.gz.cmd
> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.o
> -rw-rw-r--  1 fainelli fainelli 2.7K Sep  7 05:37 .initramfs_data.o.cmd
> -rw-rw-r--  1 fainelli fainelli 1.3K Aug 28  2016 initramfs_data.S
> -rw-rw-r--  1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
> -rw-rw-r--  1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:34 modules.builtin
> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:35 modules.order
> 
> Now let's reconfigure this kernel w/o initramfs and rebuild:
> 
> grep INITRAMFS .config
> CONFIG_INITRAMFS_SOURCE=""
> 
> fainelli@fainelli-laptop:[~/../linux]$ ls -la usr
> total 3.6M
> drwxrwxr-x  2 fainelli fainelli 4.0K Sep  7 05:44 ./
> drwxrwxr-x 27 fainelli fainelli  16K Sep  7 05:44 ../
> -rw-rw-r--  1 fainelli fainelli  146 Sep  7 05:44 built-in.o
> -rw-rw-r--  1 fainelli fainelli  112 Sep  7 05:44 .built-in.o.cmd
> -rwxrwxr-x  1 fainelli fainelli  23K Sep  7 05:36 gen_init_cpio*
> -rw-rw-r--  1 fainelli fainelli  13K Oct 13  2016 gen_init_cpio.c
> -rw-rw-r--  1 fainelli fainelli 3.3K Sep  7 05:36 .gen_init_cpio.cmd
> -rw-rw-r--  1 fainelli fainelli  151 Aug 28  2016 .gitignore
> -rw-rw-r--  1 fainelli fainelli  512 Sep  7 05:44 initramfs_data.cpio
> -rw-rw-r--  1 fainelli fainelli  105 Sep  7 05:44 .initramfs_data.cpio.cmd
> -rw-rw-r--  1 fainelli fainelli   52 Sep  7 05:44 .initramfs_data.cpio.d
> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.cpio.gz
> -rw-rw-r--  1 fainelli fainelli  220 Sep  7 05:37
> .initramfs_data.cpio.gz.cmd
> -rw-rw-r--  1 fainelli fainelli 1.3K Sep  7 05:44 initramfs_data.o
> -rw-rw-r--  1 fainelli fainelli 2.7K Sep  7 05:44 .initramfs_data.o.cmd
> -rw-rw-r--  1 fainelli fainelli 1.3K Aug 28  2016 initramfs_data.S
> -rw-rw-r--  1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
> -rw-rw-r--  1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:34 modules.builtin
> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:35 modules.order
> 
> Now let's rebuild with a new application added to the root filesystem:
> 
> grep INITRAMFS .config
> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
> CONFIG_INITRAMFS_ROOT_UID=1000
> CONFIG_INITRAMFS_ROOT_GID=1000
> # CONFIG_INITRAMFS_COMPRESSION_NONE is not set
> CONFIG_INITRAMFS_COMPRESSION_GZIP=y
> # CONFIG_INITRAMFS_COMPRESSION_BZIP2 is not set
> # CONFIG_INITRAMFS_COMPRESSION_LZMA is not set
> # CONFIG_INITRAMFS_COMPRESSION_XZ is not set
> # CONFIG_INITRAMFS_COMPRESSION_LZO is not set
> # CONFIG_INITRAMFS_COMPRESSION_LZ4 is not set
> CONFIG_INITRAMFS_COMPRESSION=".gz"
> 
> We are not fine here:
> scripts/kconfig/conf  --silentoldconfig Kconfig
> #
> # configuration written to .config
> #
>   CHK     include/config/kernel.release
>   CHK     include/generated/uapi/linux/version.h
>   CHK     scripts/mod/devicetable-offsets.h
>   CHK     include/generated/utsrelease.h
>   CHK     include/generated/timeconst.h
>   CHK     include/generated/bounds.h
>   CHK     include/generated/asm-offsets.h
>   CALL    scripts/checksyscalls.sh
>   CHK     include/generated/compile.h
>   AS      usr/initramfs_data.o
> 
> ^===== we should have called gzip to re-generate the initramfs, we did not
> 
>   AR      usr/built-in.o
>   GEN     .version
>   CHK     include/generated/compile.h
>   UPD     include/generated/compile.h
>   CC      init/version.o
>   AR      init/built-in.o
>   AR      built-in.o
>   LD      vmlinux.o
>   MODPOST vmlinux.o
> 
> And the timestamps here confirm that:
> 
> fainelli@fainelli-laptop:[~/../linux]$ ls -la usr
> total 6.9M
> drwxrwxr-x  2 fainelli fainelli 4.0K Sep  7 05:46 ./
> drwxrwxr-x 27 fainelli fainelli  16K Sep  7 05:46 ../
> -rw-rw-r--  1 fainelli fainelli  146 Sep  7 05:46 built-in.o
> -rw-rw-r--  1 fainelli fainelli  112 Sep  7 05:46 .built-in.o.cmd
> -rwxrwxr-x  1 fainelli fainelli  23K Sep  7 05:36 gen_init_cpio*
> -rw-rw-r--  1 fainelli fainelli  13K Oct 13  2016 gen_init_cpio.c
> -rw-rw-r--  1 fainelli fainelli 3.3K Sep  7 05:36 .gen_init_cpio.cmd
> -rw-rw-r--  1 fainelli fainelli  151 Aug 28  2016 .gitignore
> -rw-rw-r--  1 fainelli fainelli  512 Sep  7 05:44 initramfs_data.cpio
> -rw-rw-r--  1 fainelli fainelli  105 Sep  7 05:44 .initramfs_data.cpio.cmd
> -rw-rw-r--  1 fainelli fainelli  11K Sep  7 05:46 .initramfs_data.cpio.d
> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.cpio.gz
> -rw-rw-r--  1 fainelli fainelli  220 Sep  7 05:37
> .initramfs_data.cpio.gz.cmd
> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:46 initramfs_data.o
> -rw-rw-r--  1 fainelli fainelli 2.7K Sep  7 05:46 .initramfs_data.o.cmd
> -rw-rw-r--  1 fainelli fainelli 1.3K Aug 28  2016 initramfs_data.S
> -rw-rw-r--  1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
> -rw-rw-r--  1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:46 modules.builtin
> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:46 modules.order
> 
> Worse yet, usr/.initramfs_data.cpio.d does list the binary that was just
> added, but for some reason, this did not trigger a rebuild of
> initramfs_data.cpio.gz do you see any reasons why?

This rule  

$(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
        $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/.initramfs_data.cpio.d
        $(call if_changed,initfs)

does not have FORCE on the end, so I guess is not picking up the dependency
properly, perhaps?

Thanks,
Nick

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] initramfs: Fix disabling of initramfs (and its compression)
  2017-09-07 13:19         ` Nicholas Piggin
@ 2017-09-08  4:36           ` Florian Fainelli
  2017-09-14 22:59             ` Florian Fainelli
  0 siblings, 1 reply; 10+ messages in thread
From: Florian Fainelli @ 2017-09-08  4:36 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: Francisco Blas Izquierdo Riera (klondike),
	linux-kernel, ppandit, pebolle, mmarek, Andrew Morton,
	Daniel Thompson



On 09/07/2017 06:19 AM, Nicholas Piggin wrote:
> On Thu, 7 Sep 2017 05:50:30 -0700
> Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
>> On 08/28/2017 08:09 PM, Nicholas Piggin wrote:
>>> On Mon, 28 Aug 2017 13:03:31 -0700
>>> Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>   
>>>> On 05/21/2017 07:46 PM, Nicholas Piggin wrote:  
>>>>> On Sat, 20 May 2017 20:33:35 -0700
>>>>> Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>>>     
>>>>>> Commit db2aa7fd15e8 ("initramfs: allow again choice of the embedded
>>>>>> initram compression algorithm") introduced the possibility to select the
>>>>>> initramfs compression algorithm from Kconfig and while this is a nice
>>>>>> feature it broke the use case described below.
>>>>>>
>>>>>> Here is what my build system does:
>>>>>>
>>>>>> - kernel is initially configured not to have an initramfs included
>>>>>> - build the user space root file system
>>>>>> - re-configure the kernel to have an initramfs included
>>>>>> (CONFIG_INITRAMFS_SOURCE="/path/to/romfs") and set relevant
>>>>>> CONFIG_INITRAMFS options, in my case, no compression option
>>>>>> (CONFIG_INITRAMFS_COMPRESSION_NONE)
>>>>>> - kernel is re-built with these options -> kernel+initramfs image is
>>>>>>   copied
>>>>>> - kernel is re-built again without these options -> kernel image is
>>>>>>   copied
>>>>>>
>>>>>> Building a kernel without an initramfs means setting this option:
>>>>>>
>>>>>> CONFIG_INITRAMFS_SOURCE="" (and this one only)
>>>>>>
>>>>>> whereas building a kernel with an initramfs means setting these options:
>>>>>>
>>>>>> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
>>>>>> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
>>>>>> CONFIG_INITRAMFS_ROOT_UID=1000
>>>>>> CONFIG_INITRAMFS_ROOT_GID=1000
>>>>>> CONFIG_INITRAMFS_COMPRESSION_NONE=y
>>>>>> CONFIG_INITRAMFS_COMPRESSION=""
>>>>>>
>>>>>> Commit db2aa7fd15e857891cefbada8348c8d938c7a2bc ("initramfs: allow again
>>>>>> choice of the embedded initram compression algorithm") is problematic
>>>>>> because CONFIG_INITRAMFS_COMPRESSION which is used to determine the
>>>>>> initramfs_data.cpio extension/compression is a string, and due to how
>>>>>> Kconfig works it will evaluate in order, how to assign it.
>>>>>>
>>>>>> Setting CONFIG_INITRAMFS_COMPRESSION_NONE with
>>>>>> CONFIG_INITRAMFS_SOURCE="" cannot possibly work (because of the depends
>>>>>> on INITRAMFS_SOURCE!="" imposed on CONFIG_INITRAMFS_COMPRESSION ) yet we
>>>>>> still get CONFIG_INITRAMFS_COMPRESSION assigned to ".gz" because
>>>>>> CONFIG_RD_GZIP=y is set in my kernel, even when there is no initramfs
>>>>>> being built.
>>>>>>
>>>>>> So we basically end-up generating two initramfs_data.cpio* files, one
>>>>>> without extension, and one with .gz. This causes usr/Makefile to track
>>>>>> usr/initramfs_data.cpio.gz, and not usr/initramfs_data.cpio anymore,
>>>>>> that is also largely problematic after
>>>>>> 9e3596b0c6539e28546ff7c72a06576627068353 ("kbuild: initramfs cleanup,
>>>>>> set target from Kconfig") because we used to track all possible
>>>>>> initramfs_data files in the $(targets) variable before that commit.
>>>>>>
>>>>>> The end result is that the kernel with an initramfs clearly does not
>>>>>> contain what we expect it to, it has a stale initramfs_data.cpio file
>>>>>> built into it, and we keep re-generating an initramfs_data.cpio.gz file
>>>>>> which is not the one that we want to include in the kernel image proper.
>>>>>>
>>>>>> The fix consists in hiding CONFIG_INITRAMFS_COMPRESSION when
>>>>>> CONFIG_INITRAMFS_SOURCE="". This puts us back in a state to the pre-4.10
>>>>>> behavior where we can properly disable and re-enable initramfs within
>>>>>> the same kernel .config file, and be in control of what
>>>>>> CONFIG_INITRAMFS_COMPRESSION is set to.
>>>>>>
>>>>>> Fixes: db2aa7fd15e8 ("initramfs: allow again choice of the embedded initram compression algorithm")
>>>>>> Fixes: 9e3596b0c653 ("kbuild: initramfs cleanup, set target from Kconfig")
>>>>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>    
>>>>>
>>>>> This is very thorough, thank you for tracking it down and fixing it.
>>>>>
>>>>> I can't say I've worked through the problem in the code, but your
>>>>> changelog and the proposed fix seem reasonable to me. So for what
>>>>> it's worth:
>>>>>
>>>>> Acked-by: Nicholas Piggin <npiggin@gmail.com>    
>>>>
>>>> Well, I am looking at this again, and it's still broken, the same test
>>>> case is involved, except this time, I am switching beween no-initramfs
>>>> and initramfs with gzip compression (the key thing is using a
>>>> compression of some sort). The end result is the following:
>>>>
>>>> - change stuff in the rootfs
>>>> - build the kernel with initramfs, CONFIG_INITRAMFS_COMPRESSION_GZIP=y,
>>>> usr/initramfs_data.cpio.gz gets generated correctly the first time
>>>> - build the kernel without initramfs,
>>>> CONFIG_INITRAMFS_COMPRESSION_NONE=y, usr/initramfs_data.cpio gets generated
>>>>
>>>> Now back to step 1 add some files, and we can see that
>>>> usr/initramfs_data.cpio.gz is now stale from before...
>>>>
>>>> So while my earlier fix switched the initramfs w/o compression to no
>>>> initramfs rebuild, now this does not work because we still have two
>>>> files left to be tracked:
>>>>
>>>> usr/initramfs_data.cpio (no compression, or when initramfs is disabled)
>>>> and usr/initramfs_data.cpio.$(suffix_y)
>>>>
>>>> How would you go about solving this?  
>>>
>>> I don't see the problem. When I change back to .gz, modify the source
>>> directory, and rebuild, it rebuilds a new initramfs_data.cpio.gz here.  
>>
>> This is really puzzling me, so here is what is going on my side:
>>
>> First time build w/ initramfs enabled:
>>
>> grep INITRAMFS .config
>> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
>> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
>> CONFIG_INITRAMFS_ROOT_UID=1000
>> CONFIG_INITRAMFS_ROOT_GID=1000
>> # CONFIG_INITRAMFS_COMPRESSION_NONE is not set
>> CONFIG_INITRAMFS_COMPRESSION_GZIP=y
>> # CONFIG_INITRAMFS_COMPRESSION_BZIP2 is not set
>> # CONFIG_INITRAMFS_COMPRESSION_LZMA is not set
>> # CONFIG_INITRAMFS_COMPRESSION_XZ is not set
>> # CONFIG_INITRAMFS_COMPRESSION_LZO is not set
>> # CONFIG_INITRAMFS_COMPRESSION_LZ4 is not set
>> CONFIG_INITRAMFS_COMPRESSION=".gz"
>>
>> fainelli@fainelli-laptop:[~/../linux]$ ls -la usr
>> total 6.8M
>> drwxrwxr-x  2 fainelli fainelli 4.0K Sep  7 05:37 ./
>> drwxrwxr-x 27 fainelli fainelli  16K Sep  7 05:38 ../
>> -rw-rw-r--  1 fainelli fainelli  146 Sep  7 05:37 built-in.o
>> -rw-rw-r--  1 fainelli fainelli  112 Sep  7 05:37 .built-in.o.cmd
>> -rwxrwxr-x  1 fainelli fainelli  23K Sep  7 05:36 gen_init_cpio*
>> -rw-rw-r--  1 fainelli fainelli  13K Oct 13  2016 gen_init_cpio.c
>> -rw-rw-r--  1 fainelli fainelli 3.3K Sep  7 05:36 .gen_init_cpio.cmd
>> -rw-rw-r--  1 fainelli fainelli  151 Aug 28  2016 .gitignore
>> -rw-rw-r--  1 fainelli fainelli 9.6K Sep  7 05:36 .initramfs_data.cpio.d
>> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.cpio.gz
>> -rw-rw-r--  1 fainelli fainelli  220 Sep  7 05:37
>> .initramfs_data.cpio.gz.cmd
>> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.o
>> -rw-rw-r--  1 fainelli fainelli 2.7K Sep  7 05:37 .initramfs_data.o.cmd
>> -rw-rw-r--  1 fainelli fainelli 1.3K Aug 28  2016 initramfs_data.S
>> -rw-rw-r--  1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
>> -rw-rw-r--  1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
>> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:34 modules.builtin
>> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:35 modules.order
>>
>> Now let's reconfigure this kernel w/o initramfs and rebuild:
>>
>> grep INITRAMFS .config
>> CONFIG_INITRAMFS_SOURCE=""
>>
>> fainelli@fainelli-laptop:[~/../linux]$ ls -la usr
>> total 3.6M
>> drwxrwxr-x  2 fainelli fainelli 4.0K Sep  7 05:44 ./
>> drwxrwxr-x 27 fainelli fainelli  16K Sep  7 05:44 ../
>> -rw-rw-r--  1 fainelli fainelli  146 Sep  7 05:44 built-in.o
>> -rw-rw-r--  1 fainelli fainelli  112 Sep  7 05:44 .built-in.o.cmd
>> -rwxrwxr-x  1 fainelli fainelli  23K Sep  7 05:36 gen_init_cpio*
>> -rw-rw-r--  1 fainelli fainelli  13K Oct 13  2016 gen_init_cpio.c
>> -rw-rw-r--  1 fainelli fainelli 3.3K Sep  7 05:36 .gen_init_cpio.cmd
>> -rw-rw-r--  1 fainelli fainelli  151 Aug 28  2016 .gitignore
>> -rw-rw-r--  1 fainelli fainelli  512 Sep  7 05:44 initramfs_data.cpio
>> -rw-rw-r--  1 fainelli fainelli  105 Sep  7 05:44 .initramfs_data.cpio.cmd
>> -rw-rw-r--  1 fainelli fainelli   52 Sep  7 05:44 .initramfs_data.cpio.d
>> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.cpio.gz
>> -rw-rw-r--  1 fainelli fainelli  220 Sep  7 05:37
>> .initramfs_data.cpio.gz.cmd
>> -rw-rw-r--  1 fainelli fainelli 1.3K Sep  7 05:44 initramfs_data.o
>> -rw-rw-r--  1 fainelli fainelli 2.7K Sep  7 05:44 .initramfs_data.o.cmd
>> -rw-rw-r--  1 fainelli fainelli 1.3K Aug 28  2016 initramfs_data.S
>> -rw-rw-r--  1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
>> -rw-rw-r--  1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
>> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:34 modules.builtin
>> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:35 modules.order
>>
>> Now let's rebuild with a new application added to the root filesystem:
>>
>> grep INITRAMFS .config
>> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
>> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
>> CONFIG_INITRAMFS_ROOT_UID=1000
>> CONFIG_INITRAMFS_ROOT_GID=1000
>> # CONFIG_INITRAMFS_COMPRESSION_NONE is not set
>> CONFIG_INITRAMFS_COMPRESSION_GZIP=y
>> # CONFIG_INITRAMFS_COMPRESSION_BZIP2 is not set
>> # CONFIG_INITRAMFS_COMPRESSION_LZMA is not set
>> # CONFIG_INITRAMFS_COMPRESSION_XZ is not set
>> # CONFIG_INITRAMFS_COMPRESSION_LZO is not set
>> # CONFIG_INITRAMFS_COMPRESSION_LZ4 is not set
>> CONFIG_INITRAMFS_COMPRESSION=".gz"
>>
>> We are not fine here:
>> scripts/kconfig/conf  --silentoldconfig Kconfig
>> #
>> # configuration written to .config
>> #
>>   CHK     include/config/kernel.release
>>   CHK     include/generated/uapi/linux/version.h
>>   CHK     scripts/mod/devicetable-offsets.h
>>   CHK     include/generated/utsrelease.h
>>   CHK     include/generated/timeconst.h
>>   CHK     include/generated/bounds.h
>>   CHK     include/generated/asm-offsets.h
>>   CALL    scripts/checksyscalls.sh
>>   CHK     include/generated/compile.h
>>   AS      usr/initramfs_data.o
>>
>> ^===== we should have called gzip to re-generate the initramfs, we did not
>>
>>   AR      usr/built-in.o
>>   GEN     .version
>>   CHK     include/generated/compile.h
>>   UPD     include/generated/compile.h
>>   CC      init/version.o
>>   AR      init/built-in.o
>>   AR      built-in.o
>>   LD      vmlinux.o
>>   MODPOST vmlinux.o
>>
>> And the timestamps here confirm that:
>>
>> fainelli@fainelli-laptop:[~/../linux]$ ls -la usr
>> total 6.9M
>> drwxrwxr-x  2 fainelli fainelli 4.0K Sep  7 05:46 ./
>> drwxrwxr-x 27 fainelli fainelli  16K Sep  7 05:46 ../
>> -rw-rw-r--  1 fainelli fainelli  146 Sep  7 05:46 built-in.o
>> -rw-rw-r--  1 fainelli fainelli  112 Sep  7 05:46 .built-in.o.cmd
>> -rwxrwxr-x  1 fainelli fainelli  23K Sep  7 05:36 gen_init_cpio*
>> -rw-rw-r--  1 fainelli fainelli  13K Oct 13  2016 gen_init_cpio.c
>> -rw-rw-r--  1 fainelli fainelli 3.3K Sep  7 05:36 .gen_init_cpio.cmd
>> -rw-rw-r--  1 fainelli fainelli  151 Aug 28  2016 .gitignore
>> -rw-rw-r--  1 fainelli fainelli  512 Sep  7 05:44 initramfs_data.cpio
>> -rw-rw-r--  1 fainelli fainelli  105 Sep  7 05:44 .initramfs_data.cpio.cmd
>> -rw-rw-r--  1 fainelli fainelli  11K Sep  7 05:46 .initramfs_data.cpio.d
>> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.cpio.gz
>> -rw-rw-r--  1 fainelli fainelli  220 Sep  7 05:37
>> .initramfs_data.cpio.gz.cmd
>> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:46 initramfs_data.o
>> -rw-rw-r--  1 fainelli fainelli 2.7K Sep  7 05:46 .initramfs_data.o.cmd
>> -rw-rw-r--  1 fainelli fainelli 1.3K Aug 28  2016 initramfs_data.S
>> -rw-rw-r--  1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
>> -rw-rw-r--  1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
>> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:46 modules.builtin
>> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:46 modules.order
>>
>> Worse yet, usr/.initramfs_data.cpio.d does list the binary that was just
>> added, but for some reason, this did not trigger a rebuild of
>> initramfs_data.cpio.gz do you see any reasons why?
> 
> This rule  
> 
> $(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
>         $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/.initramfs_data.cpio.d
>         $(call if_changed,initfs)
> 
> does not have FORCE on the end, so I guess is not picking up the dependency
> properly, perhaps?

I tried that:

diff --git a/usr/Makefile b/usr/Makefile
index 0b87e71c00fc..28a958bebd27 100644
--- a/usr/Makefile
+++ b/usr/Makefile
@@ -51,6 +51,6 @@ $(deps_initramfs): klibcdirs
 # 2) There are changes in which files are included (added or deleted)
 # 3) If gen_init_cpio are newer than initramfs_data.cpio
 # 4) arguments to gen_initramfs.sh changes
-$(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
+$(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
FORCE
        $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/.initramfs_data.cpio.d
        $(call if_changed,initf

but this did not change anything.

We are re-generating .initramfs_data.cpio.d and its timestamp is clearly
updated so it's got to be cmd_initfs which does not get invoked.

.initramfs_data.cpio.gz.cmd is correct:

cmd_usr/initramfs_data.cpio.gz := /bin/bash
./scripts/gen_initramfs_list.sh -o usr/initramfs_data.cpio.gz  -u 1000
-g 1000  /home/fainelli/work/uclinux-rootfs/romfs
/home/fainelli/work/uclinux-rootfs/misc/initramfs.dev

and was generated the first time we did generate the gzip initramfs, so
the command has not changed, nor its arguments, so we just don't call it.

I did try to use if_changed_dep, which rightfully pointed that we don't
have a .initramfs_data.cpio.gz.d, but this gets us very close from the
only thing that I could get working thus far. Another way to possibly
solve it is to void generating a cmd_$($@) and use the same name
irrespective of the compression type used, that should force a change
everytime...

Thanks Nick!

diff --git a/usr/Makefile b/usr/Makefile
index 0b87e71c00fc..2d7d7f4b91f1 100644
--- a/usr/Makefile
+++ b/usr/Makefile
@@ -7,6 +7,7 @@ PHONY += klibcdirs

 suffix_y = $(subst $\",,$(CONFIG_INITRAMFS_COMPRESSION))
 datafile_y = initramfs_data.cpio$(suffix_y)
+datafile_d_y = .$(datafile_y).d
 AFLAGS_initramfs_data.o += -DINITRAMFS_IMAGE="usr/$(datafile_y)"


@@ -29,12 +30,12 @@ ramfs-args  := \
         $(if $(CONFIG_INITRAMFS_ROOT_UID), -u
$(CONFIG_INITRAMFS_ROOT_UID)) \
         $(if $(CONFIG_INITRAMFS_ROOT_GID), -g $(CONFIG_INITRAMFS_ROOT_GID))

-# .initramfs_data.cpio.d is used to identify all files included
+# $(datafile_d_y) is used to identify all files included
 # in initramfs and to detect if any files are added/removed.
 # Removed files are identified by directory timestamp being updated
 # The dependency list is generated by gen_initramfs.sh -l
-ifneq ($(wildcard $(obj)/.initramfs_data.cpio.d),)
-       include $(obj)/.initramfs_data.cpio.d
+ifneq ($(wildcard $(obj)/$(datafile_d_y)),)
+       include $(obj)/$(datafile_d_y)
 endif

 quiet_cmd_initfs = GEN     $@
@@ -52,5 +53,5 @@ $(deps_initramfs): klibcdirs
 # 3) If gen_init_cpio are newer than initramfs_data.cpio
 # 4) arguments to gen_initramfs.sh changes
 $(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
-       $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/.initramfs_data.cpio.d
+       $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/$(datafile_d_y)
        $(call if_changed,initfs)

-- 
Florian

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] initramfs: Fix disabling of initramfs (and its compression)
  2017-09-08  4:36           ` Florian Fainelli
@ 2017-09-14 22:59             ` Florian Fainelli
  2017-09-15  0:41               ` Nicholas Piggin
  0 siblings, 1 reply; 10+ messages in thread
From: Florian Fainelli @ 2017-09-14 22:59 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: Francisco Blas Izquierdo Riera (klondike),
	linux-kernel, ppandit, pebolle, mmarek, Andrew Morton,
	Daniel Thompson



On 09/07/2017 09:36 PM, Florian Fainelli wrote:
> 
> 
> On 09/07/2017 06:19 AM, Nicholas Piggin wrote:
>> On Thu, 7 Sep 2017 05:50:30 -0700
>> Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>>> On 08/28/2017 08:09 PM, Nicholas Piggin wrote:
>>>> On Mon, 28 Aug 2017 13:03:31 -0700
>>>> Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>>   
>>>>> On 05/21/2017 07:46 PM, Nicholas Piggin wrote:  
>>>>>> On Sat, 20 May 2017 20:33:35 -0700
>>>>>> Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>>>>     
>>>>>>> Commit db2aa7fd15e8 ("initramfs: allow again choice of the embedded
>>>>>>> initram compression algorithm") introduced the possibility to select the
>>>>>>> initramfs compression algorithm from Kconfig and while this is a nice
>>>>>>> feature it broke the use case described below.
>>>>>>>
>>>>>>> Here is what my build system does:
>>>>>>>
>>>>>>> - kernel is initially configured not to have an initramfs included
>>>>>>> - build the user space root file system
>>>>>>> - re-configure the kernel to have an initramfs included
>>>>>>> (CONFIG_INITRAMFS_SOURCE="/path/to/romfs") and set relevant
>>>>>>> CONFIG_INITRAMFS options, in my case, no compression option
>>>>>>> (CONFIG_INITRAMFS_COMPRESSION_NONE)
>>>>>>> - kernel is re-built with these options -> kernel+initramfs image is
>>>>>>>   copied
>>>>>>> - kernel is re-built again without these options -> kernel image is
>>>>>>>   copied
>>>>>>>
>>>>>>> Building a kernel without an initramfs means setting this option:
>>>>>>>
>>>>>>> CONFIG_INITRAMFS_SOURCE="" (and this one only)
>>>>>>>
>>>>>>> whereas building a kernel with an initramfs means setting these options:
>>>>>>>
>>>>>>> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
>>>>>>> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
>>>>>>> CONFIG_INITRAMFS_ROOT_UID=1000
>>>>>>> CONFIG_INITRAMFS_ROOT_GID=1000
>>>>>>> CONFIG_INITRAMFS_COMPRESSION_NONE=y
>>>>>>> CONFIG_INITRAMFS_COMPRESSION=""
>>>>>>>
>>>>>>> Commit db2aa7fd15e857891cefbada8348c8d938c7a2bc ("initramfs: allow again
>>>>>>> choice of the embedded initram compression algorithm") is problematic
>>>>>>> because CONFIG_INITRAMFS_COMPRESSION which is used to determine the
>>>>>>> initramfs_data.cpio extension/compression is a string, and due to how
>>>>>>> Kconfig works it will evaluate in order, how to assign it.
>>>>>>>
>>>>>>> Setting CONFIG_INITRAMFS_COMPRESSION_NONE with
>>>>>>> CONFIG_INITRAMFS_SOURCE="" cannot possibly work (because of the depends
>>>>>>> on INITRAMFS_SOURCE!="" imposed on CONFIG_INITRAMFS_COMPRESSION ) yet we
>>>>>>> still get CONFIG_INITRAMFS_COMPRESSION assigned to ".gz" because
>>>>>>> CONFIG_RD_GZIP=y is set in my kernel, even when there is no initramfs
>>>>>>> being built.
>>>>>>>
>>>>>>> So we basically end-up generating two initramfs_data.cpio* files, one
>>>>>>> without extension, and one with .gz. This causes usr/Makefile to track
>>>>>>> usr/initramfs_data.cpio.gz, and not usr/initramfs_data.cpio anymore,
>>>>>>> that is also largely problematic after
>>>>>>> 9e3596b0c6539e28546ff7c72a06576627068353 ("kbuild: initramfs cleanup,
>>>>>>> set target from Kconfig") because we used to track all possible
>>>>>>> initramfs_data files in the $(targets) variable before that commit.
>>>>>>>
>>>>>>> The end result is that the kernel with an initramfs clearly does not
>>>>>>> contain what we expect it to, it has a stale initramfs_data.cpio file
>>>>>>> built into it, and we keep re-generating an initramfs_data.cpio.gz file
>>>>>>> which is not the one that we want to include in the kernel image proper.
>>>>>>>
>>>>>>> The fix consists in hiding CONFIG_INITRAMFS_COMPRESSION when
>>>>>>> CONFIG_INITRAMFS_SOURCE="". This puts us back in a state to the pre-4.10
>>>>>>> behavior where we can properly disable and re-enable initramfs within
>>>>>>> the same kernel .config file, and be in control of what
>>>>>>> CONFIG_INITRAMFS_COMPRESSION is set to.
>>>>>>>
>>>>>>> Fixes: db2aa7fd15e8 ("initramfs: allow again choice of the embedded initram compression algorithm")
>>>>>>> Fixes: 9e3596b0c653 ("kbuild: initramfs cleanup, set target from Kconfig")
>>>>>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>    
>>>>>>
>>>>>> This is very thorough, thank you for tracking it down and fixing it.
>>>>>>
>>>>>> I can't say I've worked through the problem in the code, but your
>>>>>> changelog and the proposed fix seem reasonable to me. So for what
>>>>>> it's worth:
>>>>>>
>>>>>> Acked-by: Nicholas Piggin <npiggin@gmail.com>    
>>>>>
>>>>> Well, I am looking at this again, and it's still broken, the same test
>>>>> case is involved, except this time, I am switching beween no-initramfs
>>>>> and initramfs with gzip compression (the key thing is using a
>>>>> compression of some sort). The end result is the following:
>>>>>
>>>>> - change stuff in the rootfs
>>>>> - build the kernel with initramfs, CONFIG_INITRAMFS_COMPRESSION_GZIP=y,
>>>>> usr/initramfs_data.cpio.gz gets generated correctly the first time
>>>>> - build the kernel without initramfs,
>>>>> CONFIG_INITRAMFS_COMPRESSION_NONE=y, usr/initramfs_data.cpio gets generated
>>>>>
>>>>> Now back to step 1 add some files, and we can see that
>>>>> usr/initramfs_data.cpio.gz is now stale from before...
>>>>>
>>>>> So while my earlier fix switched the initramfs w/o compression to no
>>>>> initramfs rebuild, now this does not work because we still have two
>>>>> files left to be tracked:
>>>>>
>>>>> usr/initramfs_data.cpio (no compression, or when initramfs is disabled)
>>>>> and usr/initramfs_data.cpio.$(suffix_y)
>>>>>
>>>>> How would you go about solving this?  
>>>>
>>>> I don't see the problem. When I change back to .gz, modify the source
>>>> directory, and rebuild, it rebuilds a new initramfs_data.cpio.gz here.  
>>>
>>> This is really puzzling me, so here is what is going on my side:
>>>
>>> First time build w/ initramfs enabled:
>>>
>>> grep INITRAMFS .config
>>> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
>>> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
>>> CONFIG_INITRAMFS_ROOT_UID=1000
>>> CONFIG_INITRAMFS_ROOT_GID=1000
>>> # CONFIG_INITRAMFS_COMPRESSION_NONE is not set
>>> CONFIG_INITRAMFS_COMPRESSION_GZIP=y
>>> # CONFIG_INITRAMFS_COMPRESSION_BZIP2 is not set
>>> # CONFIG_INITRAMFS_COMPRESSION_LZMA is not set
>>> # CONFIG_INITRAMFS_COMPRESSION_XZ is not set
>>> # CONFIG_INITRAMFS_COMPRESSION_LZO is not set
>>> # CONFIG_INITRAMFS_COMPRESSION_LZ4 is not set
>>> CONFIG_INITRAMFS_COMPRESSION=".gz"
>>>
>>> fainelli@fainelli-laptop:[~/../linux]$ ls -la usr
>>> total 6.8M
>>> drwxrwxr-x  2 fainelli fainelli 4.0K Sep  7 05:37 ./
>>> drwxrwxr-x 27 fainelli fainelli  16K Sep  7 05:38 ../
>>> -rw-rw-r--  1 fainelli fainelli  146 Sep  7 05:37 built-in.o
>>> -rw-rw-r--  1 fainelli fainelli  112 Sep  7 05:37 .built-in.o.cmd
>>> -rwxrwxr-x  1 fainelli fainelli  23K Sep  7 05:36 gen_init_cpio*
>>> -rw-rw-r--  1 fainelli fainelli  13K Oct 13  2016 gen_init_cpio.c
>>> -rw-rw-r--  1 fainelli fainelli 3.3K Sep  7 05:36 .gen_init_cpio.cmd
>>> -rw-rw-r--  1 fainelli fainelli  151 Aug 28  2016 .gitignore
>>> -rw-rw-r--  1 fainelli fainelli 9.6K Sep  7 05:36 .initramfs_data.cpio.d
>>> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.cpio.gz
>>> -rw-rw-r--  1 fainelli fainelli  220 Sep  7 05:37
>>> .initramfs_data.cpio.gz.cmd
>>> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.o
>>> -rw-rw-r--  1 fainelli fainelli 2.7K Sep  7 05:37 .initramfs_data.o.cmd
>>> -rw-rw-r--  1 fainelli fainelli 1.3K Aug 28  2016 initramfs_data.S
>>> -rw-rw-r--  1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
>>> -rw-rw-r--  1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
>>> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:34 modules.builtin
>>> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:35 modules.order
>>>
>>> Now let's reconfigure this kernel w/o initramfs and rebuild:
>>>
>>> grep INITRAMFS .config
>>> CONFIG_INITRAMFS_SOURCE=""
>>>
>>> fainelli@fainelli-laptop:[~/../linux]$ ls -la usr
>>> total 3.6M
>>> drwxrwxr-x  2 fainelli fainelli 4.0K Sep  7 05:44 ./
>>> drwxrwxr-x 27 fainelli fainelli  16K Sep  7 05:44 ../
>>> -rw-rw-r--  1 fainelli fainelli  146 Sep  7 05:44 built-in.o
>>> -rw-rw-r--  1 fainelli fainelli  112 Sep  7 05:44 .built-in.o.cmd
>>> -rwxrwxr-x  1 fainelli fainelli  23K Sep  7 05:36 gen_init_cpio*
>>> -rw-rw-r--  1 fainelli fainelli  13K Oct 13  2016 gen_init_cpio.c
>>> -rw-rw-r--  1 fainelli fainelli 3.3K Sep  7 05:36 .gen_init_cpio.cmd
>>> -rw-rw-r--  1 fainelli fainelli  151 Aug 28  2016 .gitignore
>>> -rw-rw-r--  1 fainelli fainelli  512 Sep  7 05:44 initramfs_data.cpio
>>> -rw-rw-r--  1 fainelli fainelli  105 Sep  7 05:44 .initramfs_data.cpio.cmd
>>> -rw-rw-r--  1 fainelli fainelli   52 Sep  7 05:44 .initramfs_data.cpio.d
>>> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.cpio.gz
>>> -rw-rw-r--  1 fainelli fainelli  220 Sep  7 05:37
>>> .initramfs_data.cpio.gz.cmd
>>> -rw-rw-r--  1 fainelli fainelli 1.3K Sep  7 05:44 initramfs_data.o
>>> -rw-rw-r--  1 fainelli fainelli 2.7K Sep  7 05:44 .initramfs_data.o.cmd
>>> -rw-rw-r--  1 fainelli fainelli 1.3K Aug 28  2016 initramfs_data.S
>>> -rw-rw-r--  1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
>>> -rw-rw-r--  1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
>>> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:34 modules.builtin
>>> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:35 modules.order
>>>
>>> Now let's rebuild with a new application added to the root filesystem:
>>>
>>> grep INITRAMFS .config
>>> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
>>> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
>>> CONFIG_INITRAMFS_ROOT_UID=1000
>>> CONFIG_INITRAMFS_ROOT_GID=1000
>>> # CONFIG_INITRAMFS_COMPRESSION_NONE is not set
>>> CONFIG_INITRAMFS_COMPRESSION_GZIP=y
>>> # CONFIG_INITRAMFS_COMPRESSION_BZIP2 is not set
>>> # CONFIG_INITRAMFS_COMPRESSION_LZMA is not set
>>> # CONFIG_INITRAMFS_COMPRESSION_XZ is not set
>>> # CONFIG_INITRAMFS_COMPRESSION_LZO is not set
>>> # CONFIG_INITRAMFS_COMPRESSION_LZ4 is not set
>>> CONFIG_INITRAMFS_COMPRESSION=".gz"
>>>
>>> We are not fine here:
>>> scripts/kconfig/conf  --silentoldconfig Kconfig
>>> #
>>> # configuration written to .config
>>> #
>>>   CHK     include/config/kernel.release
>>>   CHK     include/generated/uapi/linux/version.h
>>>   CHK     scripts/mod/devicetable-offsets.h
>>>   CHK     include/generated/utsrelease.h
>>>   CHK     include/generated/timeconst.h
>>>   CHK     include/generated/bounds.h
>>>   CHK     include/generated/asm-offsets.h
>>>   CALL    scripts/checksyscalls.sh
>>>   CHK     include/generated/compile.h
>>>   AS      usr/initramfs_data.o
>>>
>>> ^===== we should have called gzip to re-generate the initramfs, we did not
>>>
>>>   AR      usr/built-in.o
>>>   GEN     .version
>>>   CHK     include/generated/compile.h
>>>   UPD     include/generated/compile.h
>>>   CC      init/version.o
>>>   AR      init/built-in.o
>>>   AR      built-in.o
>>>   LD      vmlinux.o
>>>   MODPOST vmlinux.o
>>>
>>> And the timestamps here confirm that:
>>>
>>> fainelli@fainelli-laptop:[~/../linux]$ ls -la usr
>>> total 6.9M
>>> drwxrwxr-x  2 fainelli fainelli 4.0K Sep  7 05:46 ./
>>> drwxrwxr-x 27 fainelli fainelli  16K Sep  7 05:46 ../
>>> -rw-rw-r--  1 fainelli fainelli  146 Sep  7 05:46 built-in.o
>>> -rw-rw-r--  1 fainelli fainelli  112 Sep  7 05:46 .built-in.o.cmd
>>> -rwxrwxr-x  1 fainelli fainelli  23K Sep  7 05:36 gen_init_cpio*
>>> -rw-rw-r--  1 fainelli fainelli  13K Oct 13  2016 gen_init_cpio.c
>>> -rw-rw-r--  1 fainelli fainelli 3.3K Sep  7 05:36 .gen_init_cpio.cmd
>>> -rw-rw-r--  1 fainelli fainelli  151 Aug 28  2016 .gitignore
>>> -rw-rw-r--  1 fainelli fainelli  512 Sep  7 05:44 initramfs_data.cpio
>>> -rw-rw-r--  1 fainelli fainelli  105 Sep  7 05:44 .initramfs_data.cpio.cmd
>>> -rw-rw-r--  1 fainelli fainelli  11K Sep  7 05:46 .initramfs_data.cpio.d
>>> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.cpio.gz
>>> -rw-rw-r--  1 fainelli fainelli  220 Sep  7 05:37
>>> .initramfs_data.cpio.gz.cmd
>>> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:46 initramfs_data.o
>>> -rw-rw-r--  1 fainelli fainelli 2.7K Sep  7 05:46 .initramfs_data.o.cmd
>>> -rw-rw-r--  1 fainelli fainelli 1.3K Aug 28  2016 initramfs_data.S
>>> -rw-rw-r--  1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
>>> -rw-rw-r--  1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
>>> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:46 modules.builtin
>>> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:46 modules.order
>>>
>>> Worse yet, usr/.initramfs_data.cpio.d does list the binary that was just
>>> added, but for some reason, this did not trigger a rebuild of
>>> initramfs_data.cpio.gz do you see any reasons why?
>>
>> This rule  
>>
>> $(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
>>         $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/.initramfs_data.cpio.d
>>         $(call if_changed,initfs)
>>
>> does not have FORCE on the end, so I guess is not picking up the dependency
>> properly, perhaps?
> 
> I tried that:
> 
> diff --git a/usr/Makefile b/usr/Makefile
> index 0b87e71c00fc..28a958bebd27 100644
> --- a/usr/Makefile
> +++ b/usr/Makefile
> @@ -51,6 +51,6 @@ $(deps_initramfs): klibcdirs
>  # 2) There are changes in which files are included (added or deleted)
>  # 3) If gen_init_cpio are newer than initramfs_data.cpio
>  # 4) arguments to gen_initramfs.sh changes
> -$(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
> +$(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
> FORCE
>         $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/.initramfs_data.cpio.d
>         $(call if_changed,initf
> 
> but this did not change anything.
> 
> We are re-generating .initramfs_data.cpio.d and its timestamp is clearly
> updated so it's got to be cmd_initfs which does not get invoked.
> 
> .initramfs_data.cpio.gz.cmd is correct:
> 
> cmd_usr/initramfs_data.cpio.gz := /bin/bash
> ./scripts/gen_initramfs_list.sh -o usr/initramfs_data.cpio.gz  -u 1000
> -g 1000  /home/fainelli/work/uclinux-rootfs/romfs
> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev
> 
> and was generated the first time we did generate the gzip initramfs, so
> the command has not changed, nor its arguments, so we just don't call it.
> 
> I did try to use if_changed_dep, which rightfully pointed that we don't
> have a .initramfs_data.cpio.gz.d, but this gets us very close from the
> only thing that I could get working thus far. Another way to possibly
> solve it is to void generating a cmd_$($@) and use the same name
> irrespective of the compression type used, that should force a change
> everytime...
> 
> Thanks Nick!

Nick, is the patch below acceptable to you?

> 
> diff --git a/usr/Makefile b/usr/Makefile
> index 0b87e71c00fc..2d7d7f4b91f1 100644
> --- a/usr/Makefile
> +++ b/usr/Makefile
> @@ -7,6 +7,7 @@ PHONY += klibcdirs
> 
>  suffix_y = $(subst $\",,$(CONFIG_INITRAMFS_COMPRESSION))
>  datafile_y = initramfs_data.cpio$(suffix_y)
> +datafile_d_y = .$(datafile_y).d
>  AFLAGS_initramfs_data.o += -DINITRAMFS_IMAGE="usr/$(datafile_y)"
> 
> 
> @@ -29,12 +30,12 @@ ramfs-args  := \
>          $(if $(CONFIG_INITRAMFS_ROOT_UID), -u
> $(CONFIG_INITRAMFS_ROOT_UID)) \
>          $(if $(CONFIG_INITRAMFS_ROOT_GID), -g $(CONFIG_INITRAMFS_ROOT_GID))
> 
> -# .initramfs_data.cpio.d is used to identify all files included
> +# $(datafile_d_y) is used to identify all files included
>  # in initramfs and to detect if any files are added/removed.
>  # Removed files are identified by directory timestamp being updated
>  # The dependency list is generated by gen_initramfs.sh -l
> -ifneq ($(wildcard $(obj)/.initramfs_data.cpio.d),)
> -       include $(obj)/.initramfs_data.cpio.d
> +ifneq ($(wildcard $(obj)/$(datafile_d_y)),)
> +       include $(obj)/$(datafile_d_y)
>  endif
> 
>  quiet_cmd_initfs = GEN     $@
> @@ -52,5 +53,5 @@ $(deps_initramfs): klibcdirs
>  # 3) If gen_init_cpio are newer than initramfs_data.cpio
>  # 4) arguments to gen_initramfs.sh changes
>  $(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
> -       $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/.initramfs_data.cpio.d
> +       $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/$(datafile_d_y)
>         $(call if_changed,initfs)
> 

-- 
Florian

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] initramfs: Fix disabling of initramfs (and its compression)
  2017-09-14 22:59             ` Florian Fainelli
@ 2017-09-15  0:41               ` Nicholas Piggin
  0 siblings, 0 replies; 10+ messages in thread
From: Nicholas Piggin @ 2017-09-15  0:41 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Francisco Blas Izquierdo Riera (klondike),
	linux-kernel, ppandit, pebolle, mmarek, Andrew Morton,
	Daniel Thompson

On Thu, 14 Sep 2017 15:59:50 -0700
Florian Fainelli <f.fainelli@gmail.com> wrote:

> On 09/07/2017 09:36 PM, Florian Fainelli wrote:
> > 
> > 
> > On 09/07/2017 06:19 AM, Nicholas Piggin wrote:  
> >> On Thu, 7 Sep 2017 05:50:30 -0700
> >> Florian Fainelli <f.fainelli@gmail.com> wrote:
> >>  
> >>> On 08/28/2017 08:09 PM, Nicholas Piggin wrote:  
> >>>> On Mon, 28 Aug 2017 13:03:31 -0700
> >>>> Florian Fainelli <f.fainelli@gmail.com> wrote:
> >>>>     
> >>>>> On 05/21/2017 07:46 PM, Nicholas Piggin wrote:    
> >>>>>> On Sat, 20 May 2017 20:33:35 -0700
> >>>>>> Florian Fainelli <f.fainelli@gmail.com> wrote:
> >>>>>>       
> >>>>>>> Commit db2aa7fd15e8 ("initramfs: allow again choice of the embedded
> >>>>>>> initram compression algorithm") introduced the possibility to select the
> >>>>>>> initramfs compression algorithm from Kconfig and while this is a nice
> >>>>>>> feature it broke the use case described below.
> >>>>>>>
> >>>>>>> Here is what my build system does:
> >>>>>>>
> >>>>>>> - kernel is initially configured not to have an initramfs included
> >>>>>>> - build the user space root file system
> >>>>>>> - re-configure the kernel to have an initramfs included
> >>>>>>> (CONFIG_INITRAMFS_SOURCE="/path/to/romfs") and set relevant
> >>>>>>> CONFIG_INITRAMFS options, in my case, no compression option
> >>>>>>> (CONFIG_INITRAMFS_COMPRESSION_NONE)
> >>>>>>> - kernel is re-built with these options -> kernel+initramfs image is
> >>>>>>>   copied
> >>>>>>> - kernel is re-built again without these options -> kernel image is
> >>>>>>>   copied
> >>>>>>>
> >>>>>>> Building a kernel without an initramfs means setting this option:
> >>>>>>>
> >>>>>>> CONFIG_INITRAMFS_SOURCE="" (and this one only)
> >>>>>>>
> >>>>>>> whereas building a kernel with an initramfs means setting these options:
> >>>>>>>
> >>>>>>> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
> >>>>>>> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
> >>>>>>> CONFIG_INITRAMFS_ROOT_UID=1000
> >>>>>>> CONFIG_INITRAMFS_ROOT_GID=1000
> >>>>>>> CONFIG_INITRAMFS_COMPRESSION_NONE=y
> >>>>>>> CONFIG_INITRAMFS_COMPRESSION=""
> >>>>>>>
> >>>>>>> Commit db2aa7fd15e857891cefbada8348c8d938c7a2bc ("initramfs: allow again
> >>>>>>> choice of the embedded initram compression algorithm") is problematic
> >>>>>>> because CONFIG_INITRAMFS_COMPRESSION which is used to determine the
> >>>>>>> initramfs_data.cpio extension/compression is a string, and due to how
> >>>>>>> Kconfig works it will evaluate in order, how to assign it.
> >>>>>>>
> >>>>>>> Setting CONFIG_INITRAMFS_COMPRESSION_NONE with
> >>>>>>> CONFIG_INITRAMFS_SOURCE="" cannot possibly work (because of the depends
> >>>>>>> on INITRAMFS_SOURCE!="" imposed on CONFIG_INITRAMFS_COMPRESSION ) yet we
> >>>>>>> still get CONFIG_INITRAMFS_COMPRESSION assigned to ".gz" because
> >>>>>>> CONFIG_RD_GZIP=y is set in my kernel, even when there is no initramfs
> >>>>>>> being built.
> >>>>>>>
> >>>>>>> So we basically end-up generating two initramfs_data.cpio* files, one
> >>>>>>> without extension, and one with .gz. This causes usr/Makefile to track
> >>>>>>> usr/initramfs_data.cpio.gz, and not usr/initramfs_data.cpio anymore,
> >>>>>>> that is also largely problematic after
> >>>>>>> 9e3596b0c6539e28546ff7c72a06576627068353 ("kbuild: initramfs cleanup,
> >>>>>>> set target from Kconfig") because we used to track all possible
> >>>>>>> initramfs_data files in the $(targets) variable before that commit.
> >>>>>>>
> >>>>>>> The end result is that the kernel with an initramfs clearly does not
> >>>>>>> contain what we expect it to, it has a stale initramfs_data.cpio file
> >>>>>>> built into it, and we keep re-generating an initramfs_data.cpio.gz file
> >>>>>>> which is not the one that we want to include in the kernel image proper.
> >>>>>>>
> >>>>>>> The fix consists in hiding CONFIG_INITRAMFS_COMPRESSION when
> >>>>>>> CONFIG_INITRAMFS_SOURCE="". This puts us back in a state to the pre-4.10
> >>>>>>> behavior where we can properly disable and re-enable initramfs within
> >>>>>>> the same kernel .config file, and be in control of what
> >>>>>>> CONFIG_INITRAMFS_COMPRESSION is set to.
> >>>>>>>
> >>>>>>> Fixes: db2aa7fd15e8 ("initramfs: allow again choice of the embedded initram compression algorithm")
> >>>>>>> Fixes: 9e3596b0c653 ("kbuild: initramfs cleanup, set target from Kconfig")
> >>>>>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>      
> >>>>>>
> >>>>>> This is very thorough, thank you for tracking it down and fixing it.
> >>>>>>
> >>>>>> I can't say I've worked through the problem in the code, but your
> >>>>>> changelog and the proposed fix seem reasonable to me. So for what
> >>>>>> it's worth:
> >>>>>>
> >>>>>> Acked-by: Nicholas Piggin <npiggin@gmail.com>      
> >>>>>
> >>>>> Well, I am looking at this again, and it's still broken, the same test
> >>>>> case is involved, except this time, I am switching beween no-initramfs
> >>>>> and initramfs with gzip compression (the key thing is using a
> >>>>> compression of some sort). The end result is the following:
> >>>>>
> >>>>> - change stuff in the rootfs
> >>>>> - build the kernel with initramfs, CONFIG_INITRAMFS_COMPRESSION_GZIP=y,
> >>>>> usr/initramfs_data.cpio.gz gets generated correctly the first time
> >>>>> - build the kernel without initramfs,
> >>>>> CONFIG_INITRAMFS_COMPRESSION_NONE=y, usr/initramfs_data.cpio gets generated
> >>>>>
> >>>>> Now back to step 1 add some files, and we can see that
> >>>>> usr/initramfs_data.cpio.gz is now stale from before...
> >>>>>
> >>>>> So while my earlier fix switched the initramfs w/o compression to no
> >>>>> initramfs rebuild, now this does not work because we still have two
> >>>>> files left to be tracked:
> >>>>>
> >>>>> usr/initramfs_data.cpio (no compression, or when initramfs is disabled)
> >>>>> and usr/initramfs_data.cpio.$(suffix_y)
> >>>>>
> >>>>> How would you go about solving this?    
> >>>>
> >>>> I don't see the problem. When I change back to .gz, modify the source
> >>>> directory, and rebuild, it rebuilds a new initramfs_data.cpio.gz here.    
> >>>
> >>> This is really puzzling me, so here is what is going on my side:
> >>>
> >>> First time build w/ initramfs enabled:
> >>>
> >>> grep INITRAMFS .config
> >>> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
> >>> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
> >>> CONFIG_INITRAMFS_ROOT_UID=1000
> >>> CONFIG_INITRAMFS_ROOT_GID=1000
> >>> # CONFIG_INITRAMFS_COMPRESSION_NONE is not set
> >>> CONFIG_INITRAMFS_COMPRESSION_GZIP=y
> >>> # CONFIG_INITRAMFS_COMPRESSION_BZIP2 is not set
> >>> # CONFIG_INITRAMFS_COMPRESSION_LZMA is not set
> >>> # CONFIG_INITRAMFS_COMPRESSION_XZ is not set
> >>> # CONFIG_INITRAMFS_COMPRESSION_LZO is not set
> >>> # CONFIG_INITRAMFS_COMPRESSION_LZ4 is not set
> >>> CONFIG_INITRAMFS_COMPRESSION=".gz"
> >>>
> >>> fainelli@fainelli-laptop:[~/../linux]$ ls -la usr
> >>> total 6.8M
> >>> drwxrwxr-x  2 fainelli fainelli 4.0K Sep  7 05:37 ./
> >>> drwxrwxr-x 27 fainelli fainelli  16K Sep  7 05:38 ../
> >>> -rw-rw-r--  1 fainelli fainelli  146 Sep  7 05:37 built-in.o
> >>> -rw-rw-r--  1 fainelli fainelli  112 Sep  7 05:37 .built-in.o.cmd
> >>> -rwxrwxr-x  1 fainelli fainelli  23K Sep  7 05:36 gen_init_cpio*
> >>> -rw-rw-r--  1 fainelli fainelli  13K Oct 13  2016 gen_init_cpio.c
> >>> -rw-rw-r--  1 fainelli fainelli 3.3K Sep  7 05:36 .gen_init_cpio.cmd
> >>> -rw-rw-r--  1 fainelli fainelli  151 Aug 28  2016 .gitignore
> >>> -rw-rw-r--  1 fainelli fainelli 9.6K Sep  7 05:36 .initramfs_data.cpio.d
> >>> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.cpio.gz
> >>> -rw-rw-r--  1 fainelli fainelli  220 Sep  7 05:37
> >>> .initramfs_data.cpio.gz.cmd
> >>> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.o
> >>> -rw-rw-r--  1 fainelli fainelli 2.7K Sep  7 05:37 .initramfs_data.o.cmd
> >>> -rw-rw-r--  1 fainelli fainelli 1.3K Aug 28  2016 initramfs_data.S
> >>> -rw-rw-r--  1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
> >>> -rw-rw-r--  1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
> >>> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:34 modules.builtin
> >>> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:35 modules.order
> >>>
> >>> Now let's reconfigure this kernel w/o initramfs and rebuild:
> >>>
> >>> grep INITRAMFS .config
> >>> CONFIG_INITRAMFS_SOURCE=""
> >>>
> >>> fainelli@fainelli-laptop:[~/../linux]$ ls -la usr
> >>> total 3.6M
> >>> drwxrwxr-x  2 fainelli fainelli 4.0K Sep  7 05:44 ./
> >>> drwxrwxr-x 27 fainelli fainelli  16K Sep  7 05:44 ../
> >>> -rw-rw-r--  1 fainelli fainelli  146 Sep  7 05:44 built-in.o
> >>> -rw-rw-r--  1 fainelli fainelli  112 Sep  7 05:44 .built-in.o.cmd
> >>> -rwxrwxr-x  1 fainelli fainelli  23K Sep  7 05:36 gen_init_cpio*
> >>> -rw-rw-r--  1 fainelli fainelli  13K Oct 13  2016 gen_init_cpio.c
> >>> -rw-rw-r--  1 fainelli fainelli 3.3K Sep  7 05:36 .gen_init_cpio.cmd
> >>> -rw-rw-r--  1 fainelli fainelli  151 Aug 28  2016 .gitignore
> >>> -rw-rw-r--  1 fainelli fainelli  512 Sep  7 05:44 initramfs_data.cpio
> >>> -rw-rw-r--  1 fainelli fainelli  105 Sep  7 05:44 .initramfs_data.cpio.cmd
> >>> -rw-rw-r--  1 fainelli fainelli   52 Sep  7 05:44 .initramfs_data.cpio.d
> >>> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.cpio.gz
> >>> -rw-rw-r--  1 fainelli fainelli  220 Sep  7 05:37
> >>> .initramfs_data.cpio.gz.cmd
> >>> -rw-rw-r--  1 fainelli fainelli 1.3K Sep  7 05:44 initramfs_data.o
> >>> -rw-rw-r--  1 fainelli fainelli 2.7K Sep  7 05:44 .initramfs_data.o.cmd
> >>> -rw-rw-r--  1 fainelli fainelli 1.3K Aug 28  2016 initramfs_data.S
> >>> -rw-rw-r--  1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
> >>> -rw-rw-r--  1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
> >>> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:34 modules.builtin
> >>> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:35 modules.order
> >>>
> >>> Now let's rebuild with a new application added to the root filesystem:
> >>>
> >>> grep INITRAMFS .config
> >>> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
> >>> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
> >>> CONFIG_INITRAMFS_ROOT_UID=1000
> >>> CONFIG_INITRAMFS_ROOT_GID=1000
> >>> # CONFIG_INITRAMFS_COMPRESSION_NONE is not set
> >>> CONFIG_INITRAMFS_COMPRESSION_GZIP=y
> >>> # CONFIG_INITRAMFS_COMPRESSION_BZIP2 is not set
> >>> # CONFIG_INITRAMFS_COMPRESSION_LZMA is not set
> >>> # CONFIG_INITRAMFS_COMPRESSION_XZ is not set
> >>> # CONFIG_INITRAMFS_COMPRESSION_LZO is not set
> >>> # CONFIG_INITRAMFS_COMPRESSION_LZ4 is not set
> >>> CONFIG_INITRAMFS_COMPRESSION=".gz"
> >>>
> >>> We are not fine here:
> >>> scripts/kconfig/conf  --silentoldconfig Kconfig
> >>> #
> >>> # configuration written to .config
> >>> #
> >>>   CHK     include/config/kernel.release
> >>>   CHK     include/generated/uapi/linux/version.h
> >>>   CHK     scripts/mod/devicetable-offsets.h
> >>>   CHK     include/generated/utsrelease.h
> >>>   CHK     include/generated/timeconst.h
> >>>   CHK     include/generated/bounds.h
> >>>   CHK     include/generated/asm-offsets.h
> >>>   CALL    scripts/checksyscalls.sh
> >>>   CHK     include/generated/compile.h
> >>>   AS      usr/initramfs_data.o
> >>>
> >>> ^===== we should have called gzip to re-generate the initramfs, we did not
> >>>
> >>>   AR      usr/built-in.o
> >>>   GEN     .version
> >>>   CHK     include/generated/compile.h
> >>>   UPD     include/generated/compile.h
> >>>   CC      init/version.o
> >>>   AR      init/built-in.o
> >>>   AR      built-in.o
> >>>   LD      vmlinux.o
> >>>   MODPOST vmlinux.o
> >>>
> >>> And the timestamps here confirm that:
> >>>
> >>> fainelli@fainelli-laptop:[~/../linux]$ ls -la usr
> >>> total 6.9M
> >>> drwxrwxr-x  2 fainelli fainelli 4.0K Sep  7 05:46 ./
> >>> drwxrwxr-x 27 fainelli fainelli  16K Sep  7 05:46 ../
> >>> -rw-rw-r--  1 fainelli fainelli  146 Sep  7 05:46 built-in.o
> >>> -rw-rw-r--  1 fainelli fainelli  112 Sep  7 05:46 .built-in.o.cmd
> >>> -rwxrwxr-x  1 fainelli fainelli  23K Sep  7 05:36 gen_init_cpio*
> >>> -rw-rw-r--  1 fainelli fainelli  13K Oct 13  2016 gen_init_cpio.c
> >>> -rw-rw-r--  1 fainelli fainelli 3.3K Sep  7 05:36 .gen_init_cpio.cmd
> >>> -rw-rw-r--  1 fainelli fainelli  151 Aug 28  2016 .gitignore
> >>> -rw-rw-r--  1 fainelli fainelli  512 Sep  7 05:44 initramfs_data.cpio
> >>> -rw-rw-r--  1 fainelli fainelli  105 Sep  7 05:44 .initramfs_data.cpio.cmd
> >>> -rw-rw-r--  1 fainelli fainelli  11K Sep  7 05:46 .initramfs_data.cpio.d
> >>> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.cpio.gz
> >>> -rw-rw-r--  1 fainelli fainelli  220 Sep  7 05:37
> >>> .initramfs_data.cpio.gz.cmd
> >>> -rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:46 initramfs_data.o
> >>> -rw-rw-r--  1 fainelli fainelli 2.7K Sep  7 05:46 .initramfs_data.o.cmd
> >>> -rw-rw-r--  1 fainelli fainelli 1.3K Aug 28  2016 initramfs_data.S
> >>> -rw-rw-r--  1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
> >>> -rw-rw-r--  1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
> >>> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:46 modules.builtin
> >>> -rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:46 modules.order
> >>>
> >>> Worse yet, usr/.initramfs_data.cpio.d does list the binary that was just
> >>> added, but for some reason, this did not trigger a rebuild of
> >>> initramfs_data.cpio.gz do you see any reasons why?  
> >>
> >> This rule  
> >>
> >> $(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
> >>         $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/.initramfs_data.cpio.d
> >>         $(call if_changed,initfs)
> >>
> >> does not have FORCE on the end, so I guess is not picking up the dependency
> >> properly, perhaps?  
> > 
> > I tried that:
> > 
> > diff --git a/usr/Makefile b/usr/Makefile
> > index 0b87e71c00fc..28a958bebd27 100644
> > --- a/usr/Makefile
> > +++ b/usr/Makefile
> > @@ -51,6 +51,6 @@ $(deps_initramfs): klibcdirs
> >  # 2) There are changes in which files are included (added or deleted)
> >  # 3) If gen_init_cpio are newer than initramfs_data.cpio
> >  # 4) arguments to gen_initramfs.sh changes
> > -$(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
> > +$(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
> > FORCE
> >         $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/.initramfs_data.cpio.d
> >         $(call if_changed,initf
> > 
> > but this did not change anything.
> > 
> > We are re-generating .initramfs_data.cpio.d and its timestamp is clearly
> > updated so it's got to be cmd_initfs which does not get invoked.
> > 
> > .initramfs_data.cpio.gz.cmd is correct:
> > 
> > cmd_usr/initramfs_data.cpio.gz := /bin/bash
> > ./scripts/gen_initramfs_list.sh -o usr/initramfs_data.cpio.gz  -u 1000
> > -g 1000  /home/fainelli/work/uclinux-rootfs/romfs
> > /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev
> > 
> > and was generated the first time we did generate the gzip initramfs, so
> > the command has not changed, nor its arguments, so we just don't call it.
> > 
> > I did try to use if_changed_dep, which rightfully pointed that we don't
> > have a .initramfs_data.cpio.gz.d, but this gets us very close from the
> > only thing that I could get working thus far. Another way to possibly
> > solve it is to void generating a cmd_$($@) and use the same name
> > irrespective of the compression type used, that should force a change
> > everytime...
> > 
> > Thanks Nick!  
> 
> Nick, is the patch below acceptable to you?

Well it looks reasonable, and it fixes your problem. I'm not maintainer
of this code or a Makefile wizard though :) Perhaps send it up to the
linux-kbuild maintainers?

Thanks,
Nick

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-09-15  0:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-21  3:33 [PATCH] initramfs: Fix disabling of initramfs (and its compression) Florian Fainelli
2017-05-22  2:46 ` Nicholas Piggin
2017-08-28 20:03   ` Florian Fainelli
2017-08-29  3:09     ` Nicholas Piggin
2017-08-29  3:52       ` Florian Fainelli
2017-09-07 12:50       ` Florian Fainelli
2017-09-07 13:19         ` Nicholas Piggin
2017-09-08  4:36           ` Florian Fainelli
2017-09-14 22:59             ` Florian Fainelli
2017-09-15  0:41               ` Nicholas Piggin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.