Btrfs and endian convert

Btrfs and endian convert

[Qu Wenruo]

Hi Marek,

I respect your independent implementation of btrfs inside U-boot, but it
looks like the code is too creative, other than follow the regular btrfs
code.

One of the biggest problem is, there is no convert of on-disk endian and
in-memory endian.

The most obvious proof is the lack of btrfs_disk_key, on-disk
btrfs_disk_key is returned and used against native endian btrfs_key.

Is U-boot only supposed to be run on little endian system?

And, would you mind me to do the full cross port of btrfs-progs code to
U-boot?
I found a lot of practice pretty far away from the common code base of
btrfs-progs and kernel, like using on-disk key pointer a lot (which
should be avoid due to endian convert).

Mind me to re-build the btrfs implementation from scratch?

Thanks,
Qu

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200409/b1e5843e/attachment.sig>

Btrfs and endian convert

[Qu Wenruo]

On 2020/4/9 ??2:27, Qu Wenruo wrote:
> Hi Marek,
> 
> I respect your independent implementation of btrfs inside U-boot, but it
> looks like the code is too creative, other than follow the regular btrfs
> code.
> 
> One of the biggest problem is, there is no convert of on-disk endian and
> in-memory endian.
> 
> The most obvious proof is the lack of btrfs_disk_key, on-disk
> btrfs_disk_key is returned and used against native endian btrfs_key.
> 
> Is U-boot only supposed to be run on little endian system?

My bad, you do the convert the convert at different timing.

For header and item/node ptr, it's converted as tree block read time,
while for other structures, you do the convert when reading them.

This is pretty different from what we do in kernel/progs.

> 
> And, would you mind me to do the full cross port of btrfs-progs code to
> U-boot?
> I found a lot of practice pretty far away from the common code base of
> btrfs-progs and kernel, like using on-disk key pointer a lot (which
> should be avoid due to endian convert).
> 
> Mind me to re-build the btrfs implementation from scratch?

While this still stands, as latest btrfs_tree.h would include both
btrfs_key and btrfs_disk_key, if we really want to sync the header, we
still need to go the regular BTRFS_SETGET_* macros way.

Thanks,
Qu

> 
> Thanks,
> Qu
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200409/ebed6fa8/attachment.sig>

Btrfs and endian convert

[Marek Behun]

On Thu, 9 Apr 2020 14:53:47 +0800
Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:

> On 2020/4/9 ??2:27, Qu Wenruo wrote:
> > Hi Marek,
> > 
> > I respect your independent implementation of btrfs inside U-boot, but it
> > looks like the code is too creative, other than follow the regular btrfs
> > code.
> > 
> > One of the biggest problem is, there is no convert of on-disk endian and
> > in-memory endian.
> > 
> > The most obvious proof is the lack of btrfs_disk_key, on-disk
> > btrfs_disk_key is returned and used against native endian btrfs_key.
> > 
> > Is U-boot only supposed to be run on little endian system?  
> 

It supports big endian as well.

> My bad, you do the convert the convert at different timing.

I never tested it on big endian, but conv-funcs.h should do it...

> For header and item/node ptr, it's converted as tree block read time,
> while for other structures, you do the convert when reading them.
> 
> This is pretty different from what we do in kernel/progs.

Yes, unfortunately.

> > 
> > And, would you mind me to do the full cross port of btrfs-progs code to
> > U-boot?
> > I found a lot of practice pretty far away from the common code base of
> > btrfs-progs and kernel, like using on-disk key pointer a lot (which
> > should be avoid due to endian convert).
> > 
> > Mind me to re-build the btrfs implementation from scratch?  
> 
> While this still stands, as latest btrfs_tree.h would include both
> btrfs_key and btrfs_disk_key, if we really want to sync the header, we
> still need to go the regular BTRFS_SETGET_* macros way.

If you think it will grow more fruit in the long way, go for it. What
is your main objective here? To support more features, like filesystem
on multiple devices, write support? Or just to be able to sync code so
that bugs are caught early?

Marek

Btrfs and endian convert

[Qu Wenruo]

On 2020/4/9 ??9:24, Marek Behun wrote:
> On Thu, 9 Apr 2020 14:53:47 +0800
> Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
> 
>> On 2020/4/9 ??2:27, Qu Wenruo wrote:
>>> Hi Marek,
>>>
>>> I respect your independent implementation of btrfs inside U-boot, but it
>>> looks like the code is too creative, other than follow the regular btrfs
>>> code.
>>>
>>> One of the biggest problem is, there is no convert of on-disk endian and
>>> in-memory endian.
>>>
>>> The most obvious proof is the lack of btrfs_disk_key, on-disk
>>> btrfs_disk_key is returned and used against native endian btrfs_key.
>>>
>>> Is U-boot only supposed to be run on little endian system?  
>>
> 
> It supports big endian as well.
> 
>> My bad, you do the convert the convert at different timing.
> 
> I never tested it on big endian, but conv-funcs.h should do it...
> 
>> For header and item/node ptr, it's converted as tree block read time,
>> while for other structures, you do the convert when reading them.
>>
>> This is pretty different from what we do in kernel/progs.
> 
> Yes, unfortunately.
> 
>>>
>>> And, would you mind me to do the full cross port of btrfs-progs code to
>>> U-boot?
>>> I found a lot of practice pretty far away from the common code base of
>>> btrfs-progs and kernel, like using on-disk key pointer a lot (which
>>> should be avoid due to endian convert).
>>>
>>> Mind me to re-build the btrfs implementation from scratch?  
>>
>> While this still stands, as latest btrfs_tree.h would include both
>> btrfs_key and btrfs_disk_key, if we really want to sync the header, we
>> still need to go the regular BTRFS_SETGET_* macros way.
> 
> If you think it will grow more fruit in the long way, go for it. What
> is your main objective here? To support more features, like filesystem
> on multiple devices, write support? Or just to be able to sync code so
> that bugs are caught early?

Both.

Write support is not a high priority, but a write-like ability for
in-memory only log replay is a critical thing.

Currently we just treat the fs as there is no log tree (caused by fsync
and dirty shutdown).

But if there are new things, especially the
extlinux/kernel/initrd/script in the log tree, we will only get to the
old data, causing some inconsistent, and for the worst case, may not
even boot.


And all the other advantage from code syncing.


The only extra concern is, is your new code designed to to make the text
section smaller to fit U-boot better or just to make it easier smaller,
without all the complexity?

If it's the former case, I need to pay extra attention to ensure the
progs implementation won't grow too large.

Thanks,
Qu

> 
> Marek
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200409/7fe5dd43/attachment.sig>

Btrfs and endian convert

[Marek Behun]

On Thu, 9 Apr 2020 21:31:08 +0800
Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:

> The only extra concern is, is your new code designed to to make the text
> section smaller to fit U-boot better or just to make it easier smaller,
> without all the complexity?
> 
> If it's the former case, I need to pay extra attention to ensure the
> progs implementation won't grow too large.

I did not pay extra attention to it, but there were some discussions
concerning this. Someone wanted to add BTRFS support to default
DISTROBOOT config, but there were complaints whether it won't break
some machines with small memory.

I wanted to try using LTO optimizations or something to check how much
it will help, but never found time for it.

Marek