public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [U-Boot, RFC] ext4fs: le32_to_cpu() used on a 16-bit field
@ 2013-07-16  8:14 Rommel Custodio
  2013-07-16  8:33 ` Lukasz Majewski
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Rommel Custodio @ 2013-07-16  8:14 UTC (permalink / raw)
  To: u-boot

Hi All,

U-Boot 2013.07-rc3 [ELDK 5.2.1 / ELDK 5.3]

Now I've started to use the new ext4 code. I need the "ext4write" command.
Though there seems to be several problems with the ext2/ext4 code.

I am testing on an ml507 (PPC440, Big Endian).
There are some cases where the a field is 16-bit but le32_to_cpu() is used.
Some checks (ie eh_magic) fails to match even if I use a correctly ext4 
formatted MMC/SD card.

Does these seem right? Or am I mistaken?

-----
diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 58880b4..22d4377 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -1429,7 +1429,7 @@ static struct ext4_extent_header 
*ext4fs_get_extent_block
 	while (1) {
 		index = (struct ext4_extent_idx *)(ext_block + 1);
 
-		if (le32_to_cpu(ext_block->eh_magic) != EXT4_EXT_MAGIC)
+		if (le16_to_cpu(ext_block->eh_magic) != EXT4_EXT_MAGIC)
 			return 0;
 
 		if (ext_block->eh_depth == 0)
@@ -1437,14 +1437,14 @@ static struct ext4_extent_header 
*ext4fs_get_extent_block
 		i = -1;
 		do {
 			i++;
-			if (i >= le32_to_cpu(ext_block->eh_entries))
+			if (i >= le16_to_cpu(ext_block->eh_entries))
 				break;
 		} while (fileblock > le32_to_cpu(index[i].ei_block));
 
 		if (--i < 0)
 			return 0;
 
-		block = le32_to_cpu(index[i].ei_leaf_hi);
+		block = le16_to_cpu(index[i].ei_leaf_hi);
 		block = (block << 32) + le32_to_cpu(index[i].ei_leaf_lo);
 
 		if (ext4fs_devread(block << log2_blksz, 0, fs->blksz, buf))
@@ -1543,17 +1543,17 @@ long int read_allocated_block(struct ext2_inode 
*inode, int fileblock)
 
 		do {
 			i++;
-			if (i >= le32_to_cpu(ext_block->eh_entries))
+			if (i >= le16_to_cpu(ext_block->eh_entries))
 				break;
 		} while (fileblock >= le32_to_cpu(extent[i].ee_block));
 		if (--i >= 0) {
 			fileblock -= le32_to_cpu(extent[i].ee_block);
-			if (fileblock >= le32_to_cpu(extent[i].ee_len)) {
+			if (fileblock >= le16_to_cpu(extent[i].ee_len)) {
 				free(buf);
 				return 0;
 			}
 
-			start = le32_to_cpu(extent[i].ee_start_hi);
+			start = le16_to_cpu(extent[i].ee_start_hi);
 			start = (start << 32) +
 					le32_to_cpu(extent[i].ee_start_lo);
 			free(buf);
-----

(Sorry, I can't CC anyone directly as I'm using the gmane "post" interface)

All the best,
Rommel

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

* [U-Boot] [U-Boot, RFC] ext4fs: le32_to_cpu() used on a 16-bit field
  2013-07-16  8:14 [U-Boot] [U-Boot, RFC] ext4fs: le32_to_cpu() used on a 16-bit field Rommel Custodio
@ 2013-07-16  8:33 ` Lukasz Majewski
       [not found]   ` <20130716204531.GA548@CrG.local>
  2013-07-19  7:03 ` Lukasz Majewski
  2013-08-05  7:31 ` Pavel Machek
  2 siblings, 1 reply; 8+ messages in thread
From: Lukasz Majewski @ 2013-07-16  8:33 UTC (permalink / raw)
  To: u-boot

Hi Rommel,

> Hi All,
> 
> U-Boot 2013.07-rc3 [ELDK 5.2.1 / ELDK 5.3]
> 
> Now I've started to use the new ext4 code. I need the "ext4write"
> command. Though there seems to be several problems with the ext2/ext4
> code.

The ext4 code at u-boot (especially ext4write) needs special attention. 

It is on top of my list to refactor and clean up this code base
(especially the ext4write command). 

> 
> I am testing on an ml507 (PPC440, Big Endian).
> There are some cases where the a field is 16-bit but le32_to_cpu() is
> used. Some checks (ie eh_magic) fails to match even if I use a
> correctly ext4 formatted MMC/SD card.
> 
> Does these seem right? Or am I mistaken?

I would need to double check it (since I'm using only ext4load at my
target board currently).

> 
> -----
> diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
> index 58880b4..22d4377 100644
> --- a/fs/ext4/ext4_common.c
> +++ b/fs/ext4/ext4_common.c
> @@ -1429,7 +1429,7 @@ static struct ext4_extent_header 
> *ext4fs_get_extent_block
>  	while (1) {
>  		index = (struct ext4_extent_idx *)(ext_block + 1);
>  
> -		if (le32_to_cpu(ext_block->eh_magic) !=
> EXT4_EXT_MAGIC)
> +		if (le16_to_cpu(ext_block->eh_magic) !=
> EXT4_EXT_MAGIC) return 0;
>  
>  		if (ext_block->eh_depth == 0)
> @@ -1437,14 +1437,14 @@ static struct ext4_extent_header 
> *ext4fs_get_extent_block
>  		i = -1;
>  		do {
>  			i++;
> -			if (i >= le32_to_cpu(ext_block->eh_entries))
> +			if (i >= le16_to_cpu(ext_block->eh_entries))
				 ^^^^^ We had some problems with
				 unaligned access to 16 bit values
				 recently (when those start at 0x2).
				 It was observed on our ARM Exynos4
				 based target.


>  				break;
>  		} while (fileblock > le32_to_cpu(index[i].ei_block));
>  
>  		if (--i < 0)
>  			return 0;
>  
> -		block = le32_to_cpu(index[i].ei_leaf_hi);
> +		block = le16_to_cpu(index[i].ei_leaf_hi);
>  		block = (block << 32) +
> le32_to_cpu(index[i].ei_leaf_lo); 
>  		if (ext4fs_devread(block << log2_blksz, 0,
> fs->blksz, buf)) @@ -1543,17 +1543,17 @@ long int
> read_allocated_block(struct ext2_inode *inode, int fileblock)
>  
>  		do {
>  			i++;
> -			if (i >= le32_to_cpu(ext_block->eh_entries))
> +			if (i >= le16_to_cpu(ext_block->eh_entries))
>  				break;
>  		} while (fileblock >=
> le32_to_cpu(extent[i].ee_block)); if (--i >= 0) {
>  			fileblock -= le32_to_cpu(extent[i].ee_block);
> -			if (fileblock >=
> le32_to_cpu(extent[i].ee_len)) {
> +			if (fileblock >=
> le16_to_cpu(extent[i].ee_len)) { free(buf);
>  				return 0;
>  			}
>  
> -			start = le32_to_cpu(extent[i].ee_start_hi);
> +			start = le16_to_cpu(extent[i].ee_start_hi);
>  			start = (start << 32) +
>  					le32_to_cpu(extent[i].ee_start_lo);
>  			free(buf);
> -----
> 
> (Sorry, I can't CC anyone directly as I'm using the gmane "post"
> interface)

You can also CC ext4 u-boot's implementation authors.

> 
> All the best,
> Rommel
> 
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

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

* [U-Boot] [U-Boot, RFC] ext4fs: le32_to_cpu() used on a 16-bit field
       [not found]   ` <20130716204531.GA548@CrG.local>
