diff mbox series

[1/2] configs/stm32mp157c_dk2: disable mkfs metadata_csum and dir_index options

Message ID 20191103115017.788940-1-b.bilas@grinn-global.com
State Accepted
Headers show
Series [1/2] configs/stm32mp157c_dk2: disable mkfs metadata_csum and dir_index options | expand

Commit Message

Bartosz Bilas Nov. 3, 2019, 11:50 a.m. UTC
To solve issue with non-possibilities to mount rootfs partition we
should disable a new mkfs features such as metadata_csum and
dir_index because there is incompatibility with these options.

Signed-off-by: Bartosz Bilas <b.bilas@grinn-global.com>
---
 configs/stm32mp157c_dk2_defconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Peter Korsgaard Nov. 3, 2019, 6:52 p.m. UTC | #1
>>>>> "Bartosz" == Bartosz Bilas <b.bilas@grinn-global.com> writes:

 > To solve issue with non-possibilities to mount rootfs partition we
 > should disable a new mkfs features such as metadata_csum and
 > dir_index because there is incompatibility with these options.

 > Signed-off-by: Bartosz Bilas <b.bilas@grinn-global.com>
 > ---
 >  configs/stm32mp157c_dk2_defconfig | 1 +
 >  1 file changed, 1 insertion(+)

 > diff --git a/configs/stm32mp157c_dk2_defconfig b/configs/stm32mp157c_dk2_defconfig
 > index a1e61d752d..ba044e5e55 100644
 > --- a/configs/stm32mp157c_dk2_defconfig
 > +++ b/configs/stm32mp157c_dk2_defconfig
 > @@ -16,6 +16,7 @@ BR2_LINUX_KERNEL_INSTALL_TARGET=y
 >  BR2_TARGET_ROOTFS_EXT2=y
 >  BR2_TARGET_ROOTFS_EXT2_4=y
 >  BR2_TARGET_ROOTFS_EXT2_SIZE="120M"
 > +BR2_TARGET_ROOTFS_EXT2_MKFS_OPTIONS="-O ^64bit,^metadata_csum,^dir_index"

Is this a general issue with the ext4 support in u-boot? Should we do
this generally by default, similar to how we do it for 64bit?
Bartosz Bilas Nov. 4, 2019, 5 p.m. UTC | #2
Hello Peter,

On 03.11.2019 19:52, Peter Korsgaard wrote:
>>>>>> "Bartosz" == Bartosz Bilas <b.bilas@grinn-global.com> writes:
>   > To solve issue with non-possibilities to mount rootfs partition we
>   > should disable a new mkfs features such as metadata_csum and
>   > dir_index because there is incompatibility with these options.
>
>   > Signed-off-by: Bartosz Bilas <b.bilas@grinn-global.com>
>   > ---
>   >  configs/stm32mp157c_dk2_defconfig | 1 +
>   >  1 file changed, 1 insertion(+)
>
>   > diff --git a/configs/stm32mp157c_dk2_defconfig b/configs/stm32mp157c_dk2_defconfig
>   > index a1e61d752d..ba044e5e55 100644
>   > --- a/configs/stm32mp157c_dk2_defconfig
>   > +++ b/configs/stm32mp157c_dk2_defconfig
>   > @@ -16,6 +16,7 @@ BR2_LINUX_KERNEL_INSTALL_TARGET=y
>   >  BR2_TARGET_ROOTFS_EXT2=y
>   >  BR2_TARGET_ROOTFS_EXT2_4=y
>   >  BR2_TARGET_ROOTFS_EXT2_SIZE="120M"
>   > +BR2_TARGET_ROOTFS_EXT2_MKFS_OPTIONS="-O ^64bit,^metadata_csum,^dir_index"
>
> Is this a general issue with the ext4 support in u-boot? Should we do
> this generally by default, similar to how we do it for 64bit?
>
probably in case when device saves env in file on ext4 it is, but it 
should be checked on the other boards in order to inquiry if I'm right. 
As I know the requests done by U-Boot are acceptable if the ext4 file 
system is generated with only the features supported by U-Boot. 
Unfortunately I don't have any board except that which allows me to do so.


