Fix aio completion vs unwritten extents

Tristan,

The reflink test is missing aio bits. Please could you add it. In short, a pread issued after an aio write, must return that data. This should be tested for both direct and buffered ios.

Similarly, fill_holes also needs to be enhanced.

Thanks
Sunil

-------- Original Message --------
Subject: [PATCH 0/2] Fix aio completion vs unwritten extents
Date: Tue, 22 Jun 2010 08:21:44 -0400
From: Christoph Hellwig <[email protected]>
To: linux-fsdevel at vger.kernel.org, xfs at oss.sgi.com,
linux-ext4 at vger.kernel.org

Some filesystems (XFS and ext4) have support for a concept called unwritten extents, where we can write data into holes / preallocated space and only mark them as allocated when the data I/O has finished.

Because the transaction to convert the extent can't be submitted from I/O completion, which normally happens from IRQ context it needs to be defered to a workqueue. This is not a problem for buffered I/O where we keep the data in cache at least until the I/O operation has finished, but it is an issue for direct I/O. XFS avoids that problem for synchronous direct I/O by waiting for all unwritten extent conversions to finish if we did one during direct I/O, but so far has ignored the problem for asynchronous I/O. Unfortunately the race is very easy to hit by using QEMU with native AIO support on a sparse image, and the result is filesystem corruption in the guest.

This contains core direct I/O changes to allow the filesystem to delay AIO completion, as well as a patch to fix XFS. ext4 also has the same issue, and from a quick look also doesn't properly complete unwritten extent conversions for synchronous direct I/O, but I'll leave that for someone more familar to figure out.

Below is a minimal reproducer for the issue. Given that we're dealing with a race condition it doesn't always fail, but in 2 core laptop it triggers 100% reproducibly in 20 runs in a loop.

---

#define _GNU_SOURCE

#include <sys/stat.h>
#include <sys/types.h>
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

#include <libaio.h>

#define BUF_SIZE 4096
#define IO_PATTERN 0xab

int main(int argc, char *argv[])

{
	struct io_context *ctx = NULL;
	struct io_event ev;
	struct iocb iocb, *iocbs[] = { &iocb };
	void *buf;
	char cmp_buf[BUF_SIZE];
	int fd, err = 0;

	fd = open(argv[1], O_DIRECT | O_CREAT | O_TRUNC | O_RDWR, 0600);
	if (fd == -1) {
		perror("open");
		return 1;
	}

	err = posix_memalign(&buf, BUF_SIZE, BUF_SIZE);
	if (err) {
		fprintf(stderr, "error %s during %s\n",
			strerror(-err),
			"posix_memalign");
		return 1;
	}
	memset(buf, IO_PATTERN, BUF_SIZE);
	memset(cmp_buf, IO_PATTERN, BUF_SIZE);

	/*
	 * Truncate to some random large file size.  Just make sure
	 * it's not smaller than our I/O size.
	 */
	if (ftruncate(fd, 1024 * 1024 * 1024) < 0) {
		perror("ftruncate");
		return 1;
	}


	/*
	 * Do a simple 4k write into a hole using aio.
	 */
	err = io_setup(1, &ctx);
	if (err) {
		fprintf(stderr, "error %s during %s\n",
			strerror(-err),
			"io_setup");
		return 1;
	}

	io_prep_pwrite(&iocb, fd, buf, BUF_SIZE, 0);

	err = io_submit(ctx, 1, iocbs);
	if (err != 1) {
		fprintf(stderr, "error %s during %s\n",
			strerror(-err),
			"io_submit");
		return 1;
	}

	err = io_getevents(ctx, 1, 1, &ev, NULL);
	if (err != 1) {
		fprintf(stderr, "error %s during %s\n",
			strerror(-err),
			"io_getevents");
		return 1;
	}

	/*
	 * And then read it back.
	 *
	 * Using pread to keep it simple, but AIO has the same effect.
	 */
	if (pread(fd, buf, BUF_SIZE, 0) != BUF_SIZE) {
		perror("pread");
		return 1;
	}

	/*
	 * And depending on the machine we'll just get zeroes back quite
	 * often here.  That's because the unwritten extent conversion
	 * hasn't finished.
	 */
	if (memcmp(buf, cmp_buf, BUF_SIZE)) {
		unsigned long long *ubuf = (unsigned long long *)buf;
		int i;

		for (i = 0; i < BUF_SIZE / sizeof(unsigned long long); i++)
			printf("%d: 0x%llx\n", i, ubuf[i]);
			
		return 1;
	}

	return 0;
}

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 7 lines · Sunil Mushran

Tristan,

The reflink test is missing aio bits. Please could you add it. In short, a pread issued after an aio write, must return that data. This should be tested for both direct and buffered ios.

