diff --git a/src/sys/dirctl.c b/src/sys/dirctl.c index fed067ff..f21e761f 100644 --- a/src/sys/dirctl.c +++ b/src/sys/dirctl.c @@ -88,7 +88,7 @@ static NTSTATUS FspFsvolQueryDirectoryCopy( Info->FileNameLength = FileName.Length;\ __VA_ARGS__\ Info = DestBuf;\ - *Info = InfoStruct;\ + RtlCopyMemory(Info, &InfoStruct, FIELD_OFFSET(TYPE, FileName));\ RtlCopyMemory(Info->FileName, DirInfo->FileNameBuf, FileName.Length);\ } while (0,0) #define FILL_INFO(TYPE, ...)\ @@ -367,6 +367,55 @@ static NTSTATUS FspFsvolQueryDirectoryRetry( PDEVICE_OBJECT FsvolDeviceObject, PIRP Irp, PIO_STACK_LOCATION IrpSp, BOOLEAN CanWait) { + /* + * The SystemBufferLength contains the length of the SystemBuffer that we + * are going to allocate (in FspFsvolQueryDirectoryBufferUserBuffer). This + * buffer is going to be used as the IRP SystemBuffer and it will also be + * mapped into the user mode file system process (in + * FspFsvolDirectoryControlPrepare) so that the file system can fill in the + * buffer. + * + * The SystemBufferLength is not the actual length that we are going to use + * when completing the IRP. This will be computed at IRP completion time + * (using FspFsvolQueryDirectoryCopy). Instead the SystemBufferLength is + * the size that we want the user mode file system to see and it may be + * different from the requested length for the following reasons: + * + * - If the FileInfoTimeout is non-zero, then the directory maintains a + * DirInfo meta cache that can be used to fulfill IRP requests without + * reaching out to user mode. In this case we want the SystemBufferLength + * to be FspFsvolDeviceDirInfoCacheItemSizeMax so that we read up to the + * cache size maximum. + * + * - If the requested DirectoryPattern (stored in FileDesc) is not the "*" + * (MatchAll) pattern, then we want to read as many entries as possible + * from the user mode file system to avoid multiple roundtrips to user + * mode when doing file name matching. In this case we set again the + * SystemBufferLength to be FspFsvolDeviceDirInfoCacheItemSizeMax. This + * is an important optimization and without it QueryDirectory is *very* + * slow without the DirInfo meta cache (i.e. when FileInfoTimeout is 0). + * + * - If the requsted DirectoryPattern is the MatchAll pattern then we set + * the SystemBufferLength to the requested (IRP) length as it is actually + * counter-productive to try to read more than we need. + * + * Note that we have made the decision not to forward the directory pattern + * to user mode and instead do all pattern matching in kernel mode. The + * primary reason for this decision was considerations about the DirInfo meta + * cache. If it is determined that this was a bad decision in the future, + * we can always start sending the directory pattern to user mode under some + * circumstances (e.g. when doing non-cached reads). + */ +#define GetSystemBufferLengthMaybeCached()\ + (0 != FsvolDeviceExtension->VolumeParams.FileInfoTimeout && 0 == FileDesc->DirectoryOffset) ||\ + FspFileDescDirectoryPatternMatchAll != FileDesc->DirectoryPattern.Buffer ?\ + FspFsvolDeviceDirInfoCacheItemSizeMax : Length +#define GetSystemBufferLengthNonCached()\ + FspFileDescDirectoryPatternMatchAll != FileDesc->DirectoryPattern.Buffer ?\ + FspFsvolDeviceDirInfoCacheItemSizeMax : Length +#define GetSystemBufferLengthBestGuess()\ + FspFsvolDeviceDirInfoCacheItemSizeMax + PAGED_CODE(); NTSTATUS Result; @@ -400,8 +449,7 @@ static NTSTATUS FspFsvolQueryDirectoryRetry( if (!Success) { if (0 == SystemBufferLength) - SystemBufferLength = 0 != FsvolDeviceExtension->VolumeParams.FileInfoTimeout ? - FspFsvolDeviceDirInfoCacheItemSizeMax : Length; /* best guess! */ + SystemBufferLength = GetSystemBufferLengthBestGuess(); Result = FspFsvolQueryDirectoryBufferUserBuffer( FsvolDeviceExtension, Irp, &SystemBufferLength); @@ -452,7 +500,7 @@ static NTSTATUS FspFsvolQueryDirectoryRetry( if (FspFileNodeReferenceDirInfo(FileNode, &DirInfoBuffer, &DirInfoSize)) { if (0 == SystemBufferLength) - SystemBufferLength = Length; + SystemBufferLength = GetSystemBufferLengthNonCached(); Result = FspFsvolQueryDirectoryCopyCache(FileDesc, IndexSpecified || RestartScan, @@ -471,8 +519,7 @@ static NTSTATUS FspFsvolQueryDirectoryRetry( else { if (0 == SystemBufferLength) - SystemBufferLength = 0 != FsvolDeviceExtension->VolumeParams.FileInfoTimeout ? - FspFsvolDeviceDirInfoCacheItemSizeMax : Length; + SystemBufferLength = GetSystemBufferLengthMaybeCached(); } FspFileNodeConvertExclusiveToShared(FileNode, Full); @@ -504,6 +551,10 @@ static NTSTATUS FspFsvolQueryDirectoryRetry( FspIopRequestContext(Request, RequestFileNode) = FileNode; return FSP_STATUS_IOQ_POST; + +#undef GetSystemBufferLengthBestGuess +#undef GetSystemBufferLengthNonCached +#undef GetSystemBufferLengthMaybeCached } static NTSTATUS FspFsvolQueryDirectory( diff --git a/tst/winfsp-tests/dirctl-test.c b/tst/winfsp-tests/dirctl-test.c index cdb07b20..d6ae38ed 100644 --- a/tst/winfsp-tests/dirctl-test.c +++ b/tst/winfsp-tests/dirctl-test.c @@ -52,6 +52,9 @@ static void querydir_dotest(ULONG Flags, PWSTR Prefix, ULONG FileInfoTimeout, UL ASSERT(Success); } + DWORD times[2]; + times[0] = GetTickCount(); + StringCbPrintfW(FilePath, sizeof FilePath, L"%s%s\\dir5\\*", Prefix ? L"" : L"\\\\?\\GLOBALROOT", Prefix ? Prefix : memfs_volumename(memfs)); Handle = FindFirstFileW(FilePath, &FindData); @@ -161,6 +164,10 @@ static void querydir_dotest(ULONG Flags, PWSTR Prefix, ULONG FileInfoTimeout, UL ASSERT(INVALID_HANDLE_VALUE == Handle); ASSERT(ERROR_FILE_NOT_FOUND == GetLastError()); + times[1] = GetTickCount(); + FspDebugLog(__FUNCTION__ "(Flags=%lx, Prefix=\"%S\", FileInfoTimeout=%ld, SleepTimeout=%ld): %ldms\n", + Flags, Prefix, FileInfoTimeout, SleepTimeout, times[1] - times[0]); + for (int j = 1; 100 >= j; j++) { StringCbPrintfW(FilePath, sizeof FilePath, L"%s%s\\dir5\\file%d",