Best
Bartek
Thomas Petazzoni Nov. 9, 2019, 2:22 p.m. UTC | #3
Hello Bartosz,

On Sun,  3 Nov 2019 12:50:16 +0100
Bartosz Bilas <b.bilas@grinn-global.com> wrote:

> To solve issue with non-possibilities to mount rootfs partition we
> should disable a new mkfs features such as metadata_csum and
> dir_index because there is incompatibility with these options.
> 
> Signed-off-by: Bartosz Bilas <b.bilas@grinn-global.com>
> ---
>  configs/stm32mp157c_dk2_defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/configs/stm32mp157c_dk2_defconfig b/configs/stm32mp157c_dk2_defconfig
> index a1e61d752d..ba044e5e55 100644
> --- a/configs/stm32mp157c_dk2_defconfig
> +++ b/configs/stm32mp157c_dk2_defconfig
> @@ -16,6 +16,7 @@ BR2_LINUX_KERNEL_INSTALL_TARGET=y
>  BR2_TARGET_ROOTFS_EXT2=y
>  BR2_TARGET_ROOTFS_EXT2_4=y
>  BR2_TARGET_ROOTFS_EXT2_SIZE="120M"
> +BR2_TARGET_ROOTFS_EXT2_MKFS_OPTIONS="-O ^64bit,^metadata_csum,^dir_index"

Are you sure it is necessary to disable those three options?

Indeed, U-Boot ext4 write support clearly excludes filesystems that
have the metadata_csum option enabled:

  https://gitlab.denx.de/u-boot/u-boot/blob/master/fs/ext4/ext4_write.c#L880

However, there's nothing about the 64bit and dir_index options. How did
you conclude that they were causing problems? Did you try with just -O
^metadata_csum ?

Thanks,

Thomas
Bartosz Bilas Nov. 9, 2019, 3:25 p.m. UTC | #4
Hello Thomas,

On 09.11.2019 15:22, Thomas Petazzoni wrote:
> Hello Bartosz,
>
> On Sun,  3 Nov 2019 12:50:16 +0100
> Bartosz Bilas <b.bilas@grinn-global.com> wrote:
>
>> To solve issue with non-possibilities to mount rootfs partition we
>> should disable a new mkfs features such as metadata_csum and
>> dir_index because there is incompatibility with these options.
>>
>> Signed-off-by: Bartosz Bilas <b.bilas@grinn-global.com>
>> ---
>>   configs/stm32mp157c_dk2_defconfig | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/configs/stm32mp157c_dk2_defconfig b/configs/stm32mp157c_dk2_defconfig
>> index a1e61d752d..ba044e5e55 100644
>> --- a/configs/stm32mp157c_dk2_defconfig
>> +++ b/configs/stm32mp157c_dk2_defconfig
>> @@ -16,6 +16,7 @@ BR2_LINUX_KERNEL_INSTALL_TARGET=y
>>   BR2_TARGET_ROOTFS_EXT2=y
>>   BR2_TARGET_ROOTFS_EXT2_4=y
>>   BR2_TARGET_ROOTFS_EXT2_SIZE="120M"
>> +BR2_TARGET_ROOTFS_EXT2_MKFS_OPTIONS="-O ^64bit,^metadata_csum,^dir_index"
> Are you sure it is necessary to disable those three options?
>
> Indeed, U-Boot ext4 write support clearly excludes filesystems that
> have the metadata_csum option enabled:
>
>    https://gitlab.denx.de/u-boot/u-boot/blob/master/fs/ext4/ext4_write.c#L880
> However, there's nothing about the 64bit and dir_index options. How did
> you conclude that they were causing problems? Did you try with just -O
> ^metadata_csum ?

I was talking with the maintainer of that board on U-Boot mailing list 
[1] and he said that I should disable those 2 options (dir_index and 
metada_csum) to have U-Boot working properly. I'm not sure about 64bit 
because I saw that was set by default in buildroot. I didn't check that 
with metadata_csum option only but I'll do that and let you know if it 
works.

