fflush() support

[adamgreen]

A few weeks ago I was reviewing PR #1458 from @neilt6 and it got me curious as to what was actually up with fflush(). I have looked at it a bit over the last few days and now have some thoughts that I figured might be worthy sharing here to potentially start a conversation and maybe even come up with a solution. Forcing aggressive syncing on within the FATFileSystem as is currently done is probably not the best solution so it would be cool if we could come up with something better.

What's up with fflush()? In newlib (used by many GCC based toolchains for ARM) it's main task is to flush any queued up writes in the buffers associated with FILE* streams. So it makes sure that outstanding writes from within the FILE* stream buffers get flushed down to the file system layer. However the code doesn't do anything which would then request that the file system layer flush it's outstanding writes to non-volatile media. The ARM/Keil fflush() functionality appears to be similar. However, ARM/Keil did previously make sync calls down to the file system through the _sys_ensure() sys call but this has since been deprecated. The mbed SDK even implements this deprecated sys call in retarget.cpp On a related note, I don't really see a convenient place to hook in the lower level flushing of file system writes in newlib's sys call layer either.

One of the ideas I had had was to implement a new mbed_fflush() API in retarget.cpp which would first call the standard fflush() routine to flush FILE* buffers and then go about calling the fsync() method on the appropriate FileHandle objects to perform a lower level file system flush to media.

This is a prototype I built of that idea:

extern "C" int mbed_fflush(FILE *file) {
    /* First call the toolchain's default implementation to let it flush the stream to the filesystem. */
    int result = fflush(file);
    if (result != 0)
        return result;

    /* Now ask the file system to flush the data to the actual storage media. */
    if (file != NULL) {
        /* Request that the file system flush the file handle of this specific stream. */
        return _fflush_fh(fileno(file));
    }

    /* NULL pointer means to flush all open files. */
    for (size_t i = 0; i < sizeof(filehandles)/sizeof(filehandles[0]); i++) {
        if (filehandles[i] != NULL && _fflush_fh(i+3) != 0)
            result = EOF;
    }
    return result;
}

static int _fflush_fh(int fh)
{
    /* Do nothing for stdin/stdout/stderr. */
    if (fh < 3)
        return 0;

    /* Call fsync() method if FileHandler pointer isn't NULL. */
    FileHandle* fhc = filehandles[fh-3];
    if (fhc == NULL)
        return EOF;
    if (fhc->fsync())
        return EOF;
    return 0;
}

I did some testing with that on GCC_ARM and it appeared to work as expected. I then took it over to the online compiler and I hit a snag! This code requires access to the fileno() API so that it can determine which FileHandle* maps to a particular FILE* stream. I couldn't find this API or anything like it with the online compiler. As far as I can tell the FILE* pointer is opaque on that platform as well so no simple dereferencing of a field appears to be possible either.

Another thought I have had is that maybe we could just break the flushing process up into two parts:

• First the user calls fflush() to cause the FILE* stream buffered writes to be pushed down to the file system level.
• Next the user calls a fflush() method on the file system object itself to have it iterate through all of its own open handles and then call the fsync() method on each.

My prototype of this solution can be found in this commit from another GitHub repo that I have been working on recently: (adamgreen/SDCard@859b1af). If we were to go with that type of solution, then it might make more sense to add the fflush() method support to the file system base classes instead of these FATFileSystem specific ones.

Thoughts??

[sg-]

Let's further investigate fileno()

[adamgreen]

I do think that fflush() as it is currently implemented is correct. It should really just be expected to flush the FILE* stream buffers to the underlying file system layer. That is really all the Standard C Library indicates that it should do.

The Posix sync() API is what you would use to dump the underlying buffer cache to physical media. So what should really be added to the mbed sdk is a sync() or mbed_sync() function that could be called after the libC fflush() function.

• It could be implemented in the FileSystemLike/FileBase classes similar to what I did in the (adamgreen/SDCard@859b1af) prototype but renamed to sync instead of flush and pushed further down into the mbed SDK bases classes.
• It could also be done in retarget.cpp by iterating through the filehandles[] array and calling the fsync() method on each open handle.

The first method has the advantage that an individual file system could override the virtual sync() method and perform some work at file system scope instead of or in addition to work at the scope of the individual files. The FATFileSystem can perform everything it needs at the file handle level so either method works for it.

With respect to fileno(), it isn't a Standard C Library function so ARM/Keil not supporting it is ok.

[bridadan]

While I admit I'm not that familiar with the FileSystem code base, I would say having a sync() call would be extremely useful for things like Serial. Right now when you change the baud on a serial instance, there are times when the baud will change even if it's in the middle of flushing out the FIFO (which had a different baud rate at the time). It would be very useful to be able fflush() before baud rate changes occur. Right now I think the only option is to busy wait.

[ciarmcom]

ARM Internal Ref: IOTMORF-213