diff: improving msdosfs write speed for large files

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

diff: improving msdosfs write speed for large files

Alexander Polakov-2
This is a diff from NetBSD pr.34583:
http://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=34583

Quoting the author:

        "I noticed that when writing large file (hundreds of megabytes)
        to an msdos disk, the writing speed to a file decreases with the
        file length.
        Since I have some experience with messydos filesystems (I wrote
        MSH: for the Amiga) I took a look.
        The obvious suspicion with operations that slow down with the
        length of a file is an excessive traversal of the FAT cluster
        chain. However, there is a cache that caches 2 positions: the
        last cluster in the file, and the "most recently looked up" one.
        Debugging info showed however that frequent full traversals were
        still made. So, apparently when extending a file and after
        updating the end cluster, the previous end is again needed.
        Adding a 3rd entry in the cache, which keeps the end position
        from just before extending a file.
        This has the desired effect of keeping the write speed constant.
        (What it is that needs that position I have not been able to
        ascertain from the filesystem code; it doesn't seem to make
        sense, actually, to read or write clusters before the original
        EOF. I was hoping to find the place where the cache is trashed
        and rewrite it to get the desired info from it beforehand, so
        that the extra cache entry is again unneeded, but alas.)"

While there, I changed 0 to NULL for two pointer arguments of
extendfile().