[1] https://lists.denx.de/pipermail/u-boot/2019-October/388618.html

Best
Bartek
>
> Thanks,
>
> Thomas
Arnout Vandecappelle Nov. 17, 2019, 7:31 p.m. UTC | #5
On 09/11/2019 16:25, Bartosz Bilas wrote:
> Hello Thomas,
> 
> On 09.11.2019 15:22, Thomas Petazzoni wrote:
>> Hello Bartosz,
>>
>> On Sun,  3 Nov 2019 12:50:16 +0100
>> Bartosz Bilas <b.bilas@grinn-global.com> wrote:
>>
>>> To solve issue with non-possibilities to mount rootfs partition we
>>> should disable a new mkfs features such as metadata_csum and
>>> dir_index because there is incompatibility with these options.
>>>
>>> Signed-off-by: Bartosz Bilas <b.bilas@grinn-global.com>
>>> ---
>>>   configs/stm32mp157c_dk2_defconfig | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/configs/stm32mp157c_dk2_defconfig
>>> b/configs/stm32mp157c_dk2_defconfig
>>> index a1e61d752d..ba044e5e55 100644
>>> --- a/configs/stm32mp157c_dk2_defconfig
>>> +++ b/configs/stm32mp157c_dk2_defconfig
>>> @@ -16,6 +16,7 @@ BR2_LINUX_KERNEL_INSTALL_TARGET=y
>>>   BR2_TARGET_ROOTFS_EXT2=y
>>>   BR2_TARGET_ROOTFS_EXT2_4=y
>>>   BR2_TARGET_ROOTFS_EXT2_SIZE="120M"
>>> +BR2_TARGET_ROOTFS_EXT2_MKFS_OPTIONS="-O ^64bit,^metadata_csum,^dir_index"
>> Are you sure it is necessary to disable those three options?
>>
>> Indeed, U-Boot ext4 write support clearly excludes filesystems that
>> have the metadata_csum option enabled:
>>
>>    https://gitlab.denx.de/u-boot/u-boot/blob/master/fs/ext4/ext4_write.c#L880
>> However, there's nothing about the 64bit and dir_index options. How did
>> you conclude that they were causing problems? Did you try with just -O
>> ^metadata_csum ?
> 
> I was talking with the maintainer of that board on U-Boot mailing list [1] and
> he said that I should disable those 2 options (dir_index and metada_csum) to
> have U-Boot working properly. I'm not sure about 64bit because I saw that was
> set by default in buildroot. I didn't check that with metadata_csum option only
> but I'll do that and let you know if it works.

 As explained in the help text of the option:

          The default is "-O ^64bit", i.e. disable 64-bit filesystem
          support. This default value has been chosen because U-Boot
          versions before 2017.02 don't support this filesystem
          option: using it may make the filesystem unreadable by
          U-Boot.

 Since this defconfig uses a more recent U-Boot, it's not necessary.

 That said, if those two options should be disabled for U-Boot to work, it's
probably a good idea to add them to the defaults (i.e. all three options, like
you put in the defconfig here).

 Regards,
 Arnout



 Regards,
 Arnout
Bartosz Bilas Nov. 18, 2019, 5:34 p.m. UTC | #6
Hello guys,

