[U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Przemyslaw Marczak
p.marczak at samsung.com
Thu Dec 18 11:26:23 CET 2014
Hello Simon,
On 12/18/2014 04:39 AM, Simon Glass wrote:
> Hi Przemyslaw,
>
> On 17 December 2014 at 02:03, Przemyslaw Marczak <p.marczak at samsung.com> wrote:
>> Hello,
>>
>>
>> On 12/16/2014 11:26 PM, Simon Glass wrote:
>>>
>>> Hi Przemyslaw,
>>>
>>> On 12 December 2014 at 08:30, Przemyslaw Marczak <p.marczak at samsung.com>
>>> wrote:
>>>>
>>>> Hello,
>>>>
>>>>
>>>> On 12/12/2014 01:32 AM, Simon Glass wrote:
>>>>>
>>>>>
>>>>> Hi Przemyslaw,
>>>>>
>>>>> On 11 December 2014 at 05:01, Przemyslaw Marczak <p.marczak at samsung.com>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> The present fat implementation ignores FAT16 long name
>>>>>> directory entries which aren't placed in a single sector.
>>>>>>
>>>>>> This was becouse of the buffer was always filled by the
>>>>>> two sectors, and the loop was made also for two sectors.
>>>>>>
>>>>>> If some file long name entries are stored in two sectors,
>>>>>> the we have two cases:
>>>>>>
>>>>>> Case 1:
>>>>>> Both of sectors are in the buffer - all required data
>>>>>> for long file name is in the buffer.
>>>>>> - Read OK!
>>>>>>
>>>>>> Case 2:
>>>>>> The current directory entry is placed at the end of the
>>>>>> second buffered sector. And the next entries are placed
>>>>>> in a sector which is not buffered yet. Then two next
>>>>>> sectors are buffered and the mentioned entry is ignored.
>>>>>> - Read fail!
>>>>>>
>>>>>> This commit fixes this issue by:
>>>>>> - read two sectors after loop on each single is done
>>>>>> - keep the last used sector as a first in the buffer
>>>>>> before the read of two next
>>>>>>
>>>>>> The commit doesn't affects the fat32 imlementation,
>>>>>> which works good as previous.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> This is very interesting\! Is this the same failure that I saw on this
>>>>> thread?
>>>>>
>>>>>
>>>>>
>>>>> http://u-boot.10912.n7.nabble.com/PATCH-U-Boot-ARM-rpi-b-detect-board-revision-td196720.html
>>>>>
>>>>> (search for fatload)
>>>>>
>>>>> "I tried this out. It worked OK for me except that it can't find the
>>>>> device tree file bcm2835-rpi-b-rev2.dtb.
>>>>>
>>>>> Oddly I can fatload it from /bcm2835-rpi-b-rev2.dtb but when I try
>>>>> from /syslinux/..//bcm2835-rpi-b-rev2.dtb it fails and cannot find the
>>>>> file. Reducing the filename length to 8 chars works. I wonder what
>>>>> year of my life FAT will stop plaguing me? "
>>>>>
>>>>>
>>>>> Also can you write a test for this in test/fs/fs-test.sh?
>>>>>
>>>>> Regards,
>>>>> Simon
>>>>>
>>>>> [snip]
>>>>>
>>>>
>>>> Probably this is an another case which is caused by the sector buffer
>>>> bug.
>>>> Does this patch fixed your issue?
>>>>
>>>> I have some simple test for manual use with the ums tool.
>>>> It just copy the test files to the tested fat16 partition mounted using
>>>> the
>>>> UMS, next it computes CRC32 of those files on the host and next using
>>>> U-Boot
>>>> fatload/crc32 commands - it tests the read feature. But it's not full
>>>> automated - I didn't work on getting the log from U-Boot console.
>>>>
>>>> So I could check if the file checksums are proper and if all files were
>>>> found on the partiion, by the U-Boot read command. It's not useful for
>>>> the
>>>> "test/fs/fs-test.sh" because this is not designed for the sandbox.
>>>> My test writes some commands directly to U-Boot console, like this: echo
>>>> "some cmd" > /dev/ttyS0.
>>>>
>>>> Unfortunately the sandbox config seems to be broken.
>>>>
>>>> The bug was not so obvious, any read/write on fat partition can change
>>>> fat
>>>> directory entries or add the new ones and then all data can be read
>>>> right.
>>>>
>>>> I will send the scripts for such simple test.
>>>
>>>
>>> I'm not sure if it fixes my problem but it seems likely. I will see if
>>> I can make time to test it.
>>>
>>> If you want to write input data to U-Boot sandbox we can do that
>>> fairly easily. You can import cros_subprocess and use the function
>>> there to generate output from your test and inspect the input from
>>> U-Boot's command line. Let me know if you'd like an example.
>>>
>>> Regards,
>>> Simon
>>>
>>
>> Before, I wrote, that sandbox seems to be broken, sorry for this - it was
>> just my dirty repo - sandbox compiles and works well.
>
> Somewhat bewildering, but it does not in fact fix my problem.
>
> Here is a compressed version of the fat filesystem if you want to take a look:
>
> https://drive.google.com/file/d/0B7WYZbZ9zd-3NGRMNkFQQTdtV2M/view?usp=sharing
>
> Regards,
> Simon
>
I had put this image on my Trats2 device on partition mmc 0:6 using ums
and dd linux command. Next I put latest mainline u-boot, commit:
e3bf81b1e841ecabe7c8b3d48621256db8b8623e : "Merge branch 'master' of
git://git.denx.de/u-boot-mpc85xx"
So this is the version with the fat bug. But I can see and load the
file: "bcm2835-rpi-b-rev2.dtb".
Trats2 # fatls mmc 0:6
17840 bootcode.bin
120 cmdline.txt
1331 config.txt
6115 fixup.dat
2324 fixup_cd.dat
9166 fixup_x.dat
3232856 kernel.img
2615064 start.elf
533080 start_cd.elf
3572200 start_x.elf
137 issue.txt
18974 license.oracle
295524 u-boot.bin
1331 config.txt~
extlinux/
3368648 zimage
3963 bcm2835-rpi-b.dtb
3963 bcm2835.dtb
3963 bcm2835-rpi-b-rev2.dtb
18 file(s), 1 dir(s)
Trats2 # fatload mmc 0:6 0x40000000 bcm2835-rpi-b-rev2.dtb
reading bcm2835-rpi-b-rev2.dtb
3963 bytes read in 5 ms (773.4 KiB/s)
Trats2 # crc32 0x40000000 0xf7b
CRC32 for 40000000 ... 40000f7a ==> c36ee8db
Trats2 #
The only missing file is "boot.scr", it's ignored by "ls" command but
can be loaded...
Trats2 # fatload mmc 0:6 0x40000000 boot.scr
reading boot.scr
256 bytes read in 2 ms (125 KiB/s)
Trats2 # crc32 0x40000000 0x100
CRC32 for 40000000 ... 400000ff ==> dc5c79b3
I suppose that the partition image which you shared for me is little
different, than this mentioned in the topic "[PATCH U-Boot] ARM: rpi_b:
detect board revision"
Probably the sequence of writing files to this partition was different,
and the different file is ignored.
After putting the debug macro on the top of fs/fat/fat.c:
Trats2 # fatls mmc 0:6
VFAT Support enabled
FAT16, fat_sect: 16, fatlength: 32
Rootdir begins at cluster: 0, sector: 80, offset: a000
Data begins at: 80
Sector size: 512, cluster size: 16
FAT read sect=80, clust_size=16, DIRENTSPERBLOCK=16
17840 bootcode.bin
120 cmdline.txt
1331 config.txt
6115 fixup.dat
2324 fixup_cd.dat
9166 fixup_x.dat
3232856 kernel.img
2615064 start.elf
END LOOP: j=0 clust_size=16
533080 start_cd.elf
3572200 start_x.elf
137 issue.txt
18974 license.oracle
295524 u-boot.bin
1331 config.txt~
END LOOP: j=1 clust_size=16
FAT read sect=82, clust_size=16, DIRENTSPERBLOCK=16
extlinux/
3368648 zimage
3963 bcm2835-rpi-b.dtb
3963 bcm2835.dtb
3963 bcm2835-rpi-b-rev2.dtb
END LOOP: j=0 clust_size=16
RootDentname == NULL - 0
18 file(s), 1 dir(s)
And next test on commit 9b416a9f4ca7cf5ac4d5f7143d67edde7f7d7326 with my
fat patch.
Trats2 # fatls mmc 0:6
17840 bootcode.bin
120 cmdline.txt
1331 config.txt
6115 fixup.dat
2324 fixup_cd.dat
9166 fixup_x.dat
3232856 kernel.img
2615064 start.elf
533080 start_cd.elf
3572200 start_x.elf
137 issue.txt
18974 license.oracle
295524 u-boot.bin
1331 config.txt~
256 boot.scr
extlinux/
3368648 zimage
3963 bcm2835-rpi-b.dtb
3963 bcm2835.dtb
3963 bcm2835-rpi-b-rev2.dtb
19 file(s), 1 dir(s)
Trats2 # fatload mmc 0:6 0x40000000 bcm2835-rpi-b-rev2.dtb
reading bcm2835-rpi-b-rev2.dtb
3963 bytes read in 5 ms (773.4 KiB/s)
Trats2 # crc32 0x40000000 0xf7b
CRC32 for 40000000 ... 40000f7a ==> c36ee8db
Trats2 # fatload mmc 0:6 0x40000000 boot.scr
reading boot.scr
256 bytes read in 2 ms (125 KiB/s)
Trats2 # crc32 0x40000000 0x100
CRC32 for 40000000 ... 400000ff ==> dc5c79b3
So the only difference on this image is, that with my patch, the file
"boot.scr" ignored by ls command is now visible - but in both cases it
can be loaded into memory and the crc is correct.
After enabling the debug:
Trats2 # fatls mmc 0:6
VFAT Support enabled
FAT16, fat_sect: 16, fatlength: 32
Rootdir begins at cluster: 0, sector: 80, offset: a000
Data begins at: 80
Sector size: 512, cluster size: 16
FAT read sect=80, clust_size=16, DIRENTSPERBLOCK=16
17840 bootcode.bin
120 cmdline.txt
1331 config.txt
6115 fixup.dat
2324 fixup_cd.dat
9166 fixup_x.dat
3232856 kernel.img
2615064 start.elf
END LOOP: j=0 clust_size=16
FAT read sect=81, clust_size=16, DIRENTSPERBLOCK=16
533080 start_cd.elf
3572200 start_x.elf
137 issue.txt
18974 license.oracle
295524 u-boot.bin
1331 config.txt~
256 boot.scr
END LOOP: j=1 clust_size=16
FAT read sect=82, clust_size=16, DIRENTSPERBLOCK=16
extlinux/
3368648 zimage
3963 bcm2835-rpi-b.dtb
3963 bcm2835.dtb
3963 bcm2835-rpi-b-rev2.dtb
END LOOP: j=0 clust_size=16
FAT read sect=83, clust_size=16, DIRENTSPERBLOCK=16
RootDentname == NULL - 0
19 file(s), 1 dir(s)
So as I checked the file:
256 boot.scr
is next behind to the:
1331 config.txt~
And this can be checked with hex dump:
hd -s 0xa400 -n 512 bad-fat.dd
Your fat image is good example of what my patch fixes.
As you can see on the simple debug info, without the fix,the sector 80
and 81 is stored in the buffer (there are always 2 sectors in the
buffer). If you see the hex dump of the second sector:
hd -s 0xa200 -n 512 bad-fat.dd
You will see that at the end of this sector, there is a long name entry
for file "boot.scr".
In the next loop (without the fix):
END LOOP: j=1 clust_size=16
FAT read sect=82, clust_size=16, DIRENTSPERBLOCK=16
extlinux/
the sector 82 and 83 is loaded in to the buffer, so the long name entry
of "boot.scr" file from the end of sector 81 is now in the heaven, and
the file will be ignored by the ls command.
The sector 82 can be checked by:
hd -s 0xa400 -n 512 bad-fat.dd
It begins with the short name entry of file "boot.scr".
After applying my fix, there are always three sectors in the buffer,
because the last one is moved into the buffer begin and two next are
loaded into the buffer next to the last one.
And the buffer pointer is always on the second buffered sector beside
first loop.
So I think this patch fixes the issue well.
Could you describe your issue more precisely?
Best regards,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com
More information about the U-Boot
mailing list