Similarly, fill_holes also needs to be enhanced.

Gotta, verifications for odirect and buffered ios have already been included in current tests I guess.

What's more, maybe we can also verify the data from aio_read() after pwrite() completed.

Tristan.

Quoted 155 lines

Thanks
Sunil

-------- Original Message --------

Subject:    [PATCH 0/2] Fix aio completion vs unwritten extents
Date:    Tue, 22 Jun 2010 08:21:44 -0400
From:    Christoph Hellwig <[email protected]>
To:    linux-fsdevel at vger.kernel.org, xfs at oss.sgi.com, linux-ext4 at vger.kernel.org

Some filesystems (XFS and ext4) have support for a concept called unwritten extents, where we can write data into holes / preallocated space and only mark them as allocated when the data I/O has finished.

Because the transaction to convert the extent can't be submitted from I/O completion, which normally happens from IRQ context it needs to be defered to a workqueue. This is not a problem for buffered I/O where we keep the data in cache at least until the I/O operation has finished, but it is an issue for direct I/O. XFS avoids that problem for synchronous direct I/O by waiting for all unwritten extent conversions
to finish if we did one during direct I/O, but so far has ignored the problem for asynchronous I/O. Unfortunately the race is very easy to hit by using QEMU with native AIO support on a sparse image, and the result is filesystem corruption in the guest.

This contains core direct I/O changes to allow the filesystem to delay
AIO completion, as well as a patch to fix XFS. ext4 also has the same
issue, and from a quick look also doesn't properly complete unwritten extent conversions for synchronous direct I/O, but I'll leave that for someone more familar to figure out.

Below is a minimal reproducer for the issue. Given that we're dealing
with a race condition it doesn't always fail, but in 2 core laptop it triggers 100% reproducibly in 20 runs in a loop.

---

#define _GNU_SOURCE

#include <sys/stat.h>
#include <sys/types.h>
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

#include <libaio.h>

#define BUF_SIZE 4096
#define IO_PATTERN 0xab

int main(int argc, char *argv[])

{
   struct io_context *ctx = NULL;
   struct io_event ev;
   struct iocb iocb, *iocbs[] = { &iocb };
   void *buf;
   char cmp_buf[BUF_SIZE];
   int fd, err = 0;

   fd = open(argv[1], O_DIRECT | O_CREAT | O_TRUNC | O_RDWR, 0600);
   if (fd == -1) {
       perror("open");
       return 1;
   }

   err = posix_memalign(&buf, BUF_SIZE, BUF_SIZE);
   if (err) {
       fprintf(stderr, "error %s during %s\n",
           strerror(-err),
           "posix_memalign");
       return 1;
   }
   memset(buf, IO_PATTERN, BUF_SIZE);
   memset(cmp_buf, IO_PATTERN, BUF_SIZE);

   /*
    * Truncate to some random large file size.  Just make sure
    * it's not smaller than our I/O size.
    */
   if (ftruncate(fd, 1024 * 1024 * 1024) < 0) {
       perror("ftruncate");
       return 1;
   }


   /*
    * Do a simple 4k write into a hole using aio.
    */
   err = io_setup(1, &ctx);
   if (err) {
       fprintf(stderr, "error %s during %s\n",
           strerror(-err),
           "io_setup");
       return 1;
   }

   io_prep_pwrite(&iocb, fd, buf, BUF_SIZE, 0);

   err = io_submit(ctx, 1, iocbs);
   if (err != 1) {
       fprintf(stderr, "error %s during %s\n",
           strerror(-err),
           "io_submit");
       return 1;
   }

   err = io_getevents(ctx, 1, 1, &ev, NULL);
   if (err != 1) {
       fprintf(stderr, "error %s during %s\n",
           strerror(-err),
           "io_getevents");
       return 1;
   }

   /*
    * And then read it back.
    *
    * Using pread to keep it simple, but AIO has the same effect.
    */
   if (pread(fd, buf, BUF_SIZE, 0) != BUF_SIZE) {
       perror("pread");
       return 1;
   }

   /*
    * And depending on the machine we'll just get zeroes back quite
    * often here.  That's because the unwritten extent conversion
    * hasn't finished.
    */
   if (memcmp(buf, cmp_buf, BUF_SIZE)) {
       unsigned long long *ubuf = (unsigned long long *)buf;
       int i;

       for (i = 0; i < BUF_SIZE / sizeof(unsigned long long); i++)
           printf("%d: 0x%llx\n", i, ubuf[i]);

       return 1;
   }

   return 0;
}

--
To unsubscribe from this list: send the line "unsubscribe linux- fsdevel" in
the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html