* [U-Boot] [PATCH] Revert "fs: fat: support write with non-zero offset"
@ 2019-03-14 9:20 Faiz Abbas
2019-03-14 15:35 ` Tom Rini
0 siblings, 1 reply; 2+ messages in thread
From: Faiz Abbas @ 2019-03-14 9:20 UTC (permalink / raw)
To: u-boot
This reverts commit cb8af8af5ba03ae8e0a7315b66bfcc46d5c55627.
fatwrites after this patch corrupt images. A fatwrite followed by a
fatload and compare yields different data.
Reproduce it with:
=>fatwrite mmc 0 0x82000000 test_32M 0x2000000;
=>fatload mmc 0 0x84000000 test_32M;
=>cmp.b 82000000 84000000 0x2000000
Result:
byte at 0x821260b2 (0xbf) != byte at 0x841260b2 (0xbd)
Total of 1204402 byte(s) were the same
Updating images in the SD card with fatwrite corrupts the images in the
board and it no longer boots. Revert this commit until a more stable
solution is found.
Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
fs/fat/fat_write.c | 290 +++------------------------------------------
1 file changed, 16 insertions(+), 274 deletions(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 852f874e58..477f68a2cc 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -457,121 +457,6 @@ set_cluster(fsdata *mydata, u32 clustnum, u8 *buffer, u32 size)
return 0;
}
-static __u8 tmpbuf_cluster[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);
-
-/*
- * Read and modify data on existing and consecutive cluster blocks
- */
-static int
-get_set_cluster(fsdata *mydata, __u32 clustnum, loff_t pos, __u8 *buffer,
- loff_t size, loff_t *gotsize)
-{
- unsigned int bytesperclust = mydata->clust_size * mydata->sect_size;
- __u32 startsect;
- loff_t wsize;
- int clustcount, i, ret;
-
- *gotsize = 0;
- if (!size)
- return 0;
-
- assert(pos < bytesperclust);
- startsect = clust_to_sect(mydata, clustnum);
-
- debug("clustnum: %d, startsect: %d, pos: %lld\n",
- clustnum, startsect, pos);
-
- /* partial write@beginning */
- if (pos) {
- wsize = min(bytesperclust - pos, size);
- ret = disk_read(startsect, mydata->clust_size, tmpbuf_cluster);
- if (ret != mydata->clust_size) {
- debug("Error reading data (got %d)\n", ret);
- return -1;
- }
-
- memcpy(tmpbuf_cluster + pos, buffer, wsize);
- ret = disk_write(startsect, mydata->clust_size, tmpbuf_cluster);
- if (ret != mydata->clust_size) {
- debug("Error writing data (got %d)\n", ret);
- return -1;
- }
-
- size -= wsize;
- buffer += wsize;
- *gotsize += wsize;
-
- startsect += mydata->clust_size;
-
- if (!size)
- return 0;
- }
-
- /* full-cluster write */
- if (size >= bytesperclust) {
- clustcount = lldiv(size, bytesperclust);
-
- if (!((unsigned long)buffer & (ARCH_DMA_MINALIGN - 1))) {
- wsize = clustcount * bytesperclust;
- ret = disk_write(startsect,
- clustcount * mydata->clust_size,
- buffer);
- if (ret != clustcount * mydata->clust_size) {
- debug("Error writing data (got %d)\n", ret);
- return -1;
- }
-
- size -= wsize;
- buffer += wsize;
- *gotsize += wsize;
-
- startsect += clustcount * mydata->clust_size;
- } else {
- for (i = 0; i < clustcount; i++) {
- memcpy(tmpbuf_cluster, buffer, bytesperclust);
- ret = disk_write(startsect,
- mydata->clust_size,
- tmpbuf_cluster);
- if (ret != mydata->clust_size) {
- debug("Error writing data (got %d)\n",
- ret);
- return -1;
- }
-
- size -= bytesperclust;
- buffer += bytesperclust;
- *gotsize += bytesperclust;
-
- startsect += mydata->clust_size;
- }
- }
- }
-
- /* partial write at end */
- if (size) {
- wsize = size;
- ret = disk_read(startsect, mydata->clust_size, tmpbuf_cluster);
- if (ret != mydata->clust_size) {
- debug("Error reading data (got %d)\n", ret);
- return -1;
- }
- memcpy(tmpbuf_cluster, buffer, wsize);
- ret = disk_write(startsect, mydata->clust_size, tmpbuf_cluster);
- if (ret != mydata->clust_size) {
- debug("Error writing data (got %d)\n", ret);
- return -1;
- }
-
- size -= wsize;
- buffer += wsize;
- *gotsize += wsize;
- }
-
- assert(!size);
-
- return 0;
-}
-
/*
* Find the first empty cluster
*/
@@ -696,162 +581,30 @@ static int
set_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos, __u8 *buffer,
loff_t maxsize, loff_t *gotsize)
{
+ loff_t filesize;
unsigned int bytesperclust = mydata->clust_size * mydata->sect_size;
__u32 curclust = START(dentptr);
__u32 endclust = 0, newclust = 0;
- u64 cur_pos, filesize;
- loff_t offset, actsize, wsize;
+ loff_t actsize;
*gotsize = 0;
- filesize = pos + maxsize;
+ filesize = maxsize;
debug("%llu bytes\n", filesize);
- if (!filesize) {
- if (!curclust)
- return 0;
- if (!CHECK_CLUST(curclust, mydata->fatsize) ||
- IS_LAST_CLUST(curclust, mydata->fatsize)) {
- clear_fatent(mydata, curclust);
- set_start_cluster(mydata, dentptr, 0);
- return 0;
- }
- debug("curclust: 0x%x\n", curclust);
- debug("Invalid FAT entry\n");
- return -1;
- }
-
- if (!curclust) {
- assert(pos == 0);
- goto set_clusters;
- }
-
- /* go to cluster at pos */
- cur_pos = bytesperclust;
- while (1) {
- if (pos <= cur_pos)
- break;
- if (IS_LAST_CLUST(curclust, mydata->fatsize))
- break;
-
- newclust = get_fatent(mydata, curclust);
- if (!IS_LAST_CLUST(newclust, mydata->fatsize) &&
- CHECK_CLUST(newclust, mydata->fatsize)) {
- debug("curclust: 0x%x\n", curclust);
- debug("Invalid FAT entry\n");
- return -1;
- }
-
- cur_pos += bytesperclust;
- curclust = newclust;
- }
- if (IS_LAST_CLUST(curclust, mydata->fatsize)) {
- assert(pos == cur_pos);
- goto set_clusters;
- }
-
- assert(pos < cur_pos);
- cur_pos -= bytesperclust;
-
- /* overwrite */
- assert(IS_LAST_CLUST(curclust, mydata->fatsize) ||
- !CHECK_CLUST(curclust, mydata->fatsize));
-
- while (1) {
- /* search for allocated consecutive clusters */
- actsize = bytesperclust;
- endclust = curclust;
- while (1) {
- if (filesize <= (cur_pos + actsize))
- break;
-
- newclust = get_fatent(mydata, endclust);
-
- if (IS_LAST_CLUST(newclust, mydata->fatsize))
- break;
- if (CHECK_CLUST(newclust, mydata->fatsize)) {
- debug("curclust: 0x%x\n", curclust);
- debug("Invalid FAT entry\n");
- return -1;
- }
-
- actsize += bytesperclust;
- endclust = newclust;
- }
-
- /* overwrite to <curclust..endclust> */
- if (pos < cur_pos)
- offset = 0;
- else
- offset = pos - cur_pos;
- wsize = min(cur_pos + actsize, filesize) - pos;
- if (get_set_cluster(mydata, curclust, offset,
- buffer, wsize, &actsize)) {
- printf("Error get-and-setting cluster\n");
+ if (curclust) {
+ /*
+ * release already-allocated clusters anyway
+ */
+ if (clear_fatent(mydata, curclust)) {
+ printf("Error: clearing FAT entries\n");
return -1;
}
- buffer += wsize;
- *gotsize += wsize;
- cur_pos += offset + wsize;
-
- if (filesize <= cur_pos)
- break;
-
- /* CHECK: newclust = get_fatent(mydata, endclust); */
-
- if (IS_LAST_CLUST(newclust, mydata->fatsize))
- /* no more clusters */
- break;
-
- curclust = newclust;
}
- if (filesize <= cur_pos) {
- /* no more write */
- newclust = get_fatent(mydata, endclust);
- if (!IS_LAST_CLUST(newclust, mydata->fatsize)) {
- /* truncate the rest */
- clear_fatent(mydata, newclust);
-
- /* Mark end of file in FAT */
- if (mydata->fatsize == 12)
- newclust = 0xfff;
- else if (mydata->fatsize == 16)
- newclust = 0xffff;
- else if (mydata->fatsize == 32)
- newclust = 0xfffffff;
- set_fatent_value(mydata, endclust, newclust);
- }
+ curclust = find_empty_cluster(mydata);
+ set_start_cluster(mydata, dentptr, curclust);
- return 0;
- }
-
- curclust = endclust;
- filesize -= cur_pos;
- assert(!do_div(cur_pos, bytesperclust));
-
-set_clusters:
- /* allocate and write */
- assert(!pos);
-
- /* Assure that curclust is valid */
- if (!curclust) {
- curclust = find_empty_cluster(mydata);
- set_start_cluster(mydata, dentptr, curclust);
- } else {
- newclust = get_fatent(mydata, curclust);
-
- if (IS_LAST_CLUST(newclust, mydata->fatsize)) {
- newclust = determine_fatent(mydata, curclust);
- set_fatent_value(mydata, curclust, newclust);
- curclust = newclust;
- } else {
- debug("error: something wrong\n");
- return -1;
- }
- }
-
- /* TODO: already partially written */
if (check_overflow(mydata, curclust, filesize)) {
printf("Error: no space left: %llu\n", filesize);
return -1;
@@ -1103,16 +856,6 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
goto exit;
}
- /* A file exists */
- if (pos == -1)
- /* Append to the end */
- pos = FAT2CPU32(retdent->size);
- if (pos > retdent->size) {
- /* No hole allowed */
- ret = -EINVAL;
- goto exit;
- }
-
/* Update file size in a directory entry */
retdent->size = cpu_to_le32(pos + size);
} else {
@@ -1133,12 +876,6 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
goto exit;
}
- if (pos) {
- /* No hole allowed */
- ret = -EINVAL;
- goto exit;
- }
-
memset(itr->dent, 0, sizeof(*itr->dent));
/* Set short name to set alias checksum field in dir_slot */
@@ -1188,6 +925,11 @@ exit:
int file_fat_write(const char *filename, void *buffer, loff_t offset,
loff_t maxsize, loff_t *actwrite)
{
+ if (offset != 0) {
+ printf("Error: non zero offset is currently not supported.\n");
+ return -EINVAL;
+ }
+
return file_fat_write_at(filename, offset, buffer, maxsize, actwrite);
}
--
2.19.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [U-Boot] [PATCH] Revert "fs: fat: support write with non-zero offset"
2019-03-14 9:20 [U-Boot] [PATCH] Revert "fs: fat: support write with non-zero offset" Faiz Abbas
@ 2019-03-14 15:35 ` Tom Rini
0 siblings, 0 replies; 2+ messages in thread
From: Tom Rini @ 2019-03-14 15:35 UTC (permalink / raw)
To: u-boot
On Thu, Mar 14, 2019 at 02:50:39PM +0530, Faiz Abbas wrote:
> This reverts commit cb8af8af5ba03ae8e0a7315b66bfcc46d5c55627.
>
> fatwrites after this patch corrupt images. A fatwrite followed by a
> fatload and compare yields different data.
>
> Reproduce it with:
> =>fatwrite mmc 0 0x82000000 test_32M 0x2000000;
> =>fatload mmc 0 0x84000000 test_32M;
> =>cmp.b 82000000 84000000 0x2000000
>
> Result:
> byte at 0x821260b2 (0xbf) != byte at 0x841260b2 (0xbd)
> Total of 1204402 byte(s) were the same
>
> Updating images in the SD card with fatwrite corrupts the images in the
> board and it no longer boots. Revert this commit until a more stable
> solution is found.
>
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
This removes offset writing support, so you need to revert the tests
too. You can see this with 'make tests' and make sure that all of
test/py/tests/test_fs/test_ext.py (yes, ugh, needs a better name) runs
and succeeds. Thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190314/279ad2ad/attachment.sig>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-03-14 15:35 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-14 9:20 [U-Boot] [PATCH] Revert "fs: fat: support write with non-zero offset" Faiz Abbas
2019-03-14 15:35 ` Tom Rini
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.