[U-Boot] [PATCH 0/4] fs: fat: fixes for write under root directory

[U-Boot] [PATCH 0/4] fs: fat: fixes for write under root directory

[AKASHI Takahiro]

This patch set contains three (independent) bug fixes relating to
write operations to root directory. See each commit message
for more details.

Without those fixes, you may lose files that you created under root
directory or will see the file system corrupted. This will happen
particularly when you create (or even delete) many files repeatedly
under root directory.
(Non-root directory won't be affected.)

I only tested my patches on limited number of file system instances,
and hope that other people will also try them.

-Takahiro Akashi

AKASHI Takahiro (4):
  fs: fat:  write to non-cluster-aligned root directory
  fs: fat: flush a directory cluster properly
  fs: fat: allocate a new cluster for root directory of fat32
  test/py: test_fs: add tests for creating/deleting many files

 fs/fat/fat.c                      |  15 ++--
 fs/fat/fat_write.c                | 117 +++++++++++++++++++-----------
 test/py/tests/test_fs/test_ext.py |  84 +++++++++++++++++++++
 3 files changed, 165 insertions(+), 51 deletions(-)

-- 
2.21.0

[U-Boot] [PATCH 1/4] fs: fat: write to non-cluster-aligned root directory

[AKASHI Takahiro]

With the commit below, fat now correctly handles a file read under
a non-cluster-aligned root directory of fat12/16.
Write operation should be fixed in the same manner.

Fixes: commit 9b18358dc05d ("fs: fat: fix reading non-cluster-aligned
       root directory")
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Anssi Hannula <anssi.hannula@bitwise.fi>
---
 fs/fat/fat.c       | 15 ++++-----
 fs/fat/fat_write.c | 78 +++++++++++++++++++++++++++++++---------------
 2 files changed, 61 insertions(+), 32 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index c5997c21735f..fccaa385d187 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -619,13 +619,14 @@ static int get_fs_info(fsdata *mydata)
 		return -1;
 	}
 
-	debug("FAT%d, fat_sect: %d, fatlength: %d\n",
-	       mydata->fatsize, mydata->fat_sect, mydata->fatlength);
-	debug("Rootdir begins at cluster: %d, sector: %d, offset: %x\n"
-	       "Data begins at: %d\n",
-	       mydata->root_cluster,
-	       mydata->rootdir_sect,
-	       mydata->rootdir_sect * mydata->sect_size, mydata->data_begin);
+	debug("FAT%d, fat_sect: %d, fatlength: %d, num: %d\n",
+	      mydata->fatsize, mydata->fat_sect, mydata->fatlength,
+	      mydata->fats);
+	debug("Rootdir begins at cluster: %d, sector: %d, size: %x\n"
+	      "Data begins at: %d\n",
+	      mydata->root_cluster,
+	      mydata->rootdir_sect,
+	      mydata->rootdir_size * mydata->sect_size, mydata->data_begin);
 	debug("Sector size: %d, cluster size: %d\n", mydata->sect_size,
 	      mydata->clust_size);
 
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 852f874e5817..3bc0dd637521 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -388,29 +388,23 @@ static __u32 determine_fatent(fsdata *mydata, __u32 entry)
 }
 
 /**
- * set_cluster() - write data to cluster
+ * set_sectors() - write data to sectors
  *
- * Write 'size' bytes from 'buffer' into the specified cluster.
+ * Write 'size' bytes from 'buffer' into the specified sector.
  *
  * @mydata:	data to be written
- * @clustnum:	cluster to be written to
+ * @startsect:	sector to be written to
  * @buffer:	data to be written
  * @size:	bytes to be written (but not more than the size of a cluster)
  * Return:	0 on success, -1 otherwise
  */
 static int