@ 2013-07-16 23:32     ` Rommel Custodio
  0 siblings, 0 replies; 8+ messages in thread
From: Rommel Custodio @ 2013-07-16 23:32 UTC (permalink / raw)
  To: u-boot

Sorry for the extra noise.

On Wednesday, July 17, 2013, Rommel G Custodio <sessyargc@gmail.com> wrote:
> Dear Lukasz Majewski,
>
> On 2013.07/16, Lukasz Majewski wrote:
>> Hi Rommel,
>>
>> > Hi All,
>> >
>> > U-Boot 2013.07-rc3 [ELDK 5.2.1 / ELDK 5.3]
>> >
>> > Now I've started to use the new ext4 code. I need the "ext4write"
>> > command. Though there seems to be several problems with the ext2/ext4
>> > code.
>>
>> The ext4 code at u-boot (especially ext4write) needs special attention.
>>
>> It is on top of my list to refactor and clean up this code base
>> (especially the ext4write command).
>>
>
> I know the code is fairly recent and it seems limited testing (on a
> limited number of platforms) has only been performed.
>
>> >
>> > I am testing on an ml507 (PPC440, Big Endian).
>> > There are some cases where the a field is 16-bit but le32_to_cpu() is
>> > used. Some checks (ie eh_magic) fails to match even if I use a
>> > correctly ext4 formatted MMC/SD card.
>> >
>> > Does these seem right? Or am I mistaken?
>>
>> I would need to double check it (since I'm using only ext4load at my
>> target board currently).
>>
>
> Without the patches below ext4load/ext2load doesn't work correctly on the
> ml507.
>
> For testing, the default settings are used enabled). The SD card mounts
> correctly using ext4 in Linux but I get this error in u-boot:
> ml507:/# ext4ls mmc 0:1
> <DIR>       4096 .
> <DIR>       4096 ..
> <DIR>      16384 lost+found
>          9294153 mybit.ace
> ml507:/# ext2ls mmc 0:1
> <DIR>       4096 .
> <DIR>       4096 ..
> <DIR>      16384 lost+found
>         9294153 mybit.ace
> ml507:/# ext4load mmc 0:1 1000000 /mybit.ace
> invalid extent block
> ml507:/# ext2load mmc 0:1 1000000 mybit.ace
> invalid extent block
>
>> >             do {
>> >                     i++;
>> > -                   if (i >= le32_to_cpu(ext_block->eh_entries))
>> > +                   if (i >= le16_to_cpu(ext_block->eh_entries))
>>                                ^^^^^ We had some problems with
>>                                unaligned access to 16 bit values
>>                                recently (when those start at 0x2).
>>                                It was observed on our ARM Exynos4
>>                                based target.
>>
>>
>> >
>> > (Sorry, I can't CC anyone directly as I'm using the gmane "post"
>> > interface)
>>
>> You can also CC ext4 u-boot's implementation authors.

Forgot to Cc the list
And the original author's email seem to be bouncing.

>>
>> >
>> > All the best,
>> > Rommel
>> >
>> >
>> > _______________________________________________
>> > U-Boot mailing list
>> > U-Boot at lists.denx.de
>> > http://lists.denx.de/mailman/listinfo/u-boot
>>
>>
>>
>> --
>> Best regards,
>>
>> Lukasz Majewski
>>
>> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
>

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

* [U-Boot] [U-Boot, RFC] ext4fs: le32_to_cpu() used on a 16-bit field
  2013-07-16  8:14 [U-Boot] [U-Boot, RFC] ext4fs: le32_to_cpu() used on a 16-bit field Rommel Custodio
  2013-07-16  8:33 ` Lukasz Majewski
