Skip to content

fs_mark dummyfs support #68

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
adamdebek wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fs_mark dummyfs support #68

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
26295ff
build.sh: export CPPFLAGS taken from make to ports
adamdebek Jan 24, 2025
6c64d4e
fs_mark: enable on NOMMU architectures
adamdebek Jan 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions build.sh
View file
Open in desktop
Comment thread
adamdebek marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,12 @@ else
fi
export HOST_TARGET HOST

# use CFLAGS/LDFLAGS/STRIP taken from make
# use CFLAGS/CPPFLAGS/LDFLAGS/STRIP taken from make
CFLAGS="$EXPORT_CFLAGS"
CPPFLAGS="$EXPORT_CPPFLAGS"
LDFLAGS="$EXPORT_LDFLAGS"
STRIP="$EXPORT_STRIP"
export CFLAGS LDFLAGS STRIP
export CFLAGS CPPFLAGS LDFLAGS STRIP

export PORTS_MIRROR_BASEURL="https://files.phoesys.com/ports/"

Expand Down
41 changes: 24 additions & 17 deletions fs_mark/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,43 +2,50 @@

set -e

FS_MARK_VER="3.3"
FS_MARK=fs_mark-${FS_MARK_VER}
VER="3.3"
FS_MARK="fs_mark-$VER"
FS_MARK_COMMIT="2628be58146de63a13260ff64550f84275556c0e"
PKG_URL="https://github.com/josefbacik/fs_mark/archive/${FS_MARK_COMMIT}.tar.gz"
PKG_MIRROR_URL="https://files.phoesys.com/ports/${FS_MARK}.tar.gz"

@Darchiv Darchiv Feb 13, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few utility functions (and variables from build.sh) available for ports. Take a look:
https://github.com/phoenix-rtos/phoenix-rtos-ports/blob/7c6793d75d5b3e92e309a232c7d37ca893035aec/build.subr

Example usage:

phoenix-rtos-ports/lua/build.sh

Line 13 in 7c6793d

b_port_download "https://www.lua.org/ftp/" "${archive_filename}"

You can use them now if you want or leave that for later (all ports will be refactored some day).


PREFIX_FS_MARK_SRC="${PREFIX_PORT_BUILD}/${FS_MARK}"
PREFIX_FS_MARK_SRC="${PREFIX_PORT_BUILD}/src"
PREFIX_FS_MARK_MARKERS="${PREFIX_PORT_BUILD}/markers"

mkdir -p "$PREFIX_FS_MARK_MARKERS"

#
# Download and unpack
# # Download and unpack
#
mkdir -p "$PREFIX_PORT_BUILD" "$PREFIX_FS_MARK_MARKERS"
if ! [ -f "${PREFIX_PORT}/${FS_MARK}.tar.gz" ]; then
if ! wget "$PKG_URL" -O "${PREFIX_PORT}/${FS_MARK}.tar.gz" --no-check-certificate; then
wget "$PKG_MIRROR_URL" -P "${PREFIX_PORT}" --no-check-certificate
wget "$PKG_MIRROR_URL" -P "$PREFIX_PORT" --no-check-certificate
fi
fi

if ! [ -d "${PREFIX_FS_MARK_SRC}" ]; then
tar xzf "${PREFIX_PORT}/${FS_MARK}.tar.gz" -C "${PREFIX_PORT_BUILD}" && mv "${PREFIX_PORT_BUILD}"/fs_mark-"${FS_MARK_COMMIT}" "${PREFIX_PORT_BUILD}"/${FS_MARK}
if [ ! -d "$PREFIX_FS_MARK_SRC" ]; then
tar xzf "${PREFIX_PORT}/${FS_MARK}.tar.gz" -C "$PREFIX_PORT_BUILD"
mv "${PREFIX_PORT_BUILD}/fs_mark-$FS_MARK_COMMIT" "$PREFIX_FS_MARK_SRC"
fi

