Skip to content

Commit

Permalink
Don't register repair writes in the trim map.
Browse files Browse the repository at this point in the history 
The trim map inflight writes tree assumes non-conflicting writes, i.e.
that there will never be two simultaneous write I/Os to the same range
on the same vdev. This seemed like a sane assumption; however, in
actual testing, it appears that repair I/Os can very well conflict
with "normal" writes.

I'm not quite sure if these conflicting writes are supposed to happen
or not, but in the mean time, let's ignore repair writes for now. This
should be safe considering that, by definition, we never repair blocks
that are freed.
  • Loading branch information
@dechamps
dechamps committed Oct 5, 2012
1 parent 479b028 commit 6a3ceba
Showing 1 changed file with 9 additions and 2 deletions.
11 changes: 9 additions & 2 deletions module/zfs/zio.c
View file
Expand Up @@ -2537,7 +2537,13 @@ zio_vdev_io_start(zio_t *zio)
}
}

if (vd->vdev_ops->vdev_op_leaf && zio->io_type == ZIO_TYPE_WRITE) {
/*
* Note that we ignore repair writes for TRIM because they can conflict
* with normal writes. This isn't an issue because, by definition, we
* only repair blocks that aren't freed.
*/
if (vd->vdev_ops->vdev_op_leaf && zio->io_type == ZIO_TYPE_WRITE &&
!(zio->io_flags & ZIO_FLAG_IO_REPAIR)) {
if (!trim_map_write_start(zio))
return (ZIO_PIPELINE_STOP);
}
Expand All @@ -2559,7 +2565,8 @@ zio_vdev_io_done(zio_t *zio)
zio->io_type == ZIO_TYPE_WRITE || zio->io_type == ZIO_TYPE_FREE);

if (vd != NULL && vd->vdev_ops->vdev_op_leaf &&
zio->io_type == ZIO_TYPE_WRITE) {
zio->io_type == ZIO_TYPE_WRITE &&
!(zio->io_flags & ZIO_FLAG_IO_REPAIR)) {
trim_map_write_done(zio);
}

Expand Down

2 comments on commit 6a3ceba

@stevenh
Copy link

@stevenh stevenh commented on 6a3ceba Nov 3, 2012

Choose a reason for hiding this comment

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

Had a panic today on FreeBSD due to conflicting writes so I'm considering using write merges to fix this, what do you think:-

--- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/trim_map.c.orig      2012-11-02 17:33:05.931742609 +0000
+++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/trim_map.c   2012-11-02 18:11:37.389988197 +0000
@@ -287,6 +287,7 @@
 trim_map_write_start(zio_t *zio)
 {
        vdev_t *vd = zio->io_vd;
+       zio_t *zs;
        trim_map_t *tm = vd->vdev_trimmap;
        trim_seg_t tsearch, *ts;
        boolean_t left_over, right_over;
@@ -329,7 +330,15 @@
                        ts = avl_find(&tm->tm_queued_frees, &tsearch, NULL);
                } while (ts != NULL);
        }
-       avl_add(&tm->tm_inflight_writes, zio);
+
+       zs = avl_find(&tm->tm_inflight_writes, zio, NULL);
+       if (zs == NULL)
+               avl_add(&tm->tm_inflight_writes, zio);
+       else if (zio->io_offset < zs->io_offset) {
+               zs->io_size += zs->io_offset - zio->io_offset;
+               zs->io_offset = zio->io_offset;
+       } else
+               zs->io_size = zio->io_offset + zio->io_size - zs->io_offset;

        mutex_exit(&tm->tm_lock);

@ryao
Copy link

@ryao ryao commented on 6a3ceba Nov 15, 2012

Choose a reason for hiding this comment

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

Do you have any thought about why the writes conflicted?

Please sign in to comment.