@ 2013-07-19  7:03 ` Lukasz Majewski
  2013-07-20  8:03   ` [PATCH, RESEND] " Rommel G Custodio
  2013-07-21  4:32   ` [U-Boot] [U-Boot, RFC] " Simon Glass
  2013-08-05  7:31 ` Pavel Machek
  2 siblings, 2 replies; 8+ messages in thread
From: Lukasz Majewski @ 2013-07-19  7:03 UTC (permalink / raw)
  To: u-boot

On Tue, 16 Jul 2013 08:14:35 +0000 (UTC) Rommel Custodio
sessyargc+uboot at gmail.com wrote,

Hi Rommel,

> Hi All,
> 
> U-Boot 2013.07-rc3 [ELDK 5.2.1 / ELDK 5.3]
> 
> Now I've started to use the new ext4 code. I need the "ext4write"
> command. Though there seems to be several problems with the ext2/ext4
> code.
> 
> I am testing on an ml507 (PPC440, Big Endian).
> There are some cases where the a field is 16-bit but le32_to_cpu() is
> used. Some checks (ie eh_magic) fails to match even if I use a
> correctly ext4 formatted MMC/SD card.
> 
> Does these seem right? Or am I mistaken?

What kind of mailer program have you used to sent this e-mail?
I receive following errors:

fatal: corrupt patch at line 111

I cannot apply this patch with either git am -3 or git apply.

Were you using git send-email?

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

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

