wutdevoptab: Make sure to always read into/write from a properly aligned buffer

libraries/wutdevoptab/devoptab_fs_read.c

@@ -1,85 +1,68 @@
 #include "devoptab_fs.h"
 
-ssize_t
-__wut_fs_read(struct _reent *r,
-              void *fd,
-              char *ptr,
-              size_t len)
-{
-   FSStatus status = 0;
-   FSCmdBlock cmd;
-   uint8_t *alignedReadBuffer;
-   uint32_t bytes, bytesRead;
-   __wut_fs_file_t *file;
+#include <sys/param.h>
 
+ssize_t __wut_fs_read(struct _reent *r, void *fd, char *ptr, size_t len) {
    if (!fd || !ptr) {
       r->_errno = EINVAL;
       return -1;
    }
 
-   FSInitCmdBlock(&cmd);
-   file = (__wut_fs_file_t *)fd;
-   bytesRead = 0;
-
    // Check that the file was opened with read access
+   __wut_fs_file_t *file = (__wut_fs_file_t *) fd;
    if ((file->flags & O_ACCMODE) == O_WRONLY) {
       r->_errno = EBADF;
       return -1;
    }
 
-   if((((uintptr_t) ptr) & 0x3F) == 0){
-      status = FSReadFile(__wut_devoptab_fs_client, &cmd, (uint8_t *) ptr, 1,
-                            len, file->fd, 0, FS_ERROR_FLAG_ALL);    
-      if(status > 0){
-         bytesRead = (uint32_t) status;
-         file->offset += bytesRead;
+   // cache-aligned, cache-line-sized
+   __attribute__((aligned(0x40))) uint8_t alignedBuffer[0x40];
+
+   FSCmdBlock cmd;
+   FSInitCmdBlock(&cmd);
+
+   size_t bytesRead = 0;
+   while (bytesRead < len) {
+      // only use input buffer if cache-aligned and read size is a multiple of cache line size
+      // otherwise read into alignedBuffer
+      uint8_t *tmp = (uint8_t *) ptr;
+      size_t size = len - bytesRead;
+
+      if ((uintptr_t) ptr & 0x3F) {
+         // read partial cache-line front-end
+         tmp = alignedBuffer;
+         size = MIN(size, 0x40 - ((uintptr_t) ptr & 0x3F));
+      } else if (size < 0x40) {
+         // read partial cache-line back-end
+         tmp = alignedBuffer;
+      } else {
+         // read whole cache lines
+         size &= ~0x3F;
       }
-   } else {
-      // Copy to internal buffer due to alignment requirement and read in chunks.
-      // Using a buffer smaller than 128KiB takes a performance hit.
-      int buffer_size = len < 128*1024 ? len : 128*1024;
-      alignedReadBuffer = memalign(0x40, buffer_size);
-      if(!alignedReadBuffer){
-         r->_errno = ENOMEM;
+
+      FSStatus status = FSReadFile(__wut_devoptab_fs_client, &cmd, tmp, 1, size,
+                                   file->fd, 0, FS_ERROR_FLAG_ALL);
+
+      if (status < 0) {
+         if (bytesRead != 0) {
+            return bytesRead; // error after partial read
+         }
+
+         r->_errno = __wut_fs_translate_error(status);
          return -1;
       }
-      while (len > 0) {
-        size_t toRead = len > buffer_size ? buffer_size : len;
 
-        // Write the data
-        status = FSReadFile(__wut_devoptab_fs_client, &cmd, alignedReadBuffer, 1,
-                            toRead, file->fd, 0, FS_ERROR_FLAG_ALL);
-        if (status <= 0) {
-           break;
-        }
-
-        // Copy to internal buffer
-        bytes = (uint32_t)status;
-        memcpy(ptr, alignedReadBuffer, bytes);
-
-        file->offset += bytes;
-        bytesRead    += bytes;
-        ptr          += bytes;
-        len          -= bytes;
-
-        if (bytes < toRead) {
-           // If we did not read the full requested toRead bytes then we reached
-           // the end of the file.
-           break;
-        }
+      if (tmp == alignedBuffer) {
+         memcpy(ptr, alignedBuffer, status);
+      }
+
+      bytesRead += status;
+      ptr += status;
+
+      if (status != size) {
+         return bytesRead; // partial read
       }
-      free(alignedReadBuffer);
    }
 
-   // Return partial read
-   if (bytesRead > 0) {
-      return bytesRead;
-   }
-
-   if (status < 0) {
-      r->_errno = __wut_fs_translate_error(status);
-      return -1;
-   }
-
-   return 0;
-}
+   return bytesRead;
+}

libraries/wutdevoptab/devoptab_fs_write.c

@@ -1,83 +1,65 @@
 #include "devoptab_fs.h"
 
-ssize_t
-__wut_fs_write(struct _reent *r,
-               void *fd,
-               const char *ptr,
-               size_t len)
-{
-   FSStatus status = 0;
-   FSCmdBlock cmd;
-   uint8_t *alignedWriteBuffer;
-   uint32_t bytes, bytesWritten;
-   __wut_fs_file_t *file;
-
+ssize_t __wut_fs_write(struct _reent *r, void *fd, const char *ptr, size_t len) {
    if (!fd || !ptr) {
       r->_errno = EINVAL;
       return -1;
    }
 
-   FSInitCmdBlock(&cmd);
-   file = (__wut_fs_file_t *)fd;
-   bytesWritten = 0;
-
    // Check that the file was opened with write access
+   __wut_fs_file_t *file = (__wut_fs_file_t *) fd;
    if ((file->flags & O_ACCMODE) == O_RDONLY) {
       r->_errno = EBADF;
       return -1;
    }
 
-   if((((uintptr_t) ptr) & 0x3F) == 0){
-      status = FSWriteFile(__wut_devoptab_fs_client, &cmd, (uint8_t *) ptr,
-                           1, len, file->fd, 0, FS_ERROR_FLAG_ALL);  
-      if(status > 0){
-         bytesWritten = (uint32_t) status;
-         file->offset += bytesWritten;
+   // cache-aligned, cache-line-sized
+   __attribute__((aligned(0x40))) uint8_t alignedBuffer[0x40];
+
+   FSCmdBlock cmd;
+   FSInitCmdBlock(&cmd);
+
+   size_t bytesWritten = 0;
+   while (bytesWritten < len) {
+      // only use input buffer if cache-aligned and write size is a multiple of cache line size
+      // otherwise write from alignedBuffer
+      uint8_t *tmp = (uint8_t *) ptr;
+      size_t size = len - bytesWritten;
+
+      if ((uintptr_t) ptr & 0x3F) {
+         // write partial cache-line front-end
+         tmp = alignedBuffer;
+         size = MIN(size, 0x40 - ((uintptr_t) ptr & 0x3F));
+      } else if (size < 0x40) {
+         // write partial cache-line back-end
+         tmp = alignedBuffer;
+      } else {
+         // write whole cache lines
+         size &= ~0x3F;
       }
-   } else {
-      // Copy to internal buffer due to alignment requirement and read in chunks.
-      // Using a buffer smaller than 128KiB takes a performance hit.
-      int buffer_size = len < 128*1024 ? len : 128*1024;
-      alignedWriteBuffer = memalign(0x40, buffer_size);
-      if(!alignedWriteBuffer){
-         r->_errno = ENOMEM;
+
+      if (tmp == alignedBuffer) {
+         memcpy(tmp, ptr, size);
+      }
+
+      FSStatus status = FSWriteFile(__wut_devoptab_fs_client, &cmd, tmp, 1, size,
+                                    file->fd, 0, FS_ERROR_FLAG_ALL);
+      if (status < 0) {
+         if (bytesWritten != 0) {
+            return bytesWritten; // error after partial write
+         }
+
+         r->_errno = __wut_fs_translate_error(status);
          return -1;
       }
-      while (len > 0) {
-         size_t toWrite = len > buffer_size ? buffer_size : len;
 
-         // Copy to internal buffer
-         memcpy(alignedWriteBuffer, ptr, toWrite);
+      bytesWritten += status;
+      ptr += status;
 
-         // Write the data
-         status = FSWriteFile(__wut_devoptab_fs_client, &cmd, alignedWriteBuffer,
-                              1, toWrite, file->fd, 0, FS_ERROR_FLAG_ALL);
-         if (status <= 0) {
-            break;
-         }
-
-         bytes = (uint32_t)status;
-         file->offset += bytes;
-         bytesWritten += bytes;
-         ptr          += bytes;
-         len          -= bytes;
-
-         if (bytes < toWrite) {
-            break;
-         }
+      if (status != size) {
+         return bytesWritten; // partial write
       }
-      free(alignedWriteBuffer);
    }
 
-   // Return partial write
-   if (bytesWritten > 0) {
-      return bytesWritten;
-   }
-
-   if (status < 0) {
-      r->_errno = __wut_fs_translate_error(status);
-      return -1;
-   }
-
-   return 0;
+   return bytesWritten;
 }