#
# Apply patches
# # Apply patches
#
for patchfile in "$PREFIX_PORT"/patch/*; do
if [ ! -f "$PREFIX_FS_MARK_MARKERS/$(basename "$patchfile").applied" ]; then
for patchfile in "${PREFIX_PORT}/patches/"*.patch; do
if [ ! -f "${PREFIX_FS_MARK_MARKERS}/$(basename "$patchfile").applied" ]; then
echo "applying patch: $patchfile"
patch -d "$PREFIX_FS_MARK_SRC" -p1 < "$patchfile"
touch "$PREFIX_FS_MARK_MARKERS/$(basename "$patchfile").applied"
touch "${PREFIX_FS_MARK_MARKERS}/$(basename "$patchfile").applied"
fi
done

# Build fs_mark
cd "${PREFIX_PORT_BUILD}/${FS_MARK}" && make
#
# # Build fs_mark
#
cd "$PREFIX_FS_MARK_SRC" && make

cp -a "$PREFIX_PORT_BUILD/${FS_MARK}/fs_mark" "$PREFIX_PROG/fs_mark"
$STRIP -o "${PREFIX_PROG_STRIPPED}/fs_mark" "${PREFIX_PROG}/fs_mark"
b_install "$PREFIX_PORTS_INSTALL/fs_mark" /bin
#
# # Strip and install
#
cp -a "${PREFIX_FS_MARK_SRC}/fs_mark" "${PREFIX_PROG}/fs_mark"
eval "$STRIP" -o "${PREFIX_PROG_STRIPPED}/fs_mark" "${PREFIX_PROG}/fs_mark"
b_install "${PREFIX_PROG_STRIPPED}/fs_mark" /bin
60 changes: 0 additions & 60 deletions fs_mark/patch/fs_mark.patch
View file
Open in desktop

This file was deleted.

10 changes: 0 additions & 10 deletions fs_mark/patch/fs_mark_h.patch
View file
Open in desktop

This file was deleted.

19 changes: 19 additions & 0 deletions fs_mark/patches/Makefile.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
--- fs_mark-3.3/Makefile 2025-01-02 19:10:48.757969200 +0100
+++ fs_mark-3.3.phoenix/Makefile 2025-01-02 19:10:48.757969200 +0100
@@ -14,14 +14,14 @@
CC ?= gcc
CFLAGS ?= -O2
LDFLAGS ?=
-CFLAGS += -Wall -D_FILE_OFFSET_BITS=64
+CFLAGS += -Wall -D_FILE_OFFSET_BITS=64 -z stack-size=8192

all: fs_mark

fs_mark.o: fs_mark.c fs_mark.h

fs_mark: fs_mark.o lib_timing.o
- $(CC) $(CFLAGS) -o fs_mark fs_mark.o lib_timing.o $(LDFLAGS)
+ $(CC) $(CPPFLAGS) $(CFLAGS) -o fs_mark fs_mark.o lib_timing.o $(LDFLAGS)

test: fs_mark
./fs_mark -d ${DIR1} -d ${DIR2} -s 51200 -n 4096
90 changes: 90 additions & 0 deletions fs_mark/patches/fs_mark.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
--- fs_mark-3.3/fs_mark.c 2025-01-02 19:10:48.757969200 +0100
+++ fs_mark-3.3.phoenix/fs_mark.c 2025-01-02 19:10:48.757969200 +0100
@@ -29,7 +29,7 @@
#include <sys/types.h>
#include <sys/wait.h>
#include <sys/stat.h>
-#include <sys/vfs.h>
+#include <sys/statvfs.h>
#include <sys/time.h>

#include <fcntl.h>
@@ -42,9 +42,8 @@
#include <ctype.h>
#include <time.h>

-#include <linux/types.h>
-#include <linux/limits.h>
-#include <linux/unistd.h>
+#include <limits.h>
+#include <signal.h>

#include "fs_mark.h"

@@ -493,12 +492,12 @@
*/
int get_df_full(char *dir_name)
{
- struct statfs fs_buf;
+ struct statvfs fs_buf;
float df_used, used_blocks;
int df_percent_used;

- if (statfs(dir_name, &fs_buf) == -1) {
- fprintf(stderr, "fs_mark: statfs failed on %s %s\n", dir_name,
+ if (statvfs(dir_name, &fs_buf) == -1) {
+ fprintf(stderr, "fs_mark: statvfs failed on %s %s\n", dir_name,
strerror(errno));
cleanup_exit();
}
@@ -517,11 +516,11 @@
*/
unsigned long long get_bytes_free(char *dir_name)
{
- struct statfs fs_buf;
+ struct statvfs fs_buf;
unsigned long long bytes_free;

- if (statfs(dir_name, &fs_buf) == -1) {
- fprintf(stderr, "fs_mark: statfs failed on %s %s\n", dir_name,
+ if (statvfs(dir_name, &fs_buf) == -1) {
+ fprintf(stderr, "fs_mark: statvfs failed on %s %s\n", dir_name,
strerror(errno));
cleanup_exit();
}
@@ -1115,6 +1114,10 @@

do_run(my_pid);

+#ifdef NOMMU
+ memset(names, 0, sizeof(struct name_entry) * num_files);

@Darchiv Darchiv Feb 13, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed on NOMMU targets? Doesn't the "work thread" exit after returning from thread_work()?

@adamdebek adamdebek Feb 14, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fs_mark can run multiple loops with each calling thread_work(), so I had to reinitialize this array back to zero's after each loop

+#endif
+
fclose(child_log_file_fp);
}

@@ -1137,8 +1140,15 @@
* Fork one thread for each of the specified children
*/

+#ifdef NOMMU
+ #define FORK vfork
+ file_count = 0;

@Darchiv Darchiv Feb 13, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is file_count = 0 needed on NOMMU targets?

@adamdebek adamdebek Feb 14, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

names and file_count are two variables that I need to reset after each loop, because NOMMU use vfork() to run thread_work()

+#else
+ #define FORK fork
+#endif
+
for (i = 0; i < num_threads; i++) {
- if ((child_tasks[i].child_pid = fork()) == -1) {
+ if ((child_tasks[i].child_pid = FORK()) == -1) {

@Darchiv Darchiv Feb 13, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using vfork() actually defeats the whole purpose of running fs_mark with multiple "child tasks", i.e. concurrent operations on files. vfork(), as opposed to fork(), causes the parent to suspend execution until its vforked child exits or execs, so every "child task" is executed sequentially.

I don't see any test commands (phoenix-rtos/phoenix-rtos-tests#233) to use more than one thread, so maybe this can be left unimplemented for now on NOMMU targets. fs_mark could use actual threads (via pthreads) instead of forking, but there are so many globals that refactoring would be quite cumbersome.

@adamdebek adamdebek Feb 14, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make it work with threads instead of processes on NOMMU can be a tedious task, so I wanted to make it run with just a 1 thread.

fprintf(stderr, "fs_mark: fork failed: %s\n",
strerror(errno));
cleanup_exit();
@@ -1262,6 +1272,7 @@
*/
df_full = get_df_full(child_tasks[0].test_dir);

+ usleep(100000);

@Darchiv Darchiv Feb 13, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed? Is this related to console output being interleaved with something else?

@adamdebek adamdebek Feb 14, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a problem with parts of logs being lost, so I used this sleep just for now to make it work.

if (verbose_stats)
fprintf(log_fp,
"%6u %12u %12u %12.1f %16llu %8llu %8llu %8llu %8llu %8llu %8llu %8llu %8llu %8llu %8llu %8llu %8llu %8llu %8llu %8llu %8llu %8llu %8llu\n",
26 changes: 26 additions & 0 deletions fs_mark/patches/fs_mark_h.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
--- fs_mark-3.3/fs_mark.h 2025-01-02 19:10:48.757969200 +0100
+++ fs_mark-3.3.phoenix/fs_mark.h 2025-01-02 19:10:48.757969200 +0100
@@ -29,6 +29,14 @@
#define FILENAME_SIZE (128) /* Max length of filenames */
#define MAX_STRING_SIZE (160) /* Max number of bytes in a string */

+#ifdef NOMMU
+ #undef MAX_IO_BUFFER_SIZE
+ #undef MAX_NAME_PATH
+
+ #define MAX_IO_BUFFER_SIZE (1024 * 32) /* Max write buffer size is 32KB */
+ #define MAX_NAME_PATH (200) /* Length of the pathname before the leaf */

@Darchiv Darchiv Feb 13, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that you are trying to avoid huge static allocations on low-RAM targets. NOMMU option itself doesn't imply that the target device has a tiny pool of RAM.

I think the most flexible solution is to use calloc(), as is the case with struct name_entry *names. The behavior would remain the same (use DEFAULT_IO_SIZE==(16 * 1024) bytes), but the problem of static allocation vanishes. Also, it seems to be a simple fix: change io_buffer into a pointer and replace memset(io_buffer, 0, io_buffer_size); with a call to calloc(). Freeing the memory is not of concern, because the author already ignores that part (it would be important if "worker threads" used threads instead of forked children).

When it comes to MAX_NAME_PATH and FILENAME_SIZE, you can use defines from <limits.h>: PATH_MAX and NAME_MAX. They are defined per-architecture and seem to be perfect for this use case. (See: libphoenix/include/arch/*/limits.h)

+#endif
+
/*
* Default values
*/
@@ -125,7 +133,7 @@
int num_threads = 1; /* Number of threads */
int do_fill_fs = 0; /* Run until the file system is full */
int verbose_stats = 0; /* Print complete stats for each system call */
-char log_file_name[PATH_MAX] = "fs_log.txt"; /* Log file name for run */
+char log_file_name[PATH_MAX - 16] = "fs_log.txt"; /* Log file name for run, reserve space for suffix */

@Darchiv Darchiv Feb 13, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You subtract 16 to accommodate for PID suffix appended in a few places in the source file? If so, please include that information in the comment to make it clear why it is changed (i.e. that fs_mark is flaky and you fix their own bugs) and note that there is
strncpy(log_file_name, optarg, PATH_MAX);
in -l option handling code which also has to be changed.

FILE *log_file_fp; /* Parent file pointer for log file */
FILE *child_log_file_fp; /* Child file pointer for log file */

4 changes: 2 additions & 2 deletions fs_mark/patch/lib_timing.patch → fs_mark/patches/lib_timing.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
--- fs_mark-3.3/lib_timing.c 2023-07-10 14:20:00.192932272 +0200
+++ fs_mark-3.3.phoenix/lib_timing.c 2023-07-10 14:20:25.032448754 +0200
--- fs_mark-3.3/lib_timing.c 2025-01-02 19:10:48.757969200 +0100
+++ fs_mark-3.3.phoenix/lib_timing.c 2025-01-02 19:10:48.757969200 +0100
@@ -24,7 +24,6 @@
#include <unistd.h>
#include <stdlib.h>
Expand Down