On 17.11.2019 20:31, Arnout Vandecappelle wrote:
>
> On 09/11/2019 16:25, Bartosz Bilas wrote:
>> Hello Thomas,
>>
>> On 09.11.2019 15:22, Thomas Petazzoni wrote:
>>> Hello Bartosz,
>>>
>>> On Sun,  3 Nov 2019 12:50:16 +0100
>>> Bartosz Bilas <b.bilas@grinn-global.com> wrote:
>>>
>>>> To solve issue with non-possibilities to mount rootfs partition we
>>>> should disable a new mkfs features such as metadata_csum and
>>>> dir_index because there is incompatibility with these options.
>>>>
>>>> Signed-off-by: Bartosz Bilas <b.bilas@grinn-global.com>
>>>> ---
>>>>    configs/stm32mp157c_dk2_defconfig | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/configs/stm32mp157c_dk2_defconfig
>>>> b/configs/stm32mp157c_dk2_defconfig
>>>> index a1e61d752d..ba044e5e55 100644
>>>> --- a/configs/stm32mp157c_dk2_defconfig
>>>> +++ b/configs/stm32mp157c_dk2_defconfig
>>>> @@ -16,6 +16,7 @@ BR2_LINUX_KERNEL_INSTALL_TARGET=y
>>>>    BR2_TARGET_ROOTFS_EXT2=y
>>>>    BR2_TARGET_ROOTFS_EXT2_4=y
>>>>    BR2_TARGET_ROOTFS_EXT2_SIZE="120M"
>>>> +BR2_TARGET_ROOTFS_EXT2_MKFS_OPTIONS="-O ^64bit,^metadata_csum,^dir_index"
>>> Are you sure it is necessary to disable those three options?
>>>
>>> Indeed, U-Boot ext4 write support clearly excludes filesystems that
>>> have the metadata_csum option enabled:
>>>
>>>     https://gitlab.denx.de/u-boot/u-boot/blob/master/fs/ext4/ext4_write.c#L880
>>> However, there's nothing about the 64bit and dir_index options. How did
>>> you conclude that they were causing problems? Did you try with just -O
>>> ^metadata_csum ?
>> I was talking with the maintainer of that board on U-Boot mailing list [1] and
>> he said that I should disable those 2 options (dir_index and metada_csum) to
>> have U-Boot working properly. I'm not sure about 64bit because I saw that was
>> set by default in buildroot. I didn't check that with metadata_csum option only
>> but I'll do that and let you know if it works.
>   As explained in the help text of the option:
>
>            The default is "-O ^64bit", i.e. disable 64-bit filesystem
>            support. This default value has been chosen because U-Boot
>            versions before 2017.02 don't support this filesystem
>            option: using it may make the filesystem unreadable by
>            U-Boot.
>
>   Since this defconfig uses a more recent U-Boot, it's not necessary.
>
>   That said, if those two options should be disabled for U-Boot to work, it's
> probably a good idea to add them to the defaults (i.e. all three options, like
> you put in the defconfig here).
I've checked if it worked with disabled metadata_csum option only and it 
turns out that Thomas is right. Kernel is able to mount rootfs partition 
properly even if I do any environment operation in U-Boot. So it means 
that the default 64bit option is not necessary as well.
>
>   Regards,
>   Arnout
>
>
>
>   Regards,
>   Arnout
Best
Bartek
Thomas Petazzoni Jan. 1, 2020, 4:20 p.m. UTC | #7
On Sun,  3 Nov 2019 12:50:16 +0100
Bartosz Bilas <b.bilas@grinn-global.com> wrote:

> To solve issue with non-possibilities to mount rootfs partition we
> should disable a new mkfs features such as metadata_csum and
> dir_index because there is incompatibility with these options.
> 
> Signed-off-by: Bartosz Bilas <b.bilas@grinn-global.com>
> ---
>  configs/stm32mp157c_dk2_defconfig | 1 +
>  1 file changed, 1 insertion(+)

Applied to master after changing the patch to only disable the
metadata_csum option, as we discussed in the thread following this
patch.

Thanks!

Thomas
diff mbox series

Patch

diff --git a/configs/stm32mp157c_dk2_defconfig b/configs/stm32mp157c_dk2_defconfig
index a1e61d752d..ba044e5e55 100644
--- a/configs/stm32mp157c_dk2_defconfig
+++ b/configs/stm32mp157c_dk2_defconfig
@@ -16,6 +16,7 @@  BR2_LINUX_KERNEL_INSTALL_TARGET=y
 BR2_TARGET_ROOTFS_EXT2=y
 BR2_TARGET_ROOTFS_EXT2_4=y
 BR2_TARGET_ROOTFS_EXT2_SIZE="120M"
+BR2_TARGET_ROOTFS_EXT2_MKFS_OPTIONS="-O ^64bit,^metadata_csum,^dir_index"
 # BR2_TARGET_ROOTFS_TAR is not set
 BR2_TARGET_UBOOT=y
 BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG=y