Re: [PATCH blktests 4/5] src/nvme-ghost-write-detector: add an ABA ghost write detector

[Mohamed Khalfella <mkhalfella@purestorage.com>]

On Mon 2026-09-21 14:38:12 -0400, Jesse Taube wrote:
> On Wed, Sep 16, 2026 at 10:09 PM Mohamed Khalfella
> <mkhalfella@purestorage.com> wrote:
> >
> > Consider a write X that is issued to an LBA and times out without ever
> > being acknowledged. The initiator retries it on another path, where it
> > lands. A write Y to the same LBA follows and lands as well. The original
> > X is still in flight somewhere in the fabric, and when it finally
> > reaches the device it overwrites Y. A read now returns X, a write the
> > initiator gave up on long ago.
> >
> > Add a program to catch this. It opens the device O_DIRECT and, on each
> > iteration, issues a series of writes of distinct byte patterns to the
> > same LBA, the 4k block at offset 0, waits, then reads that block back
> > and checks every byte holds the pattern written last. Because the
> > patterns differ, the value that comes back identifies which write
> > reappeared.
> >
> > Signed-off-by: Mohamed Khalfella <mkhalfella@purestorage.com>
> > ---
> >  src/.gitignore                  |  1 +
> >  src/Makefile                    |  1 +
> >  src/nvme-ghost-write-detector.c | 86 +++++++++++++++++++++++++++++++++
> >  3 files changed, 88 insertions(+)
> >  create mode 100644 src/nvme-ghost-write-detector.c
> >
> > diff --git a/src/.gitignore b/src/.gitignore
> > index e9869e1..9673be8 100644
> > --- a/src/.gitignore
> > +++ b/src/.gitignore
> > @@ -15,6 +15,7 @@
> >  /zbdioctl
> >  /miniublk
> >  /nvme-passthrough-meta
> > +/nvme-ghost-write-detector
> >  /ioctl-lbmd-query
> >  /nvme-passthru-admin-uring
> >  /nvme-delay-ioctl
> > diff --git a/src/Makefile b/src/Makefile
> > index dd64694..92b4d0c 100644
> > --- a/src/Makefile
> > +++ b/src/Makefile
> > @@ -24,6 +24,7 @@ C_TARGETS := \
> >         mount_clear_sock \
> >         nvme-delay-ioctl \
> >         nvme-passthrough-meta \
> > +       nvme-ghost-write-detector \
> >         ioctl-lbmd-query \
> >         nbdsetsize \
> >         openclose \
> > diff --git a/src/nvme-ghost-write-detector.c b/src/nvme-ghost-write-detector.c
> > new file mode 100644
> > index 0000000..bd42dde
> > --- /dev/null
> > +++ b/src/nvme-ghost-write-detector.c
> > @@ -0,0 +1,86 @@
> > +// SPDX-License-Identifier: GPL-3.0+
> > +// Copyright (C) 2026 Mohamed Khalfella
> > +
> > +#define _GNU_SOURCE
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <fcntl.h>
> > +#include <unistd.h>
> > +#include <string.h>
> > +#include <malloc.h>
> > +#include <errno.h>
> > +#include <libgen.h>
> 
> We don't need to include malloc.h

Okay, will drop it.

> > +
> > +#define BUF_SIZE       4096
> > +#define ITERATIONS     10
> > +#define DELAY          3               /* seconds delay between iterations */
> > +#define WRITE_COUNT    10
> > +
> > +#define WRITE_OFFSET   0
> > +#define READ_OFFSET    WRITE_OFFSET
> > +
> > +int main(int argc, char **argv)
> > +{
> > +       int fd, i, w, off, ret;
> > +       char *buff;
> > +
> 
> This is a bit pedantic but:
> 
> if (argc < 1)
>   return 1;

See below "if (argc < 2)". We need at least one argument the nvme device
to run IOs on. arg[0] is the program name, and arg[1] is the device name.

> 
> > +       fprintf(stdout, "starting %s test program\n", basename(argv[0]));
> > +
> > +       if (argc < 2) {
> > +               fprintf(stderr, "usage: %s /dev/nvmeXnY", argv[0]);
> > +               return 1;
> > +       }
> > +
> > +       fd = open(argv[1], O_RDWR | O_DIRECT);
> > +       if (fd < 0) {
> > +               fprintf(stderr, "failed to open device, errno = %d\n", errno);
> > +               return 1;
> > +       }
> > +
> > +       ret = posix_memalign((void **)&buff, BUF_SIZE, BUF_SIZE);
> > +       if (ret) {
> > +               fprintf(stderr, "failed to allocate buffer, ret = %d\n", ret);
> > +               goto out;
> > +       }
> > +
> > +       for (i = 0; i < ITERATIONS; i++) {
> > +               fprintf(stdout, "iteration number %d, writing data\n", i);
> > +
> > +               for (w = 0; w < WRITE_COUNT; w++) {
> > +                       memset(buff, w, BUF_SIZE);
> > +                       ret = pwrite(fd, buff, BUF_SIZE, WRITE_OFFSET);
> > +                       if (ret != BUF_SIZE) {
> > +                               fprintf(stderr, "failed to write buff, "
> > +                                               "ret = %d, errno = %d\n",
> > +                                               ret, errno);
> > +                               goto out;
> > +                       }
> > +               }
> > +
> > +               sleep(5);
> 
> There should be a macro for this.

Okay, will do that in next revision.

> 
> Thanks,
> Jesse Taube
> 
> > +               fprintf(stdout, "validating written data\n");
> > +
> > +               ret = pread(fd, buff, BUF_SIZE, READ_OFFSET);
> > +               if (ret != BUF_SIZE) {
> > +                       fprintf(stderr, "failed to read buff, "
> > +                                       "ret = %d, errno = %d\n",
> > +                                       ret, errno);
> > +                       goto out;
> > +               }
> > +
> > +               for (off = 0; off < BUF_SIZE; off++) {
> > +                       if (buff[off] != WRITE_COUNT - 1) {
> > +                               fprintf(stdout, "validation failed\n");
> > +                               goto out;
> > +                       }
> > +               }
> > +
> > +               fprintf(stdout, "successfully validated\n");
> > +               sleep(DELAY);
> > +       }
> > +
> > +out:
> > +       fprintf(stdout, "finished %s test program\n", basename(argv[0]));
> > +       close(fd);
> > +       return ret;
> > +}
> > --
> > 2.55.0
> >
>