fsync() for FAT file system

[Thomas Doerfler]

Sebastian,

if I get this code right, it actually parses each buffer of the _volume_
and ensures it is synced, so this is FAR from optimal ;-)

Syncing the device will defintivly be an improvement. For large files,
this will not be worse than syncing the file-specific buffers.

Another area: For really "syncing" the file, would it make sense to also
ensure that the file/directory information is updated as far as possible
to make sure the FS contains as much information as possible to read
back the file (in case of a device or power loss)?

wkr,

Thomas.



Am 04.05.2012 16:25, schrieb Sebastian Huber:
> Hi,
> 
> currently the fsync() handler for FAT file systems looks like this:
> 
> /* fat_file_datasync --
>  *     Synchronize fat-file -  flush all buffered data to the media.
>  *
>  * PARAMETERS:
>  *     mt_entry - mount table entry
>  *     fat_fd   - fat-file descriptor
>  *
>  * RETURNS:
>  *     RC_OK on success, or -1 if error occured and errno set appropriately
>  */
> int
> fat_file_datasync(
>     rtems_filesystem_mount_table_entry_t *mt_entry,
>     fat_file_fd_t                        *fat_fd
>     )
> {
>     int                 rc = RC_OK;
>     rtems_status_code   sc = RTEMS_SUCCESSFUL;
>     fat_fs_info_t      *fs_info = mt_entry->fs_info;
>     uint32_t            cur_cln = fat_fd->cln;
>     rtems_bdbuf_buffer *block = NULL;
>     uint32_t            sec = 0;
>     uint32_t            i = 0;
> 
>     if (fat_fd->fat_file_size == 0)
>         return RC_OK;
> 
>     /*
>      * we can use only one bdbuf :( and we also know that cache is useless
>      * for sync operation, so don't use it
>      */
>     rc = fat_buf_release(fs_info);
>     if (rc != RC_OK)
>         return rc;
> 
>     /* for each cluster of the file ... */
>     while ((cur_cln & fs_info->vol.mask) < fs_info->vol.eoc_val)
>     {
>         sec = fat_cluster_num_to_sector_num(mt_entry, cur_cln);
>         /* for each sector in cluster ... */
>         for ( i = 0; i < fs_info->vol.spc; i++ )
>         {
>             /* ... sync it */
>             sc = rtems_bdbuf_read(fs_info->vol.dd, (sec + i), &block);
>             if (sc != RTEMS_SUCCESSFUL)
>                 rtems_set_errno_and_return_minus_one( EIO );
> 
>             sc = rtems_bdbuf_sync(block);
>             if ( sc != RTEMS_SUCCESSFUL )
>                 rtems_set_errno_and_return_minus_one( EIO );
>         }
> 
>         rc = fat_get_fat_cluster(mt_entry, cur_cln, &cur_cln);
>         if ( rc != RC_OK )
>             return rc;
>     }
>     return rc;
> }
> 
> This function will read every cluster of the file into the cache and
> then synchronize it step-by-step.  For unmodified buffers this is a
> non-operation. For modified buffers this will wake-up the swapout task
> which performs then a single buffer write operation.  This is usually
> quite inefficient.  Firstly we do single buffer writes and secondly we
> may perform a lot of unnecessary read operations.
> 
> I think we should replace this synchronization procedure with a simple
> rtems_bdbuf_sync_dev(fs_info->vol.dev).  This has the side-effect that
> also buffers not related to the file are synchronized, but I think this
> is acceptable.
> 
> What do you think?
> 


-- 
--------------------------------------------
Embedded Brains GmbH
Thomas Doerfler           Obere Lagerstr. 30
D-82178 Puchheim          Germany
email: Thomas.Doerfler at embedded-brains.de
Phone: +49-89-18908079-2
Fax:   +49-89-18908079-9