fs: fat: fix bug when write size is less than a sector size

PATCH New Josh Wu 2 participants 5 messages Jul 18, 2013 – Jul 23, 2013 VFAT/FAT/msdos filesystem

lore

Share & tools

lore

Josh Wu

author

2013-07-18 08:07 UTC #0 lore raw

PATCH 1/1 fs: fat: fix bug when write size is less than a sector size

Tested in at91sam9x5ek and sama5d3x-ek in mmc fat.

Signed-off-by Josh Wu <[email protected]>

---
When I debugging this issue, I also find the thread talking about this:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/158698
But finally no patch is send for this bug. So I send this and cc all the people in that thread. Hope it can be merged in 2013.07.

 fs/fat/fat_write.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

fs/fat/fat_write.c +6 -2

index 5829adf..b7ba8ea 100644
@@ -561,7 +561,7 @@ set_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer,
 	     unsigned long size)
 {
 	int idx = 0;
-	__u32 startsect;
+	__u32 startsect, nr_sectors;
 
 	if (clustnum > 0)
 		startsect = mydata->data_begin +

@@ -571,7 +571,11 @@ set_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer,
 
 	debug("clustnum: %d, startsect: %d\n", clustnum, startsect);
 
-	if (disk_write(startsect, size / mydata->sect_size, buffer) < 0) {
+	nr_sectors = size / mydata->sect_size;
+	if (nr_sectors == 0)
+		nr_sectors = 1;
+
+	if (disk_write(startsect, nr_sectors, buffer) < 0) {
 		debug("Error writing data\n");
 		return -1;
 	}

Signature · 1 line

1.7.9.5

Wolfgang Denk

2013-07-18 08:24 UTC #1 lore raw

PATCH 1/1 fs: fat: fix bug when write size is less than a sector size

Dear Josh Wu,

Quoted 8 lines · In message <[email protected]> you wrote:

Tested in at91sam9x5ek and sama5d3x-ek in mmc fat.
Signed-off-by Josh Wu <[email protected]>
---
When I debugging this issue, I also find the thread talking about this:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/158698
But finally no patch is send for this bug. So I send this and cc all the people in that thread. Hope it can be merged in 2013.07.

Definitely not. This needs thurough testing before it can go mainline.

fs/fat/fat_write.c set_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, · line 576

Quoted 13 lines

fs/fat/fat_write.c +4 -2

-	__u32 startsect;
+	__u32 startsect, nr_sectors;
 
 	if (clustnum > 0)
 		startsect = mydata->data_begin +

@@ -571,7 +571,11 @@ set_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer,
 
 	debug("clustnum: %d, startsect: %d\n", clustnum, startsect);
 
-	if (disk_write(startsect, size / mydata->sect_size, buffer) < 0) {
+	nr_sectors = size / mydata->sect_size;
+	if (nr_sectors == 0)
+		nr_sectors = 1;

Why would this be needed? The case of a partial write is covered further down below in the code, in the "if (size % mydata->sect_size)" part...

Best regards,

Wolfgang Denk

Signature · 4 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I still miss my ex-wife, but my aim is getting better.

Josh Wu

author

2013-07-19 09:43 UTC #2 lore raw

PATCH 1/1 fs: fat: fix bug when write size is less than a sector size

Dear Wolfgang Denk,

Thank you for the quick reply.

Quoted 5 lines · Wolfgang Denk

Dear Josh Wu,
Quoted 2 lines · In message <[email protected]> you wrote:
Tested in at91sam9x5ek and sama5d3x-ek in mmc fat.

Quoted 8 lines

Quoted 6 lines
Signed-off-by Josh Wu <[email protected]>
---
When I debugging this issue, I also find the thread talking about this:
   http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/158698
But finally no patch is send for this bug. So I send this and cc all the people in that thread. Hope it can be merged in 2013.07.
Definitely not. This needs thurough testing before it can go mainline.

got it.

Quoted 11 lines

fs/fat/fat_write.c +1 -1

-	__u32 startsect;
+	__u32 startsect, nr_sectors;
 
  	if (clustnum > 0)
  		startsect = mydata->data_begin +

@@ -571,7 +571,11 @@ set_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer,
 
  	debug("clustnum: %d, startsect: %d\n", clustnum, startsect);

Quoted 7 lines

fs/fat/fat_write.c set_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, · line 576
Quoted 4 lines
fs/fat/fat_write.c +3 -1
-	if (disk_write(startsect, size / mydata->sect_size, buffer) < 0) {
+	nr_sectors = size / mydata->sect_size;
+	if (nr_sectors == 0)
+		nr_sectors = 1;
Why would this be needed? The case of a partial write is covered further down below in the code, in the "if (size % mydata->sect_size)" part...

The call of disk_write() will pass the sector size as 0. That will cause the mmc driver stalled.

And after a search, the fix patch is already sent by Ruud Commandeur and merge in mainline:
"mmc write bug fix"
commit a586c0aa211fb79ecaa06aee3299bfdd81329876
Author: Ruud Commandeur <[email protected]>
Date: Wed May 22 13:19:43 2013 +0200

So please forget this patch. Since it is fixed in mmc driver layer. I will test it in my board.
Sorry for the noise.

Quoted 7 lines

Best regards,
Wolfgang Denk

Best Regards,
Josh Wu

Wolfgang Denk

2013-07-19 11:20 UTC #3 lore raw

PATCH 1/1 fs: fat: fix bug when write size is less than a sector size

Dear Josh Wu,

Quoted 7 lines · In message <[email protected]> you wrote:

Quoted 4 lines
Why would this be needed? The case of a partial write is covered further down below in the code, in the "if (size % mydata->sect_size)" part...
The call of disk_write() will pass the sector size as 0. That will cause the mmc driver stalled.

I think instead of "sector size" you mean the number of sectors. The sector size is fixed.

Big question: why should we call disk_write() at all when the count is zero? Maybe a simple

	if ((size / mydata->sect_size) > 0) {
		if (disk_write(startsect, size / mydata->sect_size, buffer) < 0) {
			...
		}
	}

would fix this issue?

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: [email protected]
Der Irrtum wiederholt sich immerfort in der Tat.  Deshalb mu? man das
Wahre unerm?dlich in Worten wiederholen.                     - Goethe

Josh Wu

author

2013-07-23 10:16 UTC #4 lore raw

PATCH 1/1 fs: fat: fix bug when write size is less than a sector size

Hi, Dear Wolfgang Denk

Quoted 10 lines · Wolfgang Denk

Dear Josh Wu,
Quoted 5 lines · In message <[email protected]> you wrote:
Quoted 3 lines
Why would this be needed? The case of a partial write is covered further down below in the code, in the "if (size % mydata->sect_size)" part...
The call of disk_write() will pass the sector size as 0. That will cause the mmc driver stalled.
I think instead of "sector size" you mean the number of sectors. The sector size is fixed.

yes, I mean the number of sectors.

Quoted 11 lines

Big question: why should we call disk_write() at all when the count is zero? Maybe a simple
	if ((size / mydata->sect_size) > 0) {
		if (disk_write(startsect, size / mydata->sect_size, buffer) < 0) {
			...
		}
	}
would fix this issue?

I agree with above changes. It make the logical clearer:
Step 1. write buffer in sectors. If buffer size is less than one
sector we don't need to call disk_write() with zero sector.
Step 2. write remain buffer in one sector.

So I think I will send a v2 patch according to your suggestion. Thanks.

Quoted 6 lines

Best regards,
Wolfgang Denk

Best Regards,
Josh Wu