(Failed) explicit cache flush experriment to try to fix Win tar problems with 00 bytes in text files
Posted by Anonymous on Fri 15th Nov 2024 09:51
raw | new post

diff --git a/daemon/readdir.c b/daemon/readdir.c
index a2d7772..a9a9ead 100644
--- a/daemon/readdir.c
+++ b/daemon/readdir.c
@@ -876,6 +876,8 @@ static int marshall_readdir(unsigned char *buffer, uint32_t *length, nfs41_upcal
     int status;
     readdir_upcall_args *args = &upcall->args.readdir;

+    flush_cpu_data_cache(args->kbuf, args->query_reply_len);
+
     status = safe_write(&buffer, length, &args->query_reply_len, sizeof(args->query_reply_len));
     return status;
 }
diff --git a/daemon/readwrite.c b/daemon/readwrite.c
index 14e4bcb..35d61e5 100644
--- a/daemon/readwrite.c
+++ b/daemon/readwrite.c
@@ -26,6 +26,7 @@
 #include "nfs41_ops.h"
 #include "name_cache.h"
 #include "upcall.h"
+#include "nfs41_driver.h" /* only for |NFS41_READ| */
 #include "daemon_debug.h"
 #include "util.h"

@@ -318,6 +319,11 @@ static int marshall_rw(unsigned char *buffer, uint32_t *length, nfs41_upcall *up
 {
     readwrite_upcall_args *args = &upcall->args.rw;
     int status;
+
+    if (upcall->opcode == NFS41_READ) {
+        flush_cpu_data_cache(args->buffer, args->out_len);
+    }
+
     status = safe_write(&buffer, length, &args->out_len, sizeof(args->out_len));
     if (status) goto out;
     status = safe_write(&buffer, length, &args->ctime, sizeof(args->ctime));
diff --git a/daemon/util.c b/daemon/util.c
index ef41a30..a476115 100644
--- a/daemon/util.c
+++ b/daemon/util.c
@@ -391,6 +391,32 @@ out:
     return status;
 }

+void flush_cpu_data_cache(void *p, size_t len)
+{
+#if 1
+#if defined(_M_IX86) || defined(_M_X64)
+
+/*
+ * FIXME: We should use |GetLogicalProcessorInformation()| to get the
+ * correct cache line size instead of guessing it here
+ */
+#if defined(_M_X64)
+#define MIN_DATACACHE_LINE_SIZE (64)
+#else
+#define MIN_DATACACHE_LINE_SIZE (32)
+#endif
+    char *buf = p;
+    size_t i;
+
+    for (i = 0 ; i < len ; i+=MIN_DATACACHE_LINE_SIZE) {
+        _mm_clflush(buf);
+        buf+=MIN_DATACACHE_LINE_SIZE;
+    }
+#else
+    /* FIXME! */
+#endif /* defined(_M_IX86) || defined(_M_X64) */
+#endif
+}

 /*
  * Like Win32 |popen()| but doesn't randomly fail or genrates EINVAL
diff --git a/daemon/util.h b/daemon/util.h
index 7a4e12a..e056936 100644
--- a/daemon/util.h
+++ b/daemon/util.h
@@ -351,6 +351,8 @@ __inline int valid_handle(HANDLE handle) {
     return handle != INVALID_HANDLE_VALUE && handle != 0;
 }

+void flush_cpu_data_cache(void *p, size_t len);
+
 typedef struct _subcmd_popen_context {
     HANDLE hReadPipe;
     HANDLE hWritePipe;
diff --git a/sys/nfs41sys_readwrite.c b/sys/nfs41sys_readwrite.c
index c039d23..f44559c 100644
--- a/sys/nfs41sys_readwrite.c
+++ b/sys/nfs41sys_readwrite.c
@@ -136,6 +136,25 @@ NTSTATUS marshal_nfs41_rw(
         status = STATUS_ACCESS_VIOLATION;
         goto out;
     }
+
+    if (entry->opcode == NFS41_WRITE) {
+        /* |NFS41_WRITE| - Flush CPU cache for the
+         * |MmMapLockedPagesSpecifyCache()| supplied buffer so the
+         * usermode nfsd daemon always sees the correct data.
+         * The |CacheType| parameter of
+         * |MmMapLockedPagesSpecifyCache()| above does NOT
+         * help, as the documentation states:
+         * "... The routine uses the CacheType parameter only if the
+         * pages that are described by the MDL do not already have a
+         * cache type associated with them. However, in nearly all
+         * cases, the pages already have an associated cache type,
+         * and this cache type is used by the new mapping..."
+         *
+         * So we explicitly flush this here.
+         */
+        KeInvalidateRangeAllCaches(entry->buf, entry->buf_len);
+    }
+
     RtlCopyMemory(tmp, &entry->buf, sizeof(HANDLE));
     *len = header_len;