* [PATCH, RESEND] ext4fs: le32_to_cpu() used on a 16-bit field
  2013-07-19  7:03 ` Lukasz Majewski
@ 2013-07-20  8:03   ` Rommel G Custodio
  2013-07-21  4:32   ` [U-Boot] [U-Boot, RFC] " Simon Glass
  1 sibling, 0 replies; 8+ messages in thread
From: Rommel G Custodio @ 2013-07-20  8:03 UTC (permalink / raw)
  To: u-boot

Dear Lukasz Majewski

On 2013.07/19, Lukasz Majewski wrote:
> fatal: corrupt patch at line 111
>
> I cannot apply this patch with either git am -3 or git apply.


I'm resending the patch.
>> (Sorry, I can't CC anyone directly as I'm using the gmane "post"
>> interface)

All the best,
Rommel

Signed-off-by: Rommel G Custodio <sessyargc.jp@gmail.com>
---
 fs/ext4/ext4_common.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 2776293..ff9c4ec 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -1432,7 +1432,7 @@ static struct ext4_extent_header *ext4fs_get_extent_block
 	while (1) {
 		index = (struct ext4_extent_idx *)(ext_block + 1);
 
-		if (le32_to_cpu(ext_block->eh_magic) != EXT4_EXT_MAGIC)
+		if (le16_to_cpu(ext_block->eh_magic) != EXT4_EXT_MAGIC)
 			return 0;
 
 		if (ext_block->eh_depth == 0)
@@ -1440,14 +1440,14 @@ static struct ext4_extent_header *ext4fs_get_extent_block
 		i = -1;
 		do {
 			i++;
-			if (i >= le32_to_cpu(ext_block->eh_entries))
+			if (i >= le16_to_cpu(ext_block->eh_entries))
 				break;
 		} while (fileblock > le32_to_cpu(index[i].ei_block));
 
 		if (--i < 0)
 			return 0;
 
-		block = le32_to_cpu(index[i].ei_leaf_hi);
+		block = le16_to_cpu(index[i].ei_leaf_hi);
 		block = (block << 32) + le32_to_cpu(index[i].ei_leaf_lo);
 
 		if (ext4fs_devread((lbaint_t)block << log2_blksz, 0, fs->blksz,
@@ -1548,17 +1548,17 @@ long int read_allocated_block(struct ext2_inode *inode, int fileblock)
 
 		do {
 			i++;
-			if (i >= le32_to_cpu(ext_block->eh_entries))
+			if (i >= le16_to_cpu(ext_block->eh_entries))
 				break;
 		} while (fileblock >= le32_to_cpu(extent[i].ee_block));
 		if (--i >= 0) {
 			fileblock -= le32_to_cpu(extent[i].ee_block);
-			if (fileblock >= le32_to_cpu(extent[i].ee_len)) {
+			if (fileblock >= le16_to_cpu(extent[i].ee_len)) {
 				free(buf);
 				return 0;
 			}
 
-			start = le32_to_cpu(extent[i].ee_start_hi);
+			start = le16_to_cpu(extent[i].ee_start_hi);
 			start = (start << 32) +
 					le32_to_cpu(extent[i].ee_start_lo);
 			free(buf);
-- 
1.8.3.3

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

* [U-Boot] [U-Boot, RFC] ext4fs: le32_to_cpu() used on a 16-bit field
  2013-07-19  7:03 ` Lukasz Majewski
  2013-07-20  8:03   ` [PATCH, RESEND] " Rommel G Custodio
@ 2013-07-21  4:32   ` Simon Glass
  2013-07-21  8:24     ` Andreas Bießmann
  1 sibling, 1 reply; 8+ messages in thread
From: Simon Glass @ 2013-07-21  4:32 UTC (permalink / raw)
  To: u-boot

+Tom

On Fri, Jul 19, 2013 at 1:03 AM, Lukasz Majewski <l.majewski@samsung.com>wrote:

> On Tue, 16 Jul 2013 08:14:35 +0000 (UTC) Rommel Custodio
> sessyargc+uboot at gmail.com wrote,
>
> Hi Rommel,
>
> > Hi All,
> >
> > U-Boot 2013.07-rc3 [ELDK 5.2.1 / ELDK 5.3]
> >
> > Now I've started to use the new ext4 code. I need the "ext4write"
> > command. Though there seems to be several problems with the ext2/ext4
> > code.
> >
> > I am testing on an ml507 (PPC440, Big Endian).
> > There are some cases where the a field is 16-bit but le32_to_cpu() is
> > used. Some checks (ie eh_magic) fails to match even if I use a
> > correctly ext4 formatted MMC/SD card.
> >
> > Does these seem right? Or am I mistaken?
>
> What kind of mailer program have you used to sent this e-mail?
> I receive following errors:
>
> fatal: corrupt patch at line 111
>
> I cannot apply this patch with either git am -3 or git apply.
>
> Were you using git send-email?
>

It seems like we should try to fix this before the release?

Regards,
Simon

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

* [U-Boot] [U-Boot, RFC] ext4fs: le32_to_cpu() used on a 16-bit field
  2013-07-21  4:32   ` [U-Boot] [U-Boot, RFC] " Simon Glass
