fio_sem, diskutil: introduce fio_shared_sem and use it for diskutil lock

To report disk utilization statistics, fio allocates struct disk_util
objects for each disk of the I/O target files. These struct disk_util
objects are allocated via smalloc() from the fio shared memory, allowing
them to be allocated or freed by any fio process. The disk_util objects
are managed with the global list disk_list and are all freed by the fio
main process at the end of fio_backend() through
disk_util_prune_entries().

The struct disk_util contains the field struct fio_sem *lock which
points to the lock object. This object is allocated via mmap() with
MAP_SHARED flag. When allocated by parent process, this lock can be
shared across fio processes after forking child processes; however, the
parent process can not either access or free it when the lock object is
allocated by child processes. This lock object is also freed by the fio
main process alongside with the disk_util objects.

The commit c492cb1a9b1c ("iolog: fix disk stats issue") modified
init_iolog() to call init_disk_util(), which allocates the fio_sem lock
object for the struct disk_util. This commit enabled the disk
utilization report feature for the files recorded in the I/O replay
files. However, since the added init_disk_util() call is executed in
the child fio job processes, the disk_util object and the fio_sem lock
objects are allocated by the child processes. While allocation by child
process is acceptable for the disk_util object, it causes segmentation
faults when the fio_sem lock objects are freed at the end of the fio
main process.

The segmentation fault can be recreated by running two jobs: one job
does regular I/O to a system disk file. The other job replays I/O to
another disk. It can be triggered using the following command and the
files:

  $ sudo fio recreate.fio

  recreate.fio
  ============
  [dev_a]
  filename=test_file
  rw=read
  size=4096
  [dev_b]
  read_iolog=recreate.iolog

  recreate.iolog
  ==============
  fio version 3 iolog
  0 /dev/nullb0 add
  1 /dev/nullb0 open
  2 /dev/nullb0 read 0 4096
  3 /dev/nullb0 close

This fault happens only when fio jobs are handled as processes. When
thread=1 option is specified, fio jobs are threads and reside within
single memory space, then the fault is not observed.

To prevent the segmentation fault, allocate the fio_sem lock object not
by mmap() but by smalloc(). This ensures the fio main process can free
the fio_sem lock objects along with the disk_util objects. To achieve
this, introduce two new helper functions, fio_shared_sem_init() and
fio_shared_sem_remove(). These functions behave exactly same as the
existing functions fio_sem_init() and fio_sem_remove() except the memory
allocation method.

Do not implement the new functions in the existing source file
fio_sem.c, because it implements fio_sem_init() and fio_sem_remove()
which are used for smalloc() implementation. If the two new functions
were implemented in fio_sem.c, it would create circular references and
cause build failures. Instead, add a new source file fio_shared_sem.c to
implement the new functions.

Fixes: c492cb1a9b1c ("iolog: fix disk stats issue")
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Link: https://lore.kernel.org/r/20250513031837.74780-1-shinichiro.kawasaki@wdc.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/Makefile b/Makefile
index 173378fa44954f6a5e19092e92fa81e9e93114f3..f3a80882045fd14e3268f56241d8b0d22ddc0e59 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -53,7 +53,7 @@ SOURCE :=     $(sort $(patsubst $(SRCDIR)/%,%,$(wildcard $(SRCDIR)/crc/*.c)) \
                $(patsubst $(SRCDIR)/%,%,$(wildcard $(SRCDIR)/lib/*.c))) \
                gettime.c ioengines.c init.c stat.c log.c time.c filesetup.c \
                eta.c verify.c memory.c io_u.c parse.c fio_sem.c rwlock.c \
-               pshared.c options.c \
+               pshared.c options.c fio_shared_sem.c \
                smalloc.c filehash.c profile.c debug.c engines/cpu.c \
                engines/mmap.c engines/sync.c engines/null.c engines/net.c \
                engines/ftruncate.c engines/fileoperations.c \

diff --git a/diskutil.c b/diskutil.c
index 69b3dd263f7b11b83b544c394e3d7920150540a7..f018015cb7f4aa309610c27d784df3b0762d0ac2 100644 (file)
--- a/diskutil.c
+++ b/diskutil.c
@@ -38,7 +38,7 @@ static void disk_util_free(struct disk_util *du)
                slave->users--;
        }
 
-       fio_sem_remove(du->lock);
+       fio_shared_sem_remove(du->lock);
        free(du->sysfs_root);
        sfree(du);
 }
@@ -327,7 +327,7 @@ static struct disk_util *disk_util_add(struct thread_data *td, int majdev,
        du->minor = mindev;
        INIT_FLIST_HEAD(&du->slavelist);
        INIT_FLIST_HEAD(&du->slaves);
-       du->lock = fio_sem_init(FIO_SEM_UNLOCKED);
+       du->lock = fio_shared_sem_init(FIO_SEM_UNLOCKED);
        du->users = 0;
 
        fio_sem_down(disk_util_sem);

diff --git a/fio_sem.h b/fio_sem.h
index a796ddd74d7dda4cb328c68efce4b3f2705a8895..a06f6eb79286d456d81061f03bba37d351982cdf 100644 (file)
--- a/fio_sem.h
+++ b/fio_sem.h
@@ -21,8 +21,10 @@ enum {
 
 extern int __fio_sem_init(struct fio_sem *, int);
 extern struct fio_sem *fio_sem_init(int);
+extern struct fio_sem *fio_shared_sem_init(int);
 extern void __fio_sem_remove(struct fio_sem *);
 extern void fio_sem_remove(struct fio_sem *);
+extern void fio_shared_sem_remove(struct fio_sem *);
 extern void fio_sem_up(struct fio_sem *);
 extern void fio_sem_down(struct fio_sem *);
 extern bool fio_sem_down_trylock(struct fio_sem *);

diff --git a/fio_shared_sem.c b/fio_shared_sem.c
new file mode 100644 (file)
index 0000000..bc26bbe
--- /dev/null
+++ b/fio_shared_sem.c
@@ -0,0 +1,42 @@
+/*
+ * Separate out the two helper functions for fio_sem from "fio_sem.c".
+ * These two functions depend on fio shared memory. Other fio_sem
+ * functions in "fio_sem.c" are used for fio shared memory. This file
+ * separation is required to avoid build failures caused by circular
+ * dependency.
+ */
+
+#include <stdio.h>
+
+#include "fio_sem.h"
+#include "smalloc.h"
+
+/*
+ * Allocate and initialize fio_sem lock object in the same manner as
+ * fio_sem_init(), except the lock object is allocated from the fio
+ * shared memory. This allows the parent process to free the lock
+ * allocated by child processes.
+ */
+struct fio_sem *fio_shared_sem_init(int value)
+{
+       struct fio_sem *sem;
+
+       sem = smalloc(sizeof(struct fio_sem));
+       if (!sem)
+               return NULL;
+
+       if (!__fio_sem_init(sem, value))
+               return sem;
+
+       fio_shared_sem_remove(sem);
+       return NULL;
+}
+
+/*
+ * Free the fio_sem lock object allocated by fio_shared_sem_init().
+ */
+void fio_shared_sem_remove(struct fio_sem *sem)
+{
+       __fio_sem_remove(sem);
+       sfree(sem);
+}