-set_cluster(fsdata *mydata, u32 clustnum, u8 *buffer, u32 size)
+set_sectors(fsdata *mydata, u32 startsect, u8 *buffer, u32 size)
 {
-	u32 idx = 0;
-	u32 startsect;
+	u32 nsects = 0;
 	int ret;
 
-	if (clustnum > 0)
-		startsect = clust_to_sect(mydata, clustnum);
-	else
-		startsect = mydata->rootdir_sect;
-
-	debug("clustnum: %d, startsect: %d\n", clustnum, startsect);
+	debug("startsect: %d\n", startsect);
 
 	if ((unsigned long)buffer & (ARCH_DMA_MINALIGN - 1)) {
 		ALLOC_CACHE_ALIGN_BUFFER(__u8, tmpbuf, mydata->sect_size);
@@ -429,17 +423,16 @@ set_cluster(fsdata *mydata, u32 clustnum, u8 *buffer, u32 size)
 			size -= mydata->sect_size;
 		}
 	} else if (size >= mydata->sect_size) {
-		idx = size / mydata->sect_size;
-		ret = disk_write(startsect, idx, buffer);
-		if (ret != idx) {
+		nsects = size / mydata->sect_size;
+		ret = disk_write(startsect, nsects, buffer);
+		if (ret != nsects) {
 			debug("Error writing data (got %d)\n", ret);
 			return -1;
 		}
 
-		startsect += idx;
-		idx *= mydata->sect_size;
-		buffer += idx;
-		size -= idx;
+		startsect += nsects;
+		buffer += nsects * mydata->sect_size;
+		size -= nsects * mydata->sect_size;
 	}
 
 	if (size) {
@@ -457,6 +450,44 @@ set_cluster(fsdata *mydata, u32 clustnum, u8 *buffer, u32 size)
 	return 0;
 }
 
+/**
+ * set_cluster() - write data to cluster
+ *
+ * Write 'size' bytes from 'buffer' into the specified cluster.
+ *
+ * @mydata:	data to be written
+ * @clustnum:	cluster to be written to
+ * @buffer:	data to be written
+ * @size:	bytes to be written (but not more than the size of a cluster)
+ * Return:	0 on success, -1 otherwise
+ */
+static int
+set_cluster(fsdata *mydata, u32 clustnum, u8 *buffer, u32 size)
+{
+	return set_sectors(mydata, clust_to_sect(mydata, clustnum),
+			   buffer, size);
+}
+
+static int
+flush_dir(fat_itr *itr)
+{
+	fsdata *mydata = itr->fsdata;
+	u32 startsect, sect_offset, nsects;
+
+	if (!itr->is_root || mydata->fatsize == 32)
+		return set_cluster(mydata, itr->clust, itr->block,
+				   mydata->clust_size * mydata->sect_size);
+
+	sect_offset = itr->clust * mydata->clust_size;
+	startsect = mydata->rootdir_sect + sect_offset;
+	/* do not write past the end of rootdir */
+	nsects = min_t(u32, mydata->clust_size,
+		       mydata->rootdir_size - sect_offset);
+
+	return set_sectors(mydata, startsect, itr->block,
+			   nsects * mydata->sect_size);
+}
+
 static __u8 tmpbuf_cluster[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);
 
 /*
@@ -1171,8 +1202,7 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
 	}
 
 	/* Write directory table to device */
-	ret = set_cluster(mydata, itr->clust, itr->block,
-			  mydata->clust_size * mydata->sect_size);
+	ret = flush_dir(itr);
 	if (ret) {
 		printf("Error: writing directory entry\n");
 		ret = -EIO;
@@ -1249,8 +1279,7 @@ static int delete_dentry(fat_itr *itr)
 	memset(dentptr, 0, sizeof(*dentptr));
 	dentptr->name[0] = 0xe5;
 
-	if (set_cluster(mydata, itr->clust, itr->block,
-			mydata->clust_size * mydata->sect_size) != 0) {
+	if (flush_dir(itr)) {
 		printf("error: writing directory entry\n");
 		return -EIO;
 	}
@@ -1452,8 +1481,7 @@ int fat_mkdir(const char *new_dirname)
 	}
 
 	/* Write directory table to device */
-	ret = set_cluster(mydata, itr->clust, itr->block,
-			  mydata->clust_size * mydata->sect_size);
+	ret = flush_dir(itr);
 	if (ret)
 		printf("Error: writing directory entry\n");
 
-- 
2.21.0

[U-Boot] [PATCH 1/4] fs: fat: write to non-cluster-aligned root directory

[Heinrich Schuchardt]

On 5/13/19 7:49 AM, AKASHI Takahiro wrote:
> With the commit below, fat now correctly handles a file read under
> a non-cluster-aligned root directory of fat12/16.
> Write operation should be fixed in the same manner.
>
> Fixes: commit 9b18358dc05d ("fs: fat: fix reading non-cluster-aligned
>         root directory")
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Cc: Anssi Hannula <anssi.hannula@bitwise.fi>

Thanks a lot for addressing this.

> ---
>   fs/fat/fat.c       | 15 ++++-----
>   fs/fat/fat_write.c | 78 +++++++++++++++++++++++++++++++---------------
>   2 files changed, 61 insertions(+), 32 deletions(-)
>
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index c5997c21735f..fccaa385d187 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -619,13 +619,14 @@ static int get_fs_info(fsdata *mydata)
>   		return -1;
>   	}
>
> -	debug("FAT%d, fat_sect: %d, fatlength: %d\n",
> -	       mydata->fatsize, mydata->fat_sect, mydata->fatlength);
> -	debug("Rootdir begins at cluster: %d, sector: %d, offset: %x\n"
> -	       "Data begins at: %d\n",
> -	       mydata->root_cluster,
> -	       mydata->rootdir_sect,
> -	       mydata->rootdir_sect * mydata->sect_size, mydata->data_begin);
> +	debug("FAT%d, fat_sect: %d, fatlength: %d, num: %d\n",
> +	      mydata->fatsize, mydata->fat_sect, mydata->fatlength,
> +	      mydata->fats);
> +	debug("Rootdir begins at cluster: %d, sector: %d, size: %x\n"
> +	      "Data begins at: %d\n",
> +	      mydata->root_cluster,
> +	      mydata->rootdir_sect,
> +	      mydata->rootdir_size * mydata->sect_size, mydata->data_begin);

This seems to be an unrelated change. It should be either in a separate
patch or the commit message should explain why it is related.

Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

>   	debug("Sector size: %d, cluster size: %d\n", mydata->sect_size,
>   	      mydata->clust_size);
>
> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
> index 852f874e5817..3bc0dd637521 100644
> --- a/fs/fat/fat_write.c
> +++ b/fs/fat/fat_write.c
> @@ -388,29 +388,23 @@ static __u32 determine_fatent(fsdata *mydata, __u32 entry)
>   }
>
>   /**
> - * set_cluster() - write data to cluster
> + * set_sectors() - write data to sectors
>    *
> - * Write 'size' bytes from 'buffer' into the specified cluster.
> + * Write 'size' bytes from 'buffer' into the specified sector.
>    *
>    * @mydata:	data to be written
> - * @clustnum:	cluster to be written to
> + * @startsect:	sector to be written to
>    * @buffer:	data to be written
>    * @size:	bytes to be written (but not more than the size of a cluster)
>    * Return:	0 on success, -1 otherwise
>    */
>   static int
> -set_cluster(fsdata *mydata, u32 clustnum, u8 *buffer, u32 size)
> +set_sectors(fsdata *mydata, u32 startsect, u8 *buffer, u32 size)
>   {
> -	u32 idx = 0;
> -	u32 startsect;
> +	u32 nsects = 0;
>   	int ret;
>
> -	if (clustnum > 0)
> -		startsect = clust_to_sect(mydata, clustnum);
> -	else
> -		startsect = mydata->rootdir_sect;
> -
> -	debug("clustnum: %d, startsect: %d\n", clustnum, startsect);
> +	debug("startsect: %d\n", startsect);
>
>   	if ((unsigned long)buffer & (ARCH_DMA_MINALIGN - 1)) {
>   		ALLOC_CACHE_ALIGN_BUFFER(__u8, tmpbuf, mydata->sect_size);
> @@ -429,17 +423,16 @@ set_cluster(fsdata *mydata, u32 clustnum, u8 *buffer, u32 size)
>   			size -= mydata->sect_size;
>   		}
>   	} else if (size >= mydata->sect_size) {
> -		idx = size / mydata->sect_size;
> -		ret = disk_write(startsect, idx, buffer);
> -		if (ret != idx) {
> +		nsects = size / mydata->sect_size;
> +		ret = disk_write(startsect, nsects, buffer);
> +		if (ret != nsects) {
>   			debug("Error writing data (got %d)\n", ret);
>   			return -1;
>   		}
>
> -		startsect += idx;
> -		idx *= mydata->sect_size;
> -		buffer += idx;
> -		size -= idx;
> +		startsect += nsects;
> +		buffer += nsects * mydata->sect_size;
> +		size -= nsects * mydata->sect_size;
>   	}
>
>   	if (size) {
> @@ -457,6 +450,44 @@ set_cluster(fsdata *mydata, u32 clustnum, u8 *buffer, u32 size)
>   	return 0;
>   }
>
> +/**
> + * set_cluster() - write data to cluster
> + *
> + * Write 'size' bytes from 'buffer' into the specified cluster.
> + *
> + * @mydata:	data to be written
> + * @clustnum:	cluster to be written to
> + * @buffer:	data to be written
> + * @size:	bytes to be written (but not more than the size of a cluster)
> + * Return:	0 on success, -1 otherwise
> + */
> +static int
> +set_cluster(fsdata *mydata, u32 clustnum, u8 *buffer, u32 size)
> +{
> +	return set_sectors(mydata, clust_to_sect(mydata, clustnum),
> +			   buffer, size);
> +}
> +
> +static int
> +flush_dir(fat_itr *itr)
> +{
> +	fsdata *mydata = itr->fsdata;
> +	u32 startsect, sect_offset, nsects;
> +
> +	if (!itr->is_root || mydata->fatsize == 32)
> +		return set_cluster(mydata, itr->clust, itr->block,
> +				   mydata->clust_size * mydata->sect_size);
> +
> +	sect_offset = itr->clust * mydata->clust_size;
> +	startsect = mydata->rootdir_sect + sect_offset;
> +	/* do not write past the end of rootdir */
> +	nsects = min_t(u32, mydata->clust_size,
> +		       mydata->rootdir_size - sect_offset);
> +
> +	return set_sectors(mydata, startsect, itr->block,
> +			   nsects * mydata->sect_size);
> +}
> +
>   static __u8 tmpbuf_cluster[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);
>
>   /*
> @@ -1171,8 +1202,7 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
>   	}
>
>   	/* Write directory table to device */
> -	ret = set_cluster(mydata, itr->clust, itr->block,
> -			  mydata->clust_size * mydata->sect_size);
> +	ret = flush_dir(itr);
>   	if (ret) {
>   		printf("Error: writing directory entry\n");
>   		ret = -EIO;
> @@ -1249,8 +1279,7 @@ static int delete_dentry(fat_itr *itr)
>   	memset(dentptr, 0, sizeof(*dentptr));
>   	dentptr->name[0] = 0xe5;
>
> -	if (set_cluster(mydata, itr->clust, itr->block,
> -			mydata->clust_size * mydata->sect_size) != 0) {
> +	if (flush_dir(itr)) {
>   		printf("error: writing directory entry\n");
>   		return -EIO;
>   	}
> @@ -1452,8 +1481,7 @@ int fat_mkdir(const char *new_dirname)
>   	}
>
>   	/* Write directory table to device */
> -	ret = set_cluster(mydata, itr->clust, itr->block,
> -			  mydata->clust_size * mydata->sect_size);
> +	ret = flush_dir(itr);
>   	if (ret)
>   		printf("Error: writing directory entry\n");
>
>

[U-Boot] [PATCH 2/4] fs: fat: flush a directory cluster properly

[AKASHI Takahiro]

When a long name directory entry is created, multiple directory entries
may be occupied across a directory cluster boundary. Since only one
directory cluster is cached in a directory iterator, a first cluster must
be written back to device before switching over a second cluster.

Without this patch, some added files may be lost even if you don't see
any failures on write operation.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fat/fat_write.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 3bc0dd637521..da1753d545ac 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -209,7 +209,8 @@ name11_12:
 	return 1;
 }
 
-static int flush_dir_table(fat_itr *itr);
+static int new_dir_table(fat_itr *itr);
+static int flush_dir(fat_itr *itr);
 
 /*
  * Fill dir_slot entries with appropriate name, id, and attr
@@ -242,19 +243,15 @@ fill_dir_slot(fat_itr *itr, const char *l_name)
 		memcpy(itr->dent, slotptr, sizeof(dir_slot));
 		slotptr--;
 		counter--;
+
+		if (itr->remaining == 0)
+			flush_dir(itr);
+
 		if (!fat_itr_next(itr))
-			if (!itr->dent && !itr->is_root && flush_dir_table(itr))
+			if (!itr->dent && !itr->is_root && new_dir_table(itr))
 				return -1;
 	}
 
-	if (!itr->dent && !itr->is_root)
-		/*
-		 * don't care return value here because we have already
-		 * finished completing an entry with name, only ending up
-		 * no more entry left
-		 */
-		flush_dir_table(itr);
-
 	return 0;
 }
 
@@ -621,18 +618,14 @@ static int find_empty_cluster(fsdata *mydata)
 }
 
 /*
- * Write directory entries in itr's buffer to block device
+ * Allocate a cluster for additional directory entries
  */