Index: sys/msdosfs/denode.h
===================================================================
RCS file: /cvs/src/sys/msdosfs/denode.h,v
retrieving revision 1.23
diff -u -p -u -r1.23 denode.h
--- sys/msdosfs/denode.h 17 Jul 2010 19:27:07 -0000 1.23
+++ sys/msdosfs/denode.h 1 Apr 2012 14:27:48 -0000
@@ -116,10 +116,11 @@ struct fatcache {
  * cache is probably pretty worthless if a file is opened by multiple
  * processes.
  */
-#define FC_SIZE 2 /* number of entries in the cache */
+#define FC_SIZE 3 /* number of entries in the cache */
 #define FC_LASTMAP 0 /* entry the last call to pcbmap() resolved
  * to */
 #define FC_LASTFC 1 /* entry for the last cluster in the file */
+#define FC_NEXTTOLASTFC 2 /* entry for a close to the last cluster in the file */
 
 #define FCE_EMPTY 0xffffffff /* doesn't represent an actual cluster # */
 
@@ -130,6 +131,12 @@ struct fatcache {
  (dep)->de_fc[slot].fc_frcn = frcn; \
  (dep)->de_fc[slot].fc_fsrcn = fsrcn;
 
+#define fc_last_to_nexttolast(dep) \
+ do {  \
+ (dep)->de_fc[FC_NEXTTOLASTFC].fc_frcn = (dep)->de_fc[FC_LASTFC].fc_frcn; \
+ (dep)->de_fc[FC_NEXTTOLASTFC].fc_fsrcn = (dep)->de_fc[FC_LASTFC].fc_fsrcn; \
+ } while (0)
+
 /*
  * This is the in memory variant of a dos directory entry.  It is usually
  * contained within a vnode.
Index: sys/msdosfs/msdosfs_fat.c
===================================================================
RCS file: /cvs/src/sys/msdosfs/msdosfs_fat.c,v
retrieving revision 1.22
diff -u -p -u -r1.22 msdosfs_fat.c
--- sys/msdosfs/msdosfs_fat.c 4 Jul 2011 04:30:41 -0000 1.22
+++ sys/msdosfs/msdosfs_fat.c 1 Apr 2012 14:27:49 -0000
@@ -952,6 +952,8 @@ extendfile(struct denode *dep, uint32_t
  return (error);
  }
 
+ fc_last_to_nexttolast(dep);
+
  while (count > 0) {
  /*
  * Allocate a new cluster chain and cat onto the end of the
Index: sys/msdosfs/msdosfs_lookup.c
===================================================================
RCS file: /cvs/src/sys/msdosfs/msdosfs_lookup.c,v
retrieving revision 1.24
diff -u -p -u -r1.24 msdosfs_lookup.c
--- sys/msdosfs/msdosfs_lookup.c 4 Jul 2011 04:30:41 -0000 1.24
+++ sys/msdosfs/msdosfs_lookup.c 1 Apr 2012 14:27:49 -0000
@@ -620,8 +620,9 @@ createde(struct denode *dep, struct deno
  diroffset = ddep->de_fndoffset + sizeof(struct direntry)
     - ddep->de_FileSize;
  dirclust = de_clcount(pmp, diroffset);
- if ((error = extendfile(ddep, dirclust, 0, 0, DE_CLEAR)) != 0) {
- (void)detrunc(ddep, ddep->de_FileSize, 0, NOCRED, NULL);
+ error = extendfile(ddep, dirclust, NULL, NULL, DE_CLEAR);
+ if (error) {
+ (void) detrunc(ddep, ddep->de_FileSize, 0, NOCRED, NULL);
  return error;
  }

tests:

w/o the patch:
 time cp huge.file /mnt/storage/
   4m5.87s real     0m0.04s user     0m17.56s system

w/the patch:
 time cp huge.file /mnt/storage/
   2m22.48s real     0m0.02s user     0m45.30s system

--
Alexander Polakov | plhk.ru

Reply | Threaded
Open this post in threaded view
|

Re: diff: improving msdosfs write speed for large files

Alexander Hall
Alexander Polakov <[hidden email]> wrote:

>This is a diff from NetBSD pr.34583:
>http://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=34583
>
>Quoting the author:
>
> "I noticed that when writing large file (hundreds of megabytes)
> to an msdos disk, the writing speed to a file decreases with the
> file length.
> Since I have some experience with messydos filesystems (I wrote
> MSH: for the Amiga) I took a look.
> The obvious suspicion with operations that slow down with the
> length of a file is an excessive traversal of the FAT cluster
> chain. However, there is a cache that caches 2 positions: the
> last cluster in the file, and the "most recently looked up" one.
> Debugging info showed however that frequent full traversals were
> still made. So, apparently when extending a file and after
> updating the end cluster, the previous end is again needed.
> Adding a 3rd entry in the cache, which keeps the end position
> from just before extending a file.
> This has the desired effect of keeping the write speed constant.
> (What it is that needs that position I have not been able to
> ascertain from the filesystem code; it doesn't seem to make
> sense, actually, to read or write clusters before the original
> EOF. I was hoping to find the place where the cache is trashed
> and rewrite it to get the desired info from it beforehand, so
> that the extra cache entry is again unneeded, but alas.)"
>
>While there, I changed 0 to NULL for two pointer arguments of
>extendfile().
>
>Index: sys/msdosfs/denode.h
>===================================================================
>RCS file: /cvs/src/sys/msdosfs/denode.h,v
>retrieving revision 1.23
>diff -u -p -u -r1.23 denode.h
>--- sys/msdosfs/denode.h 17 Jul 2010 19:27:07 -0000 1.23
>+++ sys/msdosfs/denode.h 1 Apr 2012 14:27:48 -0000
>@@ -116,10 +116,11 @@ struct fatcache {
>  * cache is probably pretty worthless if a file is opened by multiple
>  * processes.
>  */
>-#define FC_SIZE 2 /* number of entries in the cache */
>+#define FC_SIZE 3 /* number of entries in the cache */
> #define FC_LASTMAP 0 /* entry the last call to pcbmap() resolved
> * to */
> #define FC_LASTFC 1 /* entry for the last cluster in the file */
>+#define FC_NEXTTOLASTFC 2 /* entry for a close to the last cluster in
>the file */
>
>#define FCE_EMPTY 0xffffffff /* doesn't represent an actual cluster #
>*/
>
>@@ -130,6 +131,12 @@ struct fatcache {
> (dep)->de_fc[slot].fc_frcn = frcn; \
> (dep)->de_fc[slot].fc_fsrcn = fsrcn;
>
>+#define fc_last_to_nexttolast(dep) \
>+ do {  \
>+ (dep)->de_fc[FC_NEXTTOLASTFC].fc_frcn =
>(dep)->de_fc[FC_LASTFC].fc_frcn; \
>+ (dep)->de_fc[FC_NEXTTOLASTFC].fc_fsrcn =
>(dep)->de_fc[FC_LASTFC].fc_fsrcn; \
>+ } while (0)
>+
> /*
>* This is the in memory variant of a dos directory entry.  It is
>usually
>  * contained within a vnode.
>Index: sys/msdosfs/msdosfs_fat.c
>===================================================================
>RCS file: /cvs/src/sys/msdosfs/msdosfs_fat.c,v
>retrieving revision 1.22
>diff -u -p -u -r1.22 msdosfs_fat.c
>--- sys/msdosfs/msdosfs_fat.c 4 Jul 2011 04:30:41 -0000 1.22
>+++ sys/msdosfs/msdosfs_fat.c 1 Apr 2012 14:27:49 -0000
>@@ -952,6 +952,8 @@ extendfile(struct denode *dep, uint32_t
> return (error);
> }
>
>+ fc_last_to_nexttolast(dep);
>+
> while (count > 0) {
> /*
> * Allocate a new cluster chain and cat onto the end of the
>Index: sys/msdosfs/msdosfs_lookup.c
>===================================================================
>RCS file: /cvs/src/sys/msdosfs/msdosfs_lookup.c,v
>retrieving revision 1.24
>diff -u -p -u -r1.24 msdosfs_lookup.c
>--- sys/msdosfs/msdosfs_lookup.c 4 Jul 2011 04:30:41 -0000 1.24
>+++ sys/msdosfs/msdosfs_lookup.c 1 Apr 2012 14:27:49 -0000
>@@ -620,8 +620,9 @@ createde(struct denode *dep, struct deno
> diroffset = ddep->de_fndoffset + sizeof(struct direntry)
>    - ddep->de_FileSize;
> dirclust = de_clcount(pmp, diroffset);
>- if ((error = extendfile(ddep, dirclust, 0, 0, DE_CLEAR)) != 0) {
>- (void)detrunc(ddep, ddep->de_FileSize, 0, NOCRED, NULL);
>+ error = extendfile(ddep, dirclust, NULL, NULL, DE_CLEAR);
>+ if (error) {
>+ (void) detrunc(ddep, ddep->de_FileSize, 0, NOCRED, NULL);
> return error;
> }
>
>tests:
>
>w/o the patch:
> time cp huge.file /mnt/storage/
>   4m5.87s real     0m0.04s user     0m17.56s system
>
>w/the patch:
> time cp huge.file /mnt/storage/
>   2m22.48s real     0m0.02s user     0m45.30s system
>
>--
>Alexander Polakov | plhk.ru

Just curious; how huge is that file?

/Alexander

Reply | Threaded
Open this post in threaded view
|

Re: diff: improving msdosfs write speed for large files

Alexander Polakov-2
* Alexander Hall <[hidden email]> [120404 16:16]:

> Alexander Polakov <[hidden email]> wrote:
> >tests:
> >
> >w/o the patch:
> > time cp huge.file /mnt/storage/
> >   4m5.87s real     0m0.04s user     0m17.56s system
> >
> >w/the patch:
> > time cp huge.file /mnt/storage/
> >   2m22.48s real     0m0.02s user     0m45.30s system
> >
> >--
> >Alexander Polakov | plhk.ru
>
> Just curious; how huge is that file?

$ ls -lah huge.file
 -rw-r--r--  1 estet  users   500M Apr  4 08:41 huge.file

--
Alexander Polakov | plhk.ru

Reply | Threaded
Open this post in threaded view
|

Re: diff: improving msdosfs write speed for large files

Mike Belopuhov
In reply to this post by Alexander Polakov-2
On Wed, Apr 04, 2012 at 14:42 +0400, Alexander Polakov wrote:

> This is a diff from NetBSD pr.34583:
> http://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=34583
>
> Quoting the author:
>
> "I noticed that when writing large file (hundreds of megabytes)
> to an msdos disk, the writing speed to a file decreases with the
> file length.
> Since I have some experience with messydos filesystems (I wrote
> MSH: for the Amiga) I took a look.
> The obvious suspicion with operations that slow down with the
> length of a file is an excessive traversal of the FAT cluster
> chain. However, there is a cache that caches 2 positions: the
> last cluster in the file, and the "most recently looked up" one.
> Debugging info showed however that frequent full traversals were
> still made. So, apparently when extending a file and after
> updating the end cluster, the previous end is again needed.
> Adding a 3rd entry in the cache, which keeps the end position
> from just before extending a file.
> This has the desired effect of keeping the write speed constant.
> (What it is that needs that position I have not been able to
> ascertain from the filesystem code; it doesn't seem to make
> sense, actually, to read or write clusters before the original
> EOF. I was hoping to find the place where the cache is trashed
> and rewrite it to get the desired info from it beforehand, so
> that the extra cache entry is again unneeded, but alas.)"
>
> While there, I changed 0 to NULL for two pointer arguments of
> extendfile().
>

i agree that this is a great find.  i don't really like the diff though.
i see no point in introducing this macro.  what do others think?

Index: msdosfs/denode.h
===================================================================
RCS file: /cvs/src/sys/msdosfs/denode.h,v
retrieving revision 1.23
diff -u -p -r1.23 denode.h
--- msdosfs/denode.h 17 Jul 2010 19:27:07 -0000 1.23
+++ msdosfs/denode.h 4 Apr 2012 12:20:23 -0000
@@ -116,10 +116,11 @@ struct fatcache {
  * cache is probably pretty worthless if a file is opened by multiple
  * processes.
  */
-#define FC_SIZE 2 /* number of entries in the cache */
+#define FC_SIZE 3 /* number of entries in the cache */
 #define FC_LASTMAP 0 /* entry the last call to pcbmap() resolved
  * to */
 #define FC_LASTFC 1 /* entry for the last cluster in the file */
+#define FC_OLASTFC 2 /* entry for the previous last cluster */
 
 #define FCE_EMPTY 0xffffffff /* doesn't represent an actual cluster # */
 
Index: msdosfs/msdosfs_fat.c
===================================================================
RCS file: /cvs/src/sys/msdosfs/msdosfs_fat.c,v
retrieving revision 1.22
diff -u -p -r1.22 msdosfs_fat.c
--- msdosfs/msdosfs_fat.c 4 Jul 2011 04:30:41 -0000 1.22
+++ msdosfs/msdosfs_fat.c 4 Apr 2012 12:20:26 -0000
@@ -952,6 +952,13 @@ extendfile(struct denode *dep, uint32_t
  return (error);
  }
 
+ /*
+ * Preserve value for the last cluster before extending the file
+ * to speed up further lookups.
+ */
+ fc_setcache(dep, FC_OLASTFC, dep->de_fc[FC_LASTFC].fc_frcn,
+    dep->de_fc[FC_LASTFC].fc_fsrcn);
+
  while (count > 0) {
  /*
  * Allocate a new cluster chain and cat onto the end of the

Reply | Threaded
Open this post in threaded view
|

Re: diff: improving msdosfs write speed for large files

Kenneth R Westerback
On Wed, Apr 04, 2012 at 03:51:38PM +0200, Mike Belopuhov wrote:

> On Wed, Apr 04, 2012 at 14:42 +0400, Alexander Polakov wrote:
> > This is a diff from NetBSD pr.34583:
> > http://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=34583
> >
> > Quoting the author:
> >
> > "I noticed that when writing large file (hundreds of megabytes)
> > to an msdos disk, the writing speed to a file decreases with the
> > file length.
> > Since I have some experience with messydos filesystems (I wrote
> > MSH: for the Amiga) I took a look.
> > The obvious suspicion with operations that slow down with the
> > length of a file is an excessive traversal of the FAT cluster
> > chain. However, there is a cache that caches 2 positions: the
> > last cluster in the file, and the "most recently looked up" one.
> > Debugging info showed however that frequent full traversals were
> > still made. So, apparently when extending a file and after
> > updating the end cluster, the previous end is again needed.
> > Adding a 3rd entry in the cache, which keeps the end position
> > from just before extending a file.
> > This has the desired effect of keeping the write speed constant.
> > (What it is that needs that position I have not been able to
> > ascertain from the filesystem code; it doesn't seem to make
> > sense, actually, to read or write clusters before the original
> > EOF. I was hoping to find the place where the cache is trashed
> > and rewrite it to get the desired info from it beforehand, so
> > that the extra cache entry is again unneeded, but alas.)"
> >
> > While there, I changed 0 to NULL for two pointer arguments of
> > extendfile().
> >
>
> i agree that this is a great find.  i don't really like the diff though.
> i see no point in introducing this macro.  what do others think?

Agreed. I like this version better.

.... Ken

>
> Index: msdosfs/denode.h
> ===================================================================
> RCS file: /cvs/src/sys/msdosfs/denode.h,v
> retrieving revision 1.23
> diff -u -p -r1.23 denode.h
> --- msdosfs/denode.h 17 Jul 2010 19:27:07 -0000 1.23
> +++ msdosfs/denode.h 4 Apr 2012 12:20:23 -0000
> @@ -116,10 +116,11 @@ struct fatcache {
>   * cache is probably pretty worthless if a file is opened by multiple
>   * processes.
>   */
> -#define FC_SIZE 2 /* number of entries in the cache */
> +#define FC_SIZE 3 /* number of entries in the cache */
>  #define FC_LASTMAP 0 /* entry the last call to pcbmap() resolved
>   * to */
>  #define FC_LASTFC 1 /* entry for the last cluster in the file */
> +#define FC_OLASTFC 2 /* entry for the previous last cluster */
>  
>  #define FCE_EMPTY 0xffffffff /* doesn't represent an actual cluster # */
>  
> Index: msdosfs/msdosfs_fat.c
> ===================================================================
> RCS file: /cvs/src/sys/msdosfs/msdosfs_fat.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 msdosfs_fat.c
> --- msdosfs/msdosfs_fat.c 4 Jul 2011 04:30:41 -0000 1.22
> +++ msdosfs/msdosfs_fat.c 4 Apr 2012 12:20:26 -0000
> @@ -952,6 +952,13 @@ extendfile(struct denode *dep, uint32_t
>   return (error);
>   }
>  
> + /*
> + * Preserve value for the last cluster before extending the file
> + * to speed up further lookups.
> + */
> + fc_setcache(dep, FC_OLASTFC, dep->de_fc[FC_LASTFC].fc_frcn,
> +    dep->de_fc[FC_LASTFC].fc_fsrcn);
> +
>   while (count > 0) {
>   /*
>   * Allocate a new cluster chain and cat onto the end of the

Reply | Threaded
Open this post in threaded view
|

Re: diff: improving msdosfs write speed for large files

Alexander Polakov-2
In reply to this post by Mike Belopuhov
* Mike Belopuhov <[hidden email]> [120404 17:51]:
> i agree that this is a great find.  i don't really like the diff though.
> i see no point in introducing this macro.  what do others think?

Your diff looks better to me.

> Index: msdosfs/denode.h
> ===================================================================
> RCS file: /cvs/src/sys/msdosfs/denode.h,v
> retrieving revision 1.23
> diff -u -p -r1.23 denode.h
> --- msdosfs/denode.h 17 Jul 2010 19:27:07 -0000 1.23
> +++ msdosfs/denode.h 4 Apr 2012 12:20:23 -0000
> @@ -116,10 +116,11 @@ struct fatcache {
>   * cache is probably pretty worthless if a file is opened by multiple
>   * processes.
>   */
> -#define FC_SIZE 2 /* number of entries in the cache */
> +#define FC_SIZE 3 /* number of entries in the cache */
>  #define FC_LASTMAP 0 /* entry the last call to pcbmap() resolved
>   * to */
>  #define FC_LASTFC 1 /* entry for the last cluster in the file */
> +#define FC_OLASTFC 2 /* entry for the previous last cluster */
>  
>  #define FCE_EMPTY 0xffffffff /* doesn't represent an actual cluster # */
>  
> Index: msdosfs/msdosfs_fat.c
> ===================================================================
> RCS file: /cvs/src/sys/msdosfs/msdosfs_fat.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 msdosfs_fat.c
> --- msdosfs/msdosfs_fat.c 4 Jul 2011 04:30:41 -0000 1.22
> +++ msdosfs/msdosfs_fat.c 4 Apr 2012 12:20:26 -0000
> @@ -952,6 +952,13 @@ extendfile(struct denode *dep, uint32_t
>   return (error);
>   }
>  
> + /*
> + * Preserve value for the last cluster before extending the file
> + * to speed up further lookups.
> + */
> + fc_setcache(dep, FC_OLASTFC, dep->de_fc[FC_LASTFC].fc_frcn,
> +    dep->de_fc[FC_LASTFC].fc_fsrcn);
> +
>   while (count > 0) {
>   /*
>   * Allocate a new cluster chain and cat onto the end of the

--
Alexander Polakov | plhk.ru

Reply | Threaded
Open this post in threaded view
|

Re: diff: improving msdosfs write speed for large files

Mike Belopuhov
On Thu, Apr 5, 2012 at 9:21 AM, Alexander Polakov <[hidden email]> wrote:
> * Mike Belopuhov <[hidden email]> [120404 17:51]:
>> i agree that this is a great find.  i don't really like the diff though.
>> i see no point in introducing this macro.  what do others think?
>
> Your diff looks better to me.
>

the diff is in. thanks for pointing this out to us!