Only write the FSINFO block back to disk when its contents change.
This optimization can be important when the underlying physical media
can wear out, i.e. Flash.
Signed-off-by: Steven J. Magnani <[email protected]>
---
diff -uprN a/fs/fat/misc.c b/fs/fat/misc.c
--- a/fs/fat/misc.c 2009-06-29 11:12:40.000000000 -0500
+++ b/fs/fat/misc.c 2009-06-29 11:46:45.000000000 -0500
@@ -61,11 +61,25 @@ void fat_clusters_flush(struct super_blo
le32_to_cpu(fsinfo->signature2),
sbi->fsinfo_sector);
} else {
- if (sbi->free_clusters != -1)
- fsinfo->free_clusters = cpu_to_le32(sbi->free_clusters);
- if (sbi->prev_free != -1)
- fsinfo->next_cluster = cpu_to_le32(sbi->prev_free);
- mark_buffer_dirty(bh);
+ char write_needed = 0;
+ __le32 le_value;
+
+ if (sbi->free_clusters != -1) {
+ le_value = cpu_to_le32(sbi->free_clusters);
+ if (fsinfo->free_clusters != le_value) {
+ fsinfo->free_clusters = le_value;
+ write_needed = 1;
+ }
+ }
+ if (sbi->prev_free != -1) {
+ le_value = cpu_to_le32(sbi->prev_free);
+ if (fsinfo->next_cluster != le_value) {
+ fsinfo->next_cluster = le_value;
+ write_needed = 1;
+ }
+ }
+ if (write_needed)
+ mark_buffer_dirty(bh);
}
brelse(bh);
}
[email protected] writes:
> Only write the FSINFO block back to disk when its contents change.
> This optimization can be important when the underlying physical media
> can wear out, i.e. Flash.
I have no objection to this though. Was this tested on recent version?
Well, now, we are using sb->s_dirty for fsinfo, so I'm wondering why
this happen frequently.
Thanks.
> Signed-off-by: Steven J. Magnani <[email protected]>
> ---
> diff -uprN a/fs/fat/misc.c b/fs/fat/misc.c
> --- a/fs/fat/misc.c 2009-06-29 11:12:40.000000000 -0500
> +++ b/fs/fat/misc.c 2009-06-29 11:46:45.000000000 -0500
> @@ -61,11 +61,25 @@ void fat_clusters_flush(struct super_blo
> le32_to_cpu(fsinfo->signature2),
> sbi->fsinfo_sector);
> } else {
> - if (sbi->free_clusters != -1)
> - fsinfo->free_clusters = cpu_to_le32(sbi->free_clusters);
> - if (sbi->prev_free != -1)
> - fsinfo->next_cluster = cpu_to_le32(sbi->prev_free);
> - mark_buffer_dirty(bh);
> + char write_needed = 0;
> + __le32 le_value;
> +
> + if (sbi->free_clusters != -1) {
> + le_value = cpu_to_le32(sbi->free_clusters);
> + if (fsinfo->free_clusters != le_value) {
> + fsinfo->free_clusters = le_value;
> + write_needed = 1;
> + }
> + }
> + if (sbi->prev_free != -1) {
> + le_value = cpu_to_le32(sbi->prev_free);
> + if (fsinfo->next_cluster != le_value) {
> + fsinfo->next_cluster = le_value;
> + write_needed = 1;
> + }
> + }
> + if (write_needed)
> + mark_buffer_dirty(bh);
> }
> brelse(bh);
> }
>
--
OGAWA Hirofumi <[email protected]>
On Wed, 2009-07-01 at 05:57 +0900, OGAWA Hirofumi wrote:
> [email protected] writes:
>
> > Only write the FSINFO block back to disk when its contents change.
> > This optimization can be important when the underlying physical media
> > can wear out, i.e. Flash.
>
> I have no objection to this though. Was this tested on recent version?
I tested it on 2.6.30.
> Well, now, we are using sb->s_dirty for fsinfo, so I'm wondering why
> this happen frequently.
My scenario was modifying a sector of an existing file and using
fdatasync() to flush it. The FSINFO sector was being updated even though
nothing about the FAT layout had changed.
>
> Thanks.
Regards,
------------------------------------------------------------------------
Steven J. Magnani "I claim this network for MARS!
http://www.digidescorp.com Earthling, return my space modulator!"
#include <standard.disclaimer>
"Steven J. Magnani" <[email protected]> writes:
>> Well, now, we are using sb->s_dirty for fsinfo, so I'm wondering why
>> this happen frequently.
>
> My scenario was modifying a sector of an existing file and using
> fdatasync() to flush it. The FSINFO sector was being updated even though
> nothing about the FAT layout had changed.
I see. Probably, I'm missing something, or handling of sb->s_dirt may be
buggy, or something.
If it was fixed, is this patch still needed? I guess this patch would
still be useful on some case though. If you can explain, it would be
good.
Thanks.
--
OGAWA Hirofumi <[email protected]>
On Wed, 2009-07-01 at 09:28 +0900, OGAWA Hirofumi wrote:
> "Steven J. Magnani" <[email protected]> writes:
>
> >> Well, now, we are using sb->s_dirty for fsinfo, so I'm wondering why
> >> this happen frequently.
> >
> > My scenario was modifying a sector of an existing file and using
> > fdatasync() to flush it. The FSINFO sector was being updated even though
> > nothing about the FAT layout had changed.
>
> I see. Probably, I'm missing something, or handling of sb->s_dirt may be
> buggy, or something.
>
> If it was fixed, is this patch still needed? I guess this patch would
> still be useful on some case though. If you can explain, it would be
> good.
It is I who am missing something. The patch originated against a 2.6.20
kernel, where it does indeed suppress unnecessary updates. I saw the
same code in the 2.6.30 kernel and assumed the same issue was present,
and tested that with the patch present there were no unnecessary
updates. It appears that there are no unnecessary updates even _without_
the patch, so I withdraw it.
>
> Thanks.
Thanks for looking at this so carefully.
Regards,
------------------------------------------------------------------------
Steven J. Magnani "I claim this network for MARS!
http://www.digidescorp.com Earthling, return my space modulator!"
#include <standard.disclaimer>