-static int flush_dir_table(fat_itr *itr)
+static int new_dir_table(fat_itr *itr)
 {
 	fsdata *mydata = itr->fsdata;
 	int dir_newclust = 0;
 	unsigned int bytesperclust = mydata->clust_size * mydata->sect_size;
 
-	if (set_cluster(mydata, itr->clust, itr->block, bytesperclust) != 0) {
-		printf("error: writing directory entry\n");
-		return -1;
-	}
 	dir_newclust = find_empty_cluster(mydata);
 	set_fatent_value(mydata, itr->clust, dir_newclust);
 	if (mydata->fatsize == 32)
@@ -987,7 +980,7 @@ static dir_entry *find_directory_entry(fat_itr *itr, char *filename)
 			return itr->dent;
 	}
 
-	if (!itr->dent && !itr->is_root && flush_dir_table(itr))
+	if (!itr->dent && !itr->is_root && new_dir_table(itr))
 		/* indicate that allocating dent failed */
 		itr->dent = NULL;
 
@@ -1172,14 +1165,16 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
 
 		memset(itr->dent, 0, sizeof(*itr->dent));
 
-		/* Set short name to set alias checksum field in dir_slot */
+		/* Calculate checksum for short name */
 		set_name(itr->dent, filename);
+
+		/* Set long name entries */
 		if (fill_dir_slot(itr, filename)) {
 			ret = -EIO;
 			goto exit;
 		}
 
-		/* Set attribute as archive for regular file */
+		/* Set short name entry */
 		fill_dentry(itr->fsdata, itr->dent, filename, 0, size, 0x20);
 
 		retdent = itr->dent;
-- 
2.21.0

[U-Boot] [PATCH 2/4] fs: fat: flush a directory cluster properly

[Heinrich Schuchardt]

On 5/13/19 7:49 AM, AKASHI Takahiro wrote:
> When a long name directory entry is created, multiple directory entries
> may be occupied across a directory cluster boundary. Since only one
> directory cluster is cached in a directory iterator, a first cluster must
> be written back to device before switching over a second cluster.
>
> Without this patch, some added files may be lost even if you don't see
> any failures on write operation.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

[U-Boot] [PATCH 3/4] fs: fat: allocate a new cluster for root directory of fat32

[AKASHI Takahiro]

Contrary to fat12/16, fat32 can have root directory at any location
and its size can be expanded.
Without this patch, root directory won't grow properly and so we will
eventually fail to add files under root directory. Please note that this
can happen even if you delete many files as deleted directory entries
are not reclaimed but just marked as "deleted" under the current
implementation.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fat/fat_write.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index da1753d545ac..3bb9eac097eb 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -247,8 +247,11 @@ fill_dir_slot(fat_itr *itr, const char *l_name)
 		if (itr->remaining == 0)
 			flush_dir(itr);
 
+		/* allocate a cluster for more entries */
 		if (!fat_itr_next(itr))
-			if (!itr->dent && !itr->is_root && new_dir_table(itr))
+			if (!itr->dent &&
+			    (!itr->is_root || itr->fsdata->fatsize == 32) &&
+			    new_dir_table(itr))
 				return -1;
 	}
 
@@ -980,7 +983,10 @@ static dir_entry *find_directory_entry(fat_itr *itr, char *filename)
 			return itr->dent;
 	}
 
-	if (!itr->dent && !itr->is_root && new_dir_table(itr))
+	/* allocate a cluster for more entries */
+	if (!itr->dent &&
+	    (!itr->is_root || itr->fsdata->fatsize == 32) &&
+	    new_dir_table(itr))
 		/* indicate that allocating dent failed */
 		itr->dent = NULL;
 
-- 
2.21.0

[U-Boot] [PATCH 3/4] fs: fat: allocate a new cluster for root directory of fat32

[Heinrich Schuchardt]

On 5/13/19 7:49 AM, AKASHI Takahiro wrote:
> Contrary to fat12/16, fat32 can have root directory at any location
> and its size can be expanded.
> Without this patch, root directory won't grow properly and so we will
> eventually fail to add files under root directory. Please note that this
> can happen even if you delete many files as deleted directory entries
> are not reclaimed but just marked as "deleted" under the current
> implementation.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

[U-Boot] [PATCH 4/4] test/py: test_fs: add tests for creating/deleting many files

[AKASHI Takahiro]

Two test cases are added under test_fs_ext:
    test case 10: for root directory
    test case 11: for non-root directory

Those will verify a behavior fixed by the commits related to
root directory
("fs: fat: allocate a new cluster for root directory of fat32" and
"fs: fat: flush a directory cluster properly").

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 test/py/tests/test_fs/test_ext.py | 84 +++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/test/py/tests/test_fs/test_ext.py b/test/py/tests/test_fs/test_ext.py
index 2c47738b8df2..361f440dd6d4 100644
--- a/test/py/tests/test_fs/test_ext.py
+++ b/test/py/tests/test_fs/test_ext.py
@@ -233,3 +233,87 @@ class TestFsExt(object):
                     % (fs_type, ADDR, MIN_FILE)])
             assert('Unable to write "/dir1' in ''.join(output))
             assert_fs_integrity(fs_type, fs_img)
+
+    def test_fs_ext10(self, u_boot_console, fs_obj_ext):
+        """
+        'Test Case 10 - create/delete as many directories under root directory
+        as amount of directory entries goes beyond one cluster size)'
+        """
+        fs_type,fs_img,md5val = fs_obj_ext
+        with u_boot_console.log.section('Test Case 10 - create/delete (many)'):
+            # Test Case 10a - Create many files
+            #   Please note that the size of directory entry is 32 bytes.
+            #   So one typical cluster may holds 64 (2048/32) entries.
+            output = u_boot_console.run_command(
+                'host bind 0 %s' % fs_img)
+
+            for i in range(0, 66):
+                output = u_boot_console.run_command(
+                    '%swrite host 0:0 %x /FILE0123456789_%02x 100'
+                    % (fs_type, ADDR, i))
+            output = u_boot_console.run_command('%sls host 0:0 /' % fs_type)
+            assert('FILE0123456789_00' in output)
+            assert('FILE0123456789_41' in output)
+
+            # Test Case 10b - Delete many files
+            for i in range(0, 66):
+                output = u_boot_console.run_command(
+                    '%srm host 0:0 /FILE0123456789_%02x'
+                    % (fs_type, i))
+            output = u_boot_console.run_command('%sls host 0:0 /' % fs_type)
+            assert(not 'FILE0123456789_00' in output)
+            assert(not 'FILE0123456789_41' in output)
+
+            # Test Case 10c - Create many files again
+            # Please note no.64 and 65 are intentionally re-created
+            for i in range(64, 128):
+                output = u_boot_console.run_command(
+                    '%swrite host 0:0 %x /FILE0123456789_%02x 100'
+                    % (fs_type, ADDR, i))
+            output = u_boot_console.run_command('%sls host 0:0 /' % fs_type)
+            assert('FILE0123456789_40' in output)
+            assert('FILE0123456789_79' in output)
+
+            assert_fs_integrity(fs_type, fs_img)
+
+    def test_fs_ext11(self, u_boot_console, fs_obj_ext):
+        """
+        'Test Case 11 - create/delete as many directories under non-root
+        directory as amount of directory entries goes beyond one cluster size)'
+        """
+        fs_type,fs_img,md5val = fs_obj_ext
+        with u_boot_console.log.section('Test Case 10 - create/delete (many)'):
+            # Test Case 11a - Create many files
+            #   Please note that the size of directory entry is 32 bytes.
+            #   So one typical cluster may holds 64 (2048/32) entries.
+            output = u_boot_console.run_command(
+                'host bind 0 %s' % fs_img)
+
+            for i in range(0, 66):
+                output = u_boot_console.run_command(
+                    '%swrite host 0:0 %x /dir1/FILE0123456789_%02x 100'
+                    % (fs_type, ADDR, i))
+            output = u_boot_console.run_command('%sls host 0:0 /dir1' % fs_type)
+            assert('FILE0123456789_00' in output)
+            assert('FILE0123456789_41' in output)
+
+            # Test Case 11b - Delete many files
+            for i in range(0, 66):
+                output = u_boot_console.run_command(
+                    '%srm host 0:0 /dir1/FILE0123456789_%02x'
+                    % (fs_type, i))
+            output = u_boot_console.run_command('%sls host 0:0 /dir1' % fs_type)
+            assert(not 'FILE0123456789_00' in output)
+            assert(not 'FILE0123456789_41' in output)
+
+            # Test Case 11c - Create many files again
+            # Please note no.64 and 65 are intentionally re-created
+            for i in range(64, 128):
+                output = u_boot_console.run_command(
+                    '%swrite host 0:0 %x /dir1/FILE0123456789_%02x 100'
+                    % (fs_type, ADDR, i))
+            output = u_boot_console.run_command('%sls host 0:0 /dir1' % fs_type)
+            assert('FILE0123456789_40' in output)
+            assert('FILE0123456789_79' in output)
+
+            assert_fs_integrity(fs_type, fs_img)
-- 
2.21.0

[U-Boot] [PATCH 4/4] test/py: test_fs: add tests for creating/deleting many files

[Heinrich Schuchardt]

On 5/13/19 7:49 AM, AKASHI Takahiro wrote:
> Two test cases are added under test_fs_ext:
>      test case 10: for root directory
>      test case 11: for non-root directory

When running `make tests` all of these tests seem to be skipped:

test/py/tests/test_fs/test_basic.py
sssssssssssssssssssssssssssssssssssssss
                                                         [  0%]
test/py/tests/test_fs/test_ext.py ssssssssssssssssssssss

                     [  0%]
test/py/tests/test_fs/test_mkdir.py ssssssssssss

                     [  0%]
test/py/tests/test_fs/test_symlink.py ssss

                     [  0%]
test/py/tests/test_fs/test_unlink.py ssssssssssssss

How can they be executed?

I think it is worthwhile to mention as a comment that you are testing
long file names (not 8.3).

Best regards

Heinrich

>
> Those will verify a behavior fixed by the commits related to
> root directory
> ("fs: fat: allocate a new cluster for root directory of fat32" and
> "fs: fat: flush a directory cluster properly").
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   test/py/tests/test_fs/test_ext.py | 84 +++++++++++++++++++++++++++++++
>   1 file changed, 84 insertions(+)
>
> diff --git a/test/py/tests/test_fs/test_ext.py b/test/py/tests/test_fs/test_ext.py
> index 2c47738b8df2..361f440dd6d4 100644
> --- a/test/py/tests/test_fs/test_ext.py
> +++ b/test/py/tests/test_fs/test_ext.py
> @@ -233,3 +233,87 @@ class TestFsExt(object):
>                       % (fs_type, ADDR, MIN_FILE)])
>               assert('Unable to write "/dir1' in ''.join(output))
>               assert_fs_integrity(fs_type, fs_img)
> +
> +    def test_fs_ext10(self, u_boot_console, fs_obj_ext):
> +        """
> +        'Test Case 10 - create/delete as many directories under root directory
> +        as amount of directory entries goes beyond one cluster size)'
> +        """
> +        fs_type,fs_img,md5val = fs_obj_ext
> +        with u_boot_console.log.section('Test Case 10 - create/delete (many)'):
> +            # Test Case 10a - Create many files
> +            #   Please note that the size of directory entry is 32 bytes.
> +            #   So one typical cluster may holds 64 (2048/32) entries.
> +            output = u_boot_console.run_command(
> +                'host bind 0 %s' % fs_img)
> +
> +            for i in range(0, 66):
> +                output = u_boot_console.run_command(
> +                    '%swrite host 0:0 %x /FILE0123456789_%02x 100'
> +                    % (fs_type, ADDR, i))
> +            output = u_boot_console.run_command('%sls host 0:0 /' % fs_type)
> +            assert('FILE0123456789_00' in output)
> +            assert('FILE0123456789_41' in output)
> +
> +            # Test Case 10b - Delete many files
> +            for i in range(0, 66):
> +                output = u_boot_console.run_command(
> +                    '%srm host 0:0 /FILE0123456789_%02x'
> +                    % (fs_type, i))
> +            output = u_boot_console.run_command('%sls host 0:0 /' % fs_type)
> +            assert(not 'FILE0123456789_00' in output)
> +            assert(not 'FILE0123456789_41' in output)
> +
> +            # Test Case 10c - Create many files again
> +            # Please note no.64 and 65 are intentionally re-created
> +            for i in range(64, 128):
> +                output = u_boot_console.run_command(
> +                    '%swrite host 0:0 %x /FILE0123456789_%02x 100'
> +                    % (fs_type, ADDR, i))
> +            output = u_boot_console.run_command('%sls host 0:0 /' % fs_type)
> +            assert('FILE0123456789_40' in output)
> +            assert('FILE0123456789_79' in output)
> +
> +            assert_fs_integrity(fs_type, fs_img)
> +
> +    def test_fs_ext11(self, u_boot_console, fs_obj_ext):
> +        """
> +        'Test Case 11 - create/delete as many directories under non-root
> +        directory as amount of directory entries goes beyond one cluster size)'
> +        """
> +        fs_type,fs_img,md5val = fs_obj_ext
> +        with u_boot_console.log.section('Test Case 10 - create/delete (many)'):
> +            # Test Case 11a - Create many files
> +            #   Please note that the size of directory entry is 32 bytes.
> +            #   So one typical cluster may holds 64 (2048/32) entries.
> +            output = u_boot_console.run_command(
> +                'host bind 0 %s' % fs_img)
> +
> +            for i in range(0, 66):
> +                output = u_boot_console.run_command(
> +                    '%swrite host 0:0 %x /dir1/FILE0123456789_%02x 100'
> +                    % (fs_type, ADDR, i))
> +            output = u_boot_console.run_command('%sls host 0:0 /dir1' % fs_type)
> +            assert('FILE0123456789_00' in output)
> +            assert('FILE0123456789_41' in output)
> +
> +            # Test Case 11b - Delete many files
> +            for i in range(0, 66):
> +                output = u_boot_console.run_command(
> +                    '%srm host 0:0 /dir1/FILE0123456789_%02x'
> +                    % (fs_type, i))
> +            output = u_boot_console.run_command('%sls host 0:0 /dir1' % fs_type)
> +            assert(not 'FILE0123456789_00' in output)
> +            assert(not 'FILE0123456789_41' in output)
> +
> +            # Test Case 11c - Create many files again
> +            # Please note no.64 and 65 are intentionally re-created
> +            for i in range(64, 128):
> +                output = u_boot_console.run_command(
> +                    '%swrite host 0:0 %x /dir1/FILE0123456789_%02x 100'
> +                    % (fs_type, ADDR, i))
> +            output = u_boot_console.run_command('%sls host 0:0 /dir1' % fs_type)
> +            assert('FILE0123456789_40' in output)
> +            assert('FILE0123456789_79' in output)
> +
> +            assert_fs_integrity(fs_type, fs_img)
>

[U-Boot] [PATCH 4/4] test/py: test_fs: add tests for creating/deleting many files

[AKASHI Takahiro]

On Tue, May 14, 2019 at 08:09:53PM +0200, Heinrich Schuchardt wrote:
> On 5/13/19 7:49 AM, AKASHI Takahiro wrote:
> >Two test cases are added under test_fs_ext:
> >     test case 10: for root directory
> >     test case 11: for non-root directory
> 
> When running `make tests` all of these tests seem to be skipped:
> 
> test/py/tests/test_fs/test_basic.py
> sssssssssssssssssssssssssssssssssssssss
>                                                         [  0%]
> test/py/tests/test_fs/test_ext.py ssssssssssssssssssssss
> 
>                     [  0%]
> test/py/tests/test_fs/test_mkdir.py ssssssssssss
> 
>                     [  0%]
> test/py/tests/test_fs/test_symlink.py ssss
> 
>                     [  0%]
> test/py/tests/test_fs/test_unlink.py ssssssssssssss
> 
> How can they be executed?

Install guestmount and set it up correctly.

-Takahiro Akashi


> I think it is worthwhile to mention as a comment that you are testing
> long file names (not 8.3).
> 
> Best regards
> 
> Heinrich
> 
> >
> >Those will verify a behavior fixed by the commits related to
> >root directory
> >("fs: fat: allocate a new cluster for root directory of fat32" and
> >"fs: fat: flush a directory cluster properly").
> >
> >Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >---
> >  test/py/tests/test_fs/test_ext.py | 84 +++++++++++++++++++++++++++++++
> >  1 file changed, 84 insertions(+)
> >
> >diff --git a/test/py/tests/test_fs/test_ext.py b/test/py/tests/test_fs/test_ext.py
> >index 2c47738b8df2..361f440dd6d4 100644
> >--- a/test/py/tests/test_fs/test_ext.py
> >+++ b/test/py/tests/test_fs/test_ext.py
> >@@ -233,3 +233,87 @@ class TestFsExt(object):
> >                      % (fs_type, ADDR, MIN_FILE)])
> >              assert('Unable to write "/dir1' in ''.join(output))
> >              assert_fs_integrity(fs_type, fs_img)
> >+
> >+    def test_fs_ext10(self, u_boot_console, fs_obj_ext):
> >+        """
> >+        'Test Case 10 - create/delete as many directories under root directory
> >+        as amount of directory entries goes beyond one cluster size)'
> >+        """
> >+        fs_type,fs_img,md5val = fs_obj_ext
> >+        with u_boot_console.log.section('Test Case 10 - create/delete (many)'):
> >+            # Test Case 10a - Create many files
> >+            #   Please note that the size of directory entry is 32 bytes.
> >+            #   So one typical cluster may holds 64 (2048/32) entries.
> >+            output = u_boot_console.run_command(
> >+                'host bind 0 %s' % fs_img)
> >+
> >+            for i in range(0, 66):
> >+                output = u_boot_console.run_command(
> >+                    '%swrite host 0:0 %x /FILE0123456789_%02x 100'
> >+                    % (fs_type, ADDR, i))
> >+            output = u_boot_console.run_command('%sls host 0:0 /' % fs_type)
> >+            assert('FILE0123456789_00' in output)
> >+            assert('FILE0123456789_41' in output)
> >+
> >+            # Test Case 10b - Delete many files
> >+            for i in range(0, 66):
> >+                output = u_boot_console.run_command(
> >+                    '%srm host 0:0 /FILE0123456789_%02x'
> >+                    % (fs_type, i))
> >+            output = u_boot_console.run_command('%sls host 0:0 /' % fs_type)
> >+            assert(not 'FILE0123456789_00' in output)
> >+            assert(not 'FILE0123456789_41' in output)
> >+
> >+            # Test Case 10c - Create many files again
> >+            # Please note no.64 and 65 are intentionally re-created
> >+            for i in range(64, 128):
> >+                output = u_boot_console.run_command(
> >+                    '%swrite host 0:0 %x /FILE0123456789_%02x 100'
> >+                    % (fs_type, ADDR, i))
> >+            output = u_boot_console.run_command('%sls host 0:0 /' % fs_type)
> >+            assert('FILE0123456789_40' in output)
> >+            assert('FILE0123456789_79' in output)
> >+
> >+            assert_fs_integrity(fs_type, fs_img)
> >+
> >+    def test_fs_ext11(self, u_boot_console, fs_obj_ext):
> >+        """
> >+        'Test Case 11 - create/delete as many directories under non-root
> >+        directory as amount of directory entries goes beyond one cluster size)'
> >+        """
> >+        fs_type,fs_img,md5val = fs_obj_ext
> >+        with u_boot_console.log.section('Test Case 10 - create/delete (many)'):
> >+            # Test Case 11a - Create many files
> >+            #   Please note that the size of directory entry is 32 bytes.
> >+            #   So one typical cluster may holds 64 (2048/32) entries.
> >+            output = u_boot_console.run_command(
> >+                'host bind 0 %s' % fs_img)
> >+
> >+            for i in range(0, 66):
> >+                output = u_boot_console.run_command(
> >+                    '%swrite host 0:0 %x /dir1/FILE0123456789_%02x 100'
> >+                    % (fs_type, ADDR, i))
> >+            output = u_boot_console.run_command('%sls host 0:0 /dir1' % fs_type)
> >+            assert('FILE0123456789_00' in output)
> >+            assert('FILE0123456789_41' in output)
> >+
> >+            # Test Case 11b - Delete many files
> >+            for i in range(0, 66):
> >+                output = u_boot_console.run_command(
> >+                    '%srm host 0:0 /dir1/FILE0123456789_%02x'
> >+                    % (fs_type, i))
> >+            output = u_boot_console.run_command('%sls host 0:0 /dir1' % fs_type)
> >+            assert(not 'FILE0123456789_00' in output)
> >+            assert(not 'FILE0123456789_41' in output)
> >+
> >+            # Test Case 11c - Create many files again
> >+            # Please note no.64 and 65 are intentionally re-created
> >+            for i in range(64, 128):
> >+                output = u_boot_console.run_command(
> >+                    '%swrite host 0:0 %x /dir1/FILE0123456789_%02x 100'
> >+                    % (fs_type, ADDR, i))
> >+            output = u_boot_console.run_command('%sls host 0:0 /dir1' % fs_type)
> >+            assert('FILE0123456789_40' in output)
> >+            assert('FILE0123456789_79' in output)
> >+
> >+            assert_fs_integrity(fs_type, fs_img)
> >
> 

[U-Boot] [PATCH 4/4] test/py: test_fs: add tests for creating/deleting many files

[Tom Rini]

On Wed, May 15, 2019 at 03:05:16PM +0900, AKASHI Takahiro wrote:
> On Tue, May 14, 2019 at 08:09:53PM +0200, Heinrich Schuchardt wrote:
> > On 5/13/19 7:49 AM, AKASHI Takahiro wrote:
> > >Two test cases are added under test_fs_ext:
> > >     test case 10: for root directory
> > >     test case 11: for non-root directory
> > 
> > When running `make tests` all of these tests seem to be skipped:
> > 
> > test/py/tests/test_fs/test_basic.py
> > sssssssssssssssssssssssssssssssssssssss
> >                                                         [  0%]
> > test/py/tests/test_fs/test_ext.py ssssssssssssssssssssss
> > 
> >                     [  0%]
> > test/py/tests/test_fs/test_mkdir.py ssssssssssss
> > 
> >                     [  0%]
> > test/py/tests/test_fs/test_symlink.py ssss
> > 
> >                     [  0%]
> > test/py/tests/test_fs/test_unlink.py ssssssssssssss
> > 
> > How can they be executed?
> 
> Install guestmount and set it up correctly.

Is that documented somewhere in U-Boot?  Are we doing that for travis?
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/20190515/8b92dd53/attachment.sig>

[U-Boot] [PATCH 0/4] fs: fat: fixes for write under root directory

[Heinrich Schuchardt]

On 5/13/19 7:49 AM, AKASHI Takahiro wrote:
> This patch set contains three (independent) bug fixes relating to
> write operations to root directory. See each commit message
> for more details.
>
> Without those fixes, you may lose files that you created under root
> directory or will see the file system corrupted. This will happen
> particularly when you create (or even delete) many files repeatedly
> under root directory.
> (Non-root directory won't be affected.)
>
> I only tested my patches on limited number of file system instances,
> and hope that other people will also try them.
>
> -Takahiro Akashi
>
> AKASHI Takahiro (4):
>   fs: fat:  write to non-cluster-aligned root directory
>   fs: fat: flush a directory cluster properly
>   fs: fat: allocate a new cluster for root directory of fat32
>   test/py: test_fs: add tests for creating/deleting many files

Hello Takahiro,

I would be really happy if these patches would make it into v2019.07.
When running the SCT they make a big difference. I hope you have time to
resubmit these.

Best regards

Heinrich

>
>  fs/fat/fat.c                      |  15 ++--
>  fs/fat/fat_write.c                | 117 +++++++++++++++++++-----------
>  test/py/tests/test_fs/test_ext.py |  84 +++++++++++++++++++++
>  3 files changed, 165 insertions(+), 51 deletions(-)
>