userlandfs-fuse: improve fuse_ll_readdir

The previous implementation needed a temporary buffer to store entries
and then sent them to the actual readdir buffer. This is now fixed, the
actual buffer is filled directly.

Also fixes problems with reading the same directory multiple times, and
reading large directories.

Change-Id: I8dc9677ee676144547d17f313a7f2d91fd2bca05
Reviewed-on: https://review.haiku-os.org/c/haiku/+/5480
Tested-by: Commit checker robot <no-reply+buildbot@haiku-os.org>
Reviewed-by: waddlesplash <waddlesplash@gmail.com>

src/add-ons/kernel/file_systems/userlandfs/server/fuse/FUSELowLevel.cpp

@@ -6,11 +6,16 @@
 #include "FUSELowLevel.h"
 
 #include <assert.h>
+#include <dirent.h>
 #include <stdio.h>
 #include <string.h>
 
 #include <Errors.h>
 
+#define ROUNDDOWN(a, b) (((a) / (b)) * (b))
+#define ROUNDUP(a, b)   ROUNDDOWN((a) + (b) - 1, b)
+
+
 
 // Reimplement fuse_req in our own way. In libfuse, the requests are communicated to the kernel,
 // but in our case, they remain entirely inside userlandfs_server. This means we can use a much
@@ -38,7 +43,8 @@ struct fuse_req {
 
 	sem_t fSyncSem;
 	ssize_t fReplyResult;
-	fuse_fill_dir_t fRequestFiller; // callback function to fill buffer for directory reads
+
+	ReadDirBufferFiller fRequestFiller;
 	void* fRequestCookie;
 
 	// The reply can contain various things, depending on which function was called
@@ -127,7 +133,6 @@ fuse_ll_readlink(const fuse_lowlevel_ops* ops, fuse_ino_t ino, char* buffer, siz
 
 	fuse_req request;
 	request.fReplyBuf = buffer;
-	request.fReplyResult = size;
 	ops->readlink(&request, ino);
 	request.Wait();
 	return request.fReplyResult;
@@ -319,25 +324,21 @@ fuse_ll_opendir(const fuse_lowlevel_ops* ops, fuse_ino_t inode, struct fuse_file
 
 
 int
-fuse_ll_readdir(const fuse_lowlevel_ops* ops, fuse_ino_t ino, void* cookie, fuse_fill_dir_t filler,
-	off_t pos, fuse_file_info* ffi)
+fuse_ll_readdir(const fuse_lowlevel_ops* ops, fuse_ino_t ino, void* cookie, char* buffer,
+	size_t bufferSize, ReadDirBufferFiller filler, off_t pos, fuse_file_info* ffi)
 {
 	if (ops->readdir == NULL)
 		return B_NOT_SUPPORTED;
 
 	fuse_req request;
+	request.fReplyBuf = buffer;
+
 	request.fRequestFiller = filler;
 	request.fRequestCookie = cookie;
-	request.fReplyBuf = NULL;
 
-	do {
-		request.fReplyResult = 0;
-		ops->readdir(&request, ino, UINT32_MAX, pos, ffi);
-		request.Wait();
-		if (request.fReplyResult > 0)
-			pos += request.fReplyResult;
-	} while (request.fReplyResult > 0);
+	ops->readdir(&request, ino, bufferSize, pos, ffi);
 
+	request.Wait();
 	return request.fReplyResult;
 }
 
@@ -377,7 +378,6 @@ fuse_ll_getxattr(const fuse_lowlevel_ops* ops, fuse_ino_t ino, const char *name,
 		return B_NOT_SUPPORTED;
 
 	fuse_req request;
-	request.fReplyResult = size;
 	request.fReplyBuf = buffer;
 	ops->getxattr(&request, ino, name, size);
 	request.Wait();
@@ -392,7 +392,6 @@ fuse_ll_listxattr(const fuse_lowlevel_ops* ops, fuse_ino_t ino, char* buffer, si
 		return B_NOT_SUPPORTED;
 
 	fuse_req request;
-	request.fReplyResult = size;
 	request.fReplyBuf = (char*)buffer;
 	ops->listxattr(&request, ino, size);
 	request.Wait();
@@ -472,9 +471,11 @@ fuse_reply_open(fuse_req_t req, const struct fuse_file_info* f)
 int
 fuse_reply_buf(fuse_req_t req, const char *buf, size_t size)
 {
-	if (req->fReplyBuf)
+	if (req->fReplyBuf && req->fReplyBuf != buf)
 		memcpy(req->fReplyBuf, buf, size);
+
 	req->fReplyResult = size;
+
 	req->Notify();
 	return 0;
 }
@@ -517,18 +518,20 @@ fuse_reply_write(fuse_req_t req, size_t count)
 }
 
 
+// return: size of the entry (no matter if it was added to the buffer or not)
+// params: pointer to where to store the entry, size
 size_t fuse_add_direntry(fuse_req_t req, char *buf, size_t bufsize, const char *name,
 	const struct stat *stbuf, off_t off)
 {
-	// Special case where the client wants to know how much space an entry will use (it's always
-	// 1 slot in our case)
-	if ((buf == NULL) && (bufsize == 0))
-		return 1;
+	size_t entryLen = offsetof(struct dirent, d_name) + strlen(name) + 1;
+	// align the entry length, so the next dirent will be aligned
+	entryLen = ROUNDUP(entryLen, 8);
 
-	int ret = req->fRequestFiller(req->fRequestCookie, name, stbuf, off);
-	if (ret != 0)
-		return 0;
-	return 1;
+	if (stbuf != NULL) {
+		req->fRequestFiller(req->fRequestCookie, buf, bufsize, name, stbuf, off);
+	}
+
+	return entryLen;
 }
 
 

src/add-ons/kernel/file_systems/userlandfs/server/fuse/FUSELowLevel.h

@@ -12,6 +12,9 @@
 #include "fuse_api.h"
 
 
+typedef	int	(*ReadDirBufferFiller) (void* buffer, char* buf, size_t bufsize, const char* name,
+	const struct stat* st, off_t offset);
+
 void fuse_ll_init(const fuse_lowlevel_ops* ops, void* userdata, struct fuse_conn_info* conn);
 void fuse_ll_destroy(const fuse_lowlevel_ops* ops, void *userdata);
 int fuse_ll_lookup(const fuse_lowlevel_ops* ops, fuse_ino_t parent, const char *name,
@@ -41,7 +44,7 @@ int fuse_ll_fsync(const fuse_lowlevel_ops* ops, fuse_ino_t ino, int datasync,
 	struct fuse_file_info *fi);
 int fuse_ll_opendir(const fuse_lowlevel_ops* ops, fuse_ino_t inode, struct fuse_file_info* ffi);
 int fuse_ll_readdir(const fuse_lowlevel_ops* ops, fuse_ino_t ino, void* cookie,
-	fuse_fill_dir_t filler, off_t pos, fuse_file_info* ffi);
+	char* buffer, size_t bufferSize, ReadDirBufferFiller filler, off_t pos, fuse_file_info* ffi);
 int fuse_ll_releasedir(const fuse_lowlevel_ops* ops, fuse_ino_t ino, struct fuse_file_info *fi);
 int fuse_ll_statfs(const fuse_lowlevel_ops* ops, fuse_ino_t inode, struct statvfs* stat);
 int fuse_ll_getxattr(const fuse_lowlevel_ops* ops, fuse_ino_t ino, const char *name,

src/add-ons/kernel/file_systems/userlandfs/server/fuse/FUSEVolume.cpp

@@ -2135,8 +2135,7 @@ FUSEVolume::ReadDir(void* _node, void* _cookie, void* buffer, size_t bufferSize,
 
 		locker.Lock();
 
-		ReadDirBuffer readDirBuffer(this, node, cookie, buffer, bufferSize,
-			count);
+		ReadDirBuffer readDirBuffer(this, node, cookie, buffer, bufferSize, count);
 		off_t offset = cookie->currentEntryOffset;
 
 		// read the dir
@@ -2145,8 +2144,28 @@ FUSEVolume::ReadDir(void* _node, void* _cookie, void* buffer, size_t bufferSize,
 			locker.Unlock();
 
 			// TODO pass the cookie from opendir here instead of NULL
-			fuseError = fuse_ll_readdir(fOps, node->id, &readDirBuffer, &_AddReadDirEntry,
-				offset, NULL);
+			fuseError = fuse_ll_readdir(fOps, node->id, &readDirBuffer, (char*)buffer, bufferSize,
+				&_AddReadDirEntryLowLevel, offset, NULL);
+
+			// The request filler may or may not be used. If the filesystem decides that it has
+			// already cached the directory, it can reply with an already filled buffer from a
+			// previous run. So, we can't rely on any updates done to the cookie by that function.
+			// So we need to check the number of entries in the buffer, and advance the
+			// currentEntryOffset.
+			if (fuseError > 0) {
+				struct dirent* dirent = (struct dirent*)buffer;
+				countRead = 0;
+				while ((char*)dirent + dirent->d_reclen <= (char*)buffer + fuseError) {
+					countRead++;
+					dirent = (struct dirent*)(((char*)dirent) + dirent->d_reclen);
+					if (dirent->d_reclen == 0)
+						break;
+				}
+				cookie->currentEntryOffset += (char*)dirent - (char*)buffer;
+
+				fuseError = 0;
+			}
+			readDirError = 0;
 		} else {
 			// get a path for the node
 			char path[B_PATH_NAME_LENGTH];
@@ -2166,6 +2185,9 @@ PRINT(("  using readdir() interface\n"));
 				fuseError = fuse_fs_readdir(fFS, path, &readDirBuffer,
 					&_AddReadDirEntry, offset, cookie);
 			}
+
+			countRead = readDirBuffer.entriesRead;
+			readDirError = readDirBuffer.error;
 		}
 
 		if (fuseError != 0)
@@ -2173,8 +2195,6 @@ PRINT(("  using readdir() interface\n"));
 
 		locker.Lock();
 
-		countRead = readDirBuffer.entriesRead;
-		readDirError = readDirBuffer.error;
 	}
 
 	if (cookie->entryCache != NULL) {
@@ -3165,6 +3185,18 @@ FUSEVolume::_BuildPath(FUSENode* node, char* path, size_t& pathLen)
 }
 
 
+/*static*/ int
+FUSEVolume::_AddReadDirEntryLowLevel(void* _buffer, char* buf, size_t bufsize, const char* name,
+	const struct stat* st, off_t offset)
+{
+	ReadDirBuffer* buffer = (ReadDirBuffer*)_buffer;
+
+	ino_t nodeID = st != NULL ? st->st_ino : 0;
+	int type = st != NULL ? st->st_mode & S_IFMT : 0;
+	return buffer->volume->_AddReadDirEntryLowLevel(buffer, buf, bufsize, name, type, nodeID, offset);
+}
+
+
 /*static*/ int
 FUSEVolume::_AddReadDirEntry(void* _buffer, const char* name,
 	const struct stat* st, off_t offset)
@@ -3182,14 +3214,135 @@ FUSEVolume::_AddReadDirEntryGetDir(fuse_dirh_t handle, const char* name,
 	int type, ino_t nodeID)
 {
 	ReadDirBuffer* buffer = (ReadDirBuffer*)handle;
-	return buffer->volume->_AddReadDirEntry(buffer, name, type << 12, nodeID,
-		0);
+	return buffer->volume->_AddReadDirEntry(buffer, name, type << 12, nodeID, 0);
 }
 
 
 int
-FUSEVolume::_AddReadDirEntry(ReadDirBuffer* buffer, const char* name, int type,
-	ino_t nodeID, off_t offset)
+FUSEVolume::_AddReadDirEntryLowLevel(ReadDirBuffer* buffer, char* buf, size_t bufsize, const char* name,
+	int type, ino_t nodeID, off_t offset)
+{
+	PRINT(("FUSEVolume::_AddReadDirEntryLowLevel(%p, \"%s\", %#x, %" B_PRId64 ", %"
+		B_PRId64 "\n", buffer, name, type, nodeID, offset));
+
+	AutoLocker<Locker> locker(fLock);
+
+	size_t entryLen = 0;
+
+	// create a node and an entry, if necessary
+	ino_t dirID = buffer->directory->id;
+	FUSEEntry* entry;
+	if (strcmp(name, ".") == 0) {
+		// current dir entry
+		nodeID = dirID;
+		type = S_IFDIR;
+	} else if (strcmp(name, "..") == 0) {
+		// parent dir entry
+		FUSEEntry* parentEntry = buffer->directory->entries.Head();
+		if (parentEntry == NULL) {
+			ERROR(("FUSEVolume::_AddReadDirEntry(): dir %" B_PRId64
+				" has no entry!\n", dirID));
+			return 0;
+		}
+		nodeID = parentEntry->parent->id;
+		type = S_IFDIR;
+	} else if ((entry = fEntries.Lookup(FUSEEntryRef(dirID, name))) == NULL) {
+		// get the node
+		FUSENode* node = NULL;
+		if (fUseNodeIDs)
+			node = fNodes.Lookup(nodeID);
+		else
+			nodeID = _GenerateNodeID();
+
+		if (node == NULL) {
+			// no node yet -- create one
+
+			// If we don't have a valid type, we need to stat the node first.
+			if (type == 0) {
+				struct stat st;
+				int fuseError;
+				if (fOps != NULL) {
+					fuseError = fuse_ll_getattr(fOps, node->id, &st);
+				} else {
+					char path[B_PATH_NAME_LENGTH];
+					size_t pathLen;
+					status_t error = _BuildPath(buffer->directory, name, path,
+							pathLen);
+					if (error != B_OK) {
+						buffer->error = error;
+						return 0;
+					}
+
+					locker.Unlock();
+
+					// stat the path
+					fuseError = fuse_fs_getattr(fFS, path, &st);
+				}
+
+				locker.Lock();
+
+				if (fuseError != 0) {
+					buffer->error = fuseError;
+					return 0;
+				}
+
+				type = st.st_mode & S_IFMT;
+			}
+
+			node = new(std::nothrow) FUSENode(nodeID, type);
+			if (node == NULL) {
+				buffer->error = B_NO_MEMORY;
+				return 1;
+			}
+			PRINT(("  -> create node: %p, id: %" B_PRId64 "\n", node, nodeID));
+
+			fNodes.Insert(node);
+		} else {
+			// get a node reference for the entry
+			node->refCount++;
+		}
+
+		// create the entry
+		entry = FUSEEntry::Create(buffer->directory, name, node);
+		if (entry == NULL) {
+			_PutNode(node);
+			buffer->error = B_NO_MEMORY;
+			return 1;
+		}
+
+		buffer->directory->refCount++;
+			// dir reference for the entry
+
+		fEntries.Insert(entry);
+		node->entries.Add(entry);
+	} else {
+		// TODO: Check whether the node's ID matches the one we got (if any)!
+		nodeID = entry->node->id;
+		type = entry->node->type;
+	}
+
+	// fill in the dirent
+	dirent* dirEntry = (dirent*)(buf);
+	dirEntry->d_dev = fID;
+	dirEntry->d_ino = nodeID;
+	strcpy(dirEntry->d_name, name);
+
+	// align the entry length, so the next dirent will be aligned
+	entryLen = offsetof(struct dirent, d_name) + strlen(name) + 1;
+	entryLen = ROUNDUP(entryLen, 8);
+
+	dirEntry->d_reclen = entryLen;
+
+	// update the buffer
+	buffer->usedSize += entryLen;
+
+	return 0;
+}
+
+
+int
+FUSEVolume::_AddReadDirEntry(ReadDirBuffer* buffer, const char* name,
+	int type, ino_t nodeID, off_t offset)
 {
 	PRINT(("FUSEVolume::_AddReadDirEntry(%p, \"%s\", %#x, %" B_PRId64 ", %"
 		B_PRId64 "\n", buffer, name, type, nodeID, offset));

src/add-ons/kernel/file_systems/userlandfs/server/fuse/FUSEVolume.h

@@ -196,10 +196,15 @@ private:
 			status_t			_BuildPath(FUSENode* node, char* path,
 									size_t& pathLen);
 
+	static	int					_AddReadDirEntryLowLevel(void* buffer, char* buf, size_t bufsize,
+									const char* name, const struct stat* st, off_t offset);
 	static	int					_AddReadDirEntry(void* buffer, const char* name,
 									const struct stat* st, off_t offset);
 	static	int					_AddReadDirEntryGetDir(fuse_dirh_t handle,
 									const char* name, int type, ino_t nodeID);
+			int					_AddReadDirEntryLowLevel(ReadDirBuffer* buffer,
+									char* buf, size_t bufSize, const char* name, int type,
+									ino_t nodeID, off_t offset);
 			int					_AddReadDirEntry(ReadDirBuffer* buffer,
 									const char* name, int type, ino_t nodeID,
 									off_t offset);