DAOS-16822 dfs: follow on improvement for dfs_sys_mkdir_p (#15569)

- improve performance when caching is disabled by not looking up full path each iteration.
- return ENOTDIR instead of EEXIST if a file exists instead of dir (if not at leaf).

Signed-off-by: Mohamad Chaarawi <[email protected]>

src/client/dfs/dfs_sys.c

@@ -11,14 +11,14 @@
 #include <linux/xattr.h>
 #include <daos/common.h>
 #include <daos/event.h>
-
 #include <gurt/atomic.h>
 
 #include <daos.h>
 #include <daos_fs.h>
-
 #include <daos_fs_sys.h>
 
+#include "dfs_internal.h"
+
 /** Number of entries for readdir */
 #define DFS_SYS_NUM_DIRENTS 24
 
@@ -1399,70 +1399,59 @@ dfs_sys_mkdir(dfs_sys_t *dfs_sys, const char *dir, mode_t mode,
 	return rc;
 }
 
-static int
-check_existing_dir(dfs_sys_t *dfs_sys, const char *dir_path)
-{
-	struct stat st = {0};
-	int         rc;
-
-	rc = dfs_sys_stat(dfs_sys, dir_path, 0, &st);
-	if (rc != 0) {
-		D_DEBUG(DB_TRACE, "failed to stat %s: (%d)\n", dir_path, rc);
-		return rc;
-	}
-
-	/* if it's not a directory, fail */
-	if (!S_ISDIR(st.st_mode))
-		return EEXIST;
-
-	/* if it is a directory, then it's not an error */
-	return 0;
-}
-
 int
 dfs_sys_mkdir_p(dfs_sys_t *dfs_sys, const char *dir_path, mode_t mode, daos_oclass_id_t cid)
 {
-	int   path_len = strnlen(dir_path, PATH_MAX);
-	char *_path    = NULL;
-	char *ptr      = NULL;
-	int   rc       = 0;
+	size_t     path_len;
+	char      *_path = NULL;
+	char      *sptr  = NULL;
+	char      *tok;
+	dfs_obj_t *parent;
+	int        rc = 0;
 
 	if (dfs_sys == NULL)
 		return EINVAL;
 	if (dir_path == NULL)
 		return EINVAL;
+	if (dir_path[0] != '/')
+		return EINVAL;
+
+	path_len = strnlen(dir_path, PATH_MAX);
 	if (path_len == PATH_MAX)
 		return ENAMETOOLONG;
 
 	D_STRNDUP(_path, dir_path, path_len);
 	if (_path == NULL)
 		return ENOMEM;
 
+	parent = NULL;
 	/* iterate through the parent directories and create them if necessary */
-	for (ptr = _path + 1; *ptr != '\0'; ptr++) {
-		if (*ptr != '/')
-			continue;
+	for (tok = strtok_r(_path, "/", &sptr); tok != NULL; tok = strtok_r(NULL, "/", &sptr)) {
+		dfs_obj_t *cur;
 
-		/* truncate the string here to create the parent */
-		*ptr = '\0';
-		rc   = dfs_sys_mkdir(dfs_sys, _path, mode, cid);
+		rc = dfs_open(dfs_sys->dfs, parent, tok, mode | S_IFDIR, O_RDWR | O_CREAT, cid, 0,
+			      NULL, &cur);
 		if (rc != 0) {
-			if (rc != EEXIST)
-				D_GOTO(out_free, rc);
-			rc = check_existing_dir(dfs_sys, _path);
-			if (rc != 0)
-				D_GOTO(out_free, rc);
+			/*
+			 * If this is the last entry and dfs_open returns ENOTDIR, change that err
+			 * code to EEXISTS to match what posix mkdir does.
+			 */
+			if (rc == ENOTDIR) {
+				tok = strtok_r(NULL, "/", &sptr);
+				if (tok == NULL)
+					rc = EEXIST;
+			}
+			D_GOTO(out_free, rc);
 		}
-		/* reset to keep going */
-		*ptr = '/';
-	}
 
-	/* create the final directory */
-	rc = dfs_sys_mkdir(dfs_sys, _path, mode, cid);
-	if (rc == EEXIST)
-		rc = check_existing_dir(dfs_sys, _path);
+		if (parent)
+			dfs_release(parent);
+		parent = cur;
+	}
 
 out_free:
+	if (parent)
+		dfs_release(parent);
 	D_FREE(_path);
 	return rc;
 }

src/tests/suite/dfs_sys_unit_test.c

@@ -848,6 +848,10 @@ dfs_sys_test_mkdir(void **state)
 	rc = dfs_sys_mkdir(dfs_sys_mt, file, S_IWUSR | S_IRUSR, 0);
 	assert_int_equal(rc, EEXIST);
 
+	rc = dfs_sys_remove(dfs_sys_mt, file, true, NULL);
+	assert_int_equal(rc, 0);
+	rc = dfs_sys_remove(dfs_sys_mt, child, true, NULL);
+	assert_int_equal(rc, 0);
 	rc = dfs_sys_remove(dfs_sys_mt, parent, true, NULL);
 	assert_int_equal(rc, 0);
 }
@@ -881,6 +885,10 @@ dfs_sys_test_mkdir_p(void **state)
 	rc = dfs_sys_mkdir_p(dfs_sys_mt, file, S_IWUSR | S_IRUSR, 0);
 	assert_int_equal(rc, EEXIST);
 
+	rc = dfs_sys_remove(dfs_sys_mt, file, true, NULL);
+	assert_int_equal(rc, 0);
+	rc = dfs_sys_remove(dfs_sys_mt, child, true, NULL);
+	assert_int_equal(rc, 0);
 	rc = dfs_sys_remove(dfs_sys_mt, parent, true, NULL);
 	assert_int_equal(rc, 0);
 }