fsync() for FAT file system

[Thomas Doerfler]

Sebastian,

after sleeping on it one night: Maybe the impact of syncing the whole
file system may be too big for small files.

Could we implement a size limit, meaning that for files smaller than a
given size, each file cluster is synced and for larger files, the file
system is synced?

Syncing a file might use a new bdbuf function (e.g. bdbuf_read_cached()
) that avoids reading buffers from disk, if they are not already in the
cache.

The size limit might be a constant or derived from the cache size(?) or
the FS size.

When syncing the file instead of the file system, also FAT and directory
info should be synced...

Hm, this gets complicated. Maybe we should at least sketch this
functionality as comments for "future enhancements". ;-)

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