diff --git a/tests/wintartests/wintartest_seq001.bash b/tests/wintartests/wintartest_seq001.bash
index 489d169..1f22858 100644
--- a/tests/wintartests/wintartest_seq001.bash
+++ b/tests/wintartests/wintartest_seq001.bash
@@ -11,19 +11,45 @@
 # Written by Roland Mainz <roland.mainz@nrubsig.org>
 #

-export PATH='/bin:/usr/bin'
+function test_wintar_seq
+{
+       set -o xtrace
+       set -o errexit
+       set -o nounset

+       # config
+       typeset use_bzip2=$1
+       typeset use_localdiskfortar=$2
+
+       # local vars
+       typeset tarfile_dir
+       typeset tarfilename
        typeset -i i
        typeset out
+       typeset -a testfiles
+       typeset currf

-set -o xtrace
-set -o errexit
+       # seq 1040 == 4093 bytes
+       # seq 1042 == 4103 bytes
+       for i in 1 100 1040 5000 10000 12000 ; do
+               rm -f -- "${i}seq.txt"
+               seq "$i" >"${i}seq.txt"
+               testfiles+=( "${i}seq.txt" )
+       done

-# Set umask=0000 to avoid permission trouble with SMB filesystems
-umask 0000
+       if "${use_localdiskfortar}" ; then
+               tarfile_dir='/tmp'
+       else
+               tarfile_dir="$PWD"
+       fi

-rm -f '10000seq.txt'
-seq 100000 >'10000seq.txt' ; tar -cvf - '10000seq.txt' >'10000seq.tar' #| pbzip2 -1 >'10000seq.tar.bz2'
+       if ${use_bzip2} ; then
+               tarfilename="${tarfile_dir}/test_seq.tar.bz2"
+               tar -cvf - "${testfiles[@]}" | pbzip2 -1 >"${tarfilename}"
+       else
+               tarfilename="${tarfile_dir}/test_seq.tar"
+               tar -cvf - "${testfiles[@]}" >"${tarfilename}"
+       fi

        rm -Rf 'tmp'
        mkdir 'tmp'
@@ -32,20 +58,33 @@ cd 'tmp'
        set +o xtrace

        for (( i=0 ; i < 2000 ; i++ )) ; do
-       printf '# Cycle %d:\n' "$i"
-       /cygdrive/c/Windows/system32/tar -xvf "$(cygpath -w '../10000seq.tar')"
-       out="$(od -x -v '10000seq.txt' | grep -F ' 0000' | head -n 5)"
+               printf '#### Test cycle %d (usingbzip=%s,tarfileonlocaldisk=%s):\n' "$i" "$use_bzip2" "$use_localdiskfortar"
+               /cygdrive/c/Windows/system32/tar -xvf "$(cygpath -w "${tarfilename}")"
+
+               for currf in "${testfiles[@]}" ; do
+                       out="$(od -t x1 -v "$currf" | grep -F ' 00' | head -n 5)"

                        if [[ "$out" != '' ]] ; then
-               printf '# ERROR: Sequence of zero bytes in plain /usr/bin/seq output found:\n'
+                               printf '# ERROR: Zero byte in plain /usr/bin/seq output %q found:\n' "$currf"
                                printf -- '---- snip ----\n%s\n---- snip ----\n' "$out"
-               exit 1
+                               return 1
                        fi
+               done

-       rm -f '10000seq.txt'
+               rm -f -- "${testfiles[@]}"
        done

        printf '# SUCCESS\n'

-exit 0
+       return 0
+}
+
+export PATH='/bin:/usr/bin'
+
+
+# Set umask=0000 to avoid permission trouble on SMB filesystems
+umask 0000
+
+test_wintar_seq true true
+
 # EOF.