@ 2013-07-21  8:24     ` Andreas Bießmann
  0 siblings, 0 replies; 8+ messages in thread
From: Andreas Bießmann @ 2013-07-21  8:24 UTC (permalink / raw)
  To: u-boot

Hi all,

On 21.07.2013 06:32, Simon Glass wrote:
> +Tom
>
> On Fri, Jul 19, 2013 at 1:03 AM, Lukasz Majewski <l.majewski@samsung.com>wrote:
>
>> On Tue, 16 Jul 2013 08:14:35 +0000 (UTC) Rommel Custodio
>> sessyargc+uboot at gmail.com wrote,
>>
>> Hi Rommel,
>>
>>> Hi All,
>>>
>>> U-Boot 2013.07-rc3 [ELDK 5.2.1 / ELDK 5.3]
>>>
>>> Now I've started to use the new ext4 code. I need the "ext4write"
>>> command. Though there seems to be several problems with the ext2/ext4
>>> code.
>>>
>>> I am testing on an ml507 (PPC440, Big Endian).
>>> There are some cases where the a field is 16-bit but le32_to_cpu() is
>>> used. Some checks (ie eh_magic) fails to match even if I use a
>>> correctly ext4 formatted MMC/SD card.
>>>
>>> Does these seem right? Or am I mistaken?
>>
>> What kind of mailer program have you used to sent this e-mail?
>> I receive following errors:
>>
>> fatal: corrupt patch at line 111
>>
>> I cannot apply this patch with either git am -3 or git apply.
>>
>> Were you using git send-email?
>>
>
> It seems like we should try to fix this before the release?

+1

I can confirm that reading ext4 extended header on BE machine (avr32) is 
broken. I'm currently on it to get it working by using Rommel's patch.

Regards

Andreas Bie?mann

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

* [U-Boot] [U-Boot, RFC] ext4fs: le32_to_cpu() used on a 16-bit field
  2013-07-16  8:14 [U-Boot] [U-Boot, RFC] ext4fs: le32_to_cpu() used on a 16-bit field Rommel Custodio
  2013-07-16  8:33 ` Lukasz Majewski
  2013-07-19  7:03 ` Lukasz Majewski
@ 2013-08-05  7:31 ` Pavel Machek
  2 siblings, 0 replies; 8+ messages in thread
From: Pavel Machek @ 2013-08-05  7:31 UTC (permalink / raw)
  To: u-boot

Hi!

> U-Boot 2013.07-rc3 [ELDK 5.2.1 / ELDK 5.3]
> 
> Now I've started to use the new ext4 code. I need the "ext4write" command.
> Though there seems to be several problems with the ext2/ext4 code.
> 
> I am testing on an ml507 (PPC440, Big Endian).
> There are some cases where the a field is 16-bit but le32_to_cpu() is used.
> Some checks (ie eh_magic) fails to match even if I use a correctly ext4 
> formatted MMC/SD card.
> 
> Does these seem right? Or am I mistaken?

This fixed ext4 on powerpc-based board. Thanks!

Tested-by: Pavel Machek <pavel@denx.de>
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

end of thread, other threads:[~2013-08-05  7:31 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-16  8:14 [U-Boot] [U-Boot, RFC] ext4fs: le32_to_cpu() used on a 16-bit field Rommel Custodio
2013-07-16  8:33 ` Lukasz Majewski
     [not found]   ` <20130716204531.GA548@CrG.local>
2013-07-16 23:32     ` Rommel Custodio
2013-07-19  7:03 ` Lukasz Majewski
2013-07-20  8:03   ` [PATCH, RESEND] " Rommel G Custodio
2013-07-21  4:32   ` [U-Boot] [U-Boot, RFC] " Simon Glass
2013-07-21  8:24     ` Andreas Bießmann
2013-08-05  7:31 ` Pavel Machek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox