Skip to contentSkip to conversation

fs/fat: reduce stack usage for SPL

PATCH New Rob Clark 3 participants 5 messages Sep 17, 2017 – Sep 22, 2017 VFAT/FAT/msdos filesystem

lore
Share & tools
lore

Rob Clark

author
#0 lore raw

PATCH 1/1 fs/fat: reduce stack usage for SPL

It seems like stack usage is a problem for SPL builds. So move itrblock off the stack.

Please test this and see if it helps w/ current issues with SPL builds. Long term, I'm not sure if it is better to do this conditional on SPL builds, or move to malloc()? At any rate, if this fixes SPL builds it should be a perfectly ok short term solution.

Signed-off-by Rob Clark <[email protected]>
---
 fs/fat/fat.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
fs/fat/fat.c +5 -3
index f0284398b4..93140c9bcb 100644
@@ -1031,10 +1031,12 @@ int file_fat_detectfs(void)
 	return 0;
 }
 
+static fat_itr itrblock;
+
 int fat_exists(const char *filename)
 {
 	fsdata fsdata;
-	fat_itr itrblock, *itr = &itrblock;
+	fat_itr *itr = &itrblock;
 	int ret;
 
 	ret = fat_itr_root(itr, &fsdata);

@@ -1049,7 +1051,7 @@ int fat_exists(const char *filename)
 int fat_size(const char *filename, loff_t *size)
 {
 	fsdata fsdata;
-	fat_itr itrblock, *itr = &itrblock;
+	fat_itr *itr = &itrblock;
 	int ret;
 
 	ret = fat_itr_root(itr, &fsdata);

@@ -1081,7 +1083,7 @@ int file_fat_read_at(const char *filename, loff_t pos, void *buffer,
 		     loff_t maxsize, loff_t *actread)
 {
 	fsdata fsdata;
-	fat_itr itrblock, *itr = &itrblock;
+	fat_itr *itr = &itrblock;
 	int ret;
 
 	ret = fat_itr_root(itr, &fsdata);
Signature · 1 line
2.13.5
#1 lore raw

PATCH 1/1 fs/fat: reduce stack usage for SPL

Quoted 9 lines · Rob Clark

It seems like stack usage is a problem for SPL builds. So move itrblock off the stack.

Please test this and see if it helps w/ current issues with SPL builds. Long term, I'm not sure if it is better to do this conditional on SPL builds, or move to malloc()? At any rate, if this fixes SPL builds it should be a perfectly ok short term solution.

Signed-off-by Rob Clark <[email protected]>
Tested-by Tom Rini <[email protected]>

Note that malloc would also be fine as we already have a dependency on malloc, that's just not (but should be!) expressed in Kconfig, for FAT support already. Thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170917/10264044/attachment.sig>
#2 lore raw

PATCH 1/1 fs/fat: reduce stack usage for SPL

Quoted 5 lines · Tom Rini
Quoted 3 lines · Rob Clark

It seems like stack usage is a problem for SPL builds. So move itrblock off the stack.

Quoted 9 lines
Quoted 6 lines

Please test this and see if it helps w/ current issues with SPL builds. Long term, I'm not sure if it is better to do this conditional on SPL builds, or move to malloc()? At any rate, if this fixes SPL builds it should be a perfectly ok short term solution.

Signed-off-by Rob Clark <[email protected]>
Tested-by Tom Rini <[email protected]>

This fixed the problem on the am3517-evm. Thank you,

Tested-by Adam Ford <[email protected]>
Quoted 6 lines

Note that malloc would also be fine as we already have a dependency on malloc, that's just not (but should be!) expressed in Kconfig, for FAT support already. Thanks!

--
Tom

#3 lore raw

PATCH v2 1/1 fs/fat: Reduce stack usage

In reply to Rob Clark

We have limited stack in SPL builds. Drop itrblock and move to malloc/free of itr to move this off of the stack. As part of this fix a double-free issue in fat_size().

Signed-off-by Tom Rini <[email protected]>

---
Rework to use malloc/free as moving this to a global overflows some SH targets.

 fs/fat/fat.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)
fs/fat/fat.c +10 -4
index f0284398b41d..36a309c73c27 100644
@@ -1034,24 +1034,27 @@ int file_fat_detectfs(void)
 int fat_exists(const char *filename)
 {
 	fsdata fsdata;
-	fat_itr itrblock, *itr = &itrblock;
+	fat_itr *itr;
 	int ret;
 
+	itr = malloc(sizeof(fat_itr));
 	ret = fat_itr_root(itr, &fsdata);
 	if (ret)
 		return 0;
 
 	ret = fat_itr_resolve(itr, filename, TYPE_ANY);
 	free(fsdata.fatbuf);
+	free(itr);
 	return ret == 0;
 }
 
 int fat_size(const char *filename, loff_t *size)
 {
 	fsdata fsdata;
-	fat_itr itrblock, *itr = &itrblock;
+	fat_itr *itr;
 	int ret;
 
+	itr = malloc(sizeof(fat_itr));
 	ret = fat_itr_root(itr, &fsdata);
 	if (ret)
 		return ret;

@@ -1072,8 +1075,9 @@ int fat_size(const char *filename, loff_t *size)
 	}
 
 	*size = FAT2CPU32(itr->dent->size);
-out:
 	free(fsdata.fatbuf);
+out:
+	free(itr);
 	return ret;
 }
 

@@ -1081,9 +1085,10 @@ int file_fat_read_at(const char *filename, loff_t pos, void *buffer,
 		     loff_t maxsize, loff_t *actread)
 {
 	fsdata fsdata;
-	fat_itr itrblock, *itr = &itrblock;
+	fat_itr *itr;
 	int ret;
 
+	itr = malloc(sizeof(fat_itr));
 	ret = fat_itr_root(itr, &fsdata);
 	if (ret)
 		return ret;

@@ -1097,6 +1102,7 @@ int file_fat_read_at(const char *filename, loff_t pos, void *buffer,
 
 out:
 	free(fsdata.fatbuf);
+	free(itr);
 	return ret;
 }
Signature · 1 line
1.9.1
#4 lore raw

PATCH v2 1/1 fs/fat: Reduce stack usage

Quoted 5 lines · Tom Rini

We have limited stack in SPL builds. Drop itrblock and move to malloc/free of itr to move this off of the stack. As part of this fix a double-free issue in fat_size().

Signed-off-by Tom Rini <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170922/111c395a/attachment.sig>