Date: Sun, 09 Nov 2014 13:25:55 +0000 From: Steven Hartland <steven@multiplay.co.uk> To: Xin LI <delphij@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r274304 - in head: cddl/contrib/opensolaris/lib/libzpool/common sys/cddl/contrib/opensolaris/uts/common/fs/zfs sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys Message-ID: <545F6B63.1020601@freebsd.org> In-Reply-To: <201411090737.sA97b1iS008781@svn.freebsd.org> References: <201411090737.sA97b1iS008781@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
We need to keep an eye on this one as it replaces a number of
zio_interrupt calls with zio_execute which will increase stack usage.
Xin did you do any tests on i386 at all?
Regards
Steve
On 09/11/2014 07:37, Xin LI wrote:
> Author: delphij
> Date: Sun Nov 9 07:37:00 2014
> New Revision: 274304
> URL: https://svnweb.freebsd.org/changeset/base/274304
>
> Log:
> MFV r274272 and diff reduction with upstream.
>
> Illumos issue:
> 5244 zio pipeline callers should explicitly invoke next stage
>
> Tested with: ztest plus ZFS over GELI configuration
> MFC after: 1 month
>
> Modified:
> head/cddl/contrib/opensolaris/lib/libzpool/common/taskq.c
> head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/vdev_impl.h
> head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zio.h
> head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_disk.c
> head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_file.c
> head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
> head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_mirror.c
> head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_missing.c
> head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_raidz.c
> head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zio.c
> Directory Properties:
> head/cddl/contrib/opensolaris/ (props changed)
> head/sys/cddl/contrib/opensolaris/ (props changed)
>
> Modified: head/cddl/contrib/opensolaris/lib/libzpool/common/taskq.c
> ==============================================================================
> --- head/cddl/contrib/opensolaris/lib/libzpool/common/taskq.c Sun Nov 9 01:42:28 2014 (r274303)
> +++ head/cddl/contrib/opensolaris/lib/libzpool/common/taskq.c Sun Nov 9 07:37:00 2014 (r274304)
> @@ -25,6 +25,7 @@
> /*
> * Copyright 2011 Nexenta Systems, Inc. All rights reserved.
> * Copyright 2012 Garrett D'Amore <garrett@damore.org>. All rights reserved.
> + * Copyright (c) 2014 by Delphix. All rights reserved.
> */
>
> #include <sys/zfs_context.h>
> @@ -33,8 +34,10 @@ int taskq_now;
> taskq_t *system_taskq;
>
> #define TASKQ_ACTIVE 0x00010000
> +#define TASKQ_NAMELEN 31
>
> struct taskq {
> + char tq_name[TASKQ_NAMELEN + 1];
> kmutex_t tq_lock;
> krwlock_t tq_threadlock;
> kcondvar_t tq_dispatch_cv;
> @@ -247,6 +250,7 @@ taskq_create(const char *name, int nthre
> cv_init(&tq->tq_dispatch_cv, NULL, CV_DEFAULT, NULL);
> cv_init(&tq->tq_wait_cv, NULL, CV_DEFAULT, NULL);
> cv_init(&tq->tq_maxalloc_cv, NULL, CV_DEFAULT, NULL);
> + (void) strncpy(tq->tq_name, name, TASKQ_NAMELEN + 1);
> tq->tq_flags = flags | TASKQ_ACTIVE;
> tq->tq_active = nthreads;
> tq->tq_nthreads = nthreads;
>
> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/vdev_impl.h
> ==============================================================================
> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/vdev_impl.h Sun Nov 9 01:42:28 2014 (r274303)
> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/vdev_impl.h Sun Nov 9 07:37:00 2014 (r274304)
> @@ -60,7 +60,7 @@ typedef int vdev_open_func_t(vdev_t *vd,
> uint64_t *logical_ashift, uint64_t *physical_ashift);
> typedef void vdev_close_func_t(vdev_t *vd);
> typedef uint64_t vdev_asize_func_t(vdev_t *vd, uint64_t psize);
> -typedef int vdev_io_start_func_t(zio_t *zio);
> +typedef void vdev_io_start_func_t(zio_t *zio);
> typedef void vdev_io_done_func_t(zio_t *zio);
> typedef void vdev_state_change_func_t(vdev_t *vd, int, int);
> typedef void vdev_hold_func_t(vdev_t *vd);
>
> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zio.h
> ==============================================================================
> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zio.h Sun Nov 9 01:42:28 2014 (r274303)
> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zio.h Sun Nov 9 07:37:00 2014 (r274304)
> @@ -152,9 +152,6 @@ typedef enum zio_priority {
> ZIO_PRIORITY_NOW /* non-queued I/Os (e.g. ioctl) */
> } zio_priority_t;
>
> -#define ZIO_PIPELINE_CONTINUE 0x100
> -#define ZIO_PIPELINE_STOP 0x101
> -
> enum zio_flag {
> /*
> * Flags inherited by gang, ddt, and vdev children,
>
> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_disk.c
> ==============================================================================
> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_disk.c Sun Nov 9 01:42:28 2014 (r274303)
> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_disk.c Sun Nov 9 07:37:00 2014 (r274304)
> @@ -715,7 +715,7 @@ vdev_disk_ioctl_done(void *zio_arg, int
> zio_interrupt(zio);
> }
>
> -static int
> +static void
> vdev_disk_io_start(zio_t *zio)
> {
> vdev_t *vd = zio->io_vd;
> @@ -732,7 +732,7 @@ vdev_disk_io_start(zio_t *zio)
> if (dvd == NULL || (dvd->vd_ldi_offline && dvd->vd_lh == NULL)) {
> zio->io_error = SET_ERROR(ENXIO);
> zio_interrupt(zio);
> - return (ZIO_PIPELINE_STOP);
> + return;
> }
>
> if (zio->io_type == ZIO_TYPE_IOCTL) {
> @@ -740,7 +740,7 @@ vdev_disk_io_start(zio_t *zio)
> if (!vdev_readable(vd)) {
> zio->io_error = SET_ERROR(ENXIO);
> zio_interrupt(zio);
> - return (ZIO_PIPELINE_STOP);
> + return;
> }
>
> switch (zio->io_cmd) {
> @@ -771,7 +771,7 @@ vdev_disk_io_start(zio_t *zio)
> * and will call vdev_disk_ioctl_done()
> * upon completion.
> */
> - return (ZIO_PIPELINE_STOP);
> + return;
> }
>
> if (error == ENOTSUP || error == ENOTTY) {
> @@ -792,8 +792,8 @@ vdev_disk_io_start(zio_t *zio)
> zio->io_error = SET_ERROR(ENOTSUP);
> }
>
> - zio_interrupt(zio);
> - return (ZIO_PIPELINE_STOP);
> + zio_execute(zio);
> + return;
> }
>
> vb = kmem_alloc(sizeof (vdev_buf_t), KM_SLEEP);
> @@ -814,8 +814,6 @@ vdev_disk_io_start(zio_t *zio)
>
> /* ldi_strategy() will return non-zero only on programming errors */
> VERIFY(ldi_strategy(dvd->vd_lh, bp) == 0);
> -
> - return (ZIO_PIPELINE_STOP);
> }
>
> static void
>
> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_file.c
> ==============================================================================
> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_file.c Sun Nov 9 01:42:28 2014 (r274303)
> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_file.c Sun Nov 9 07:37:00 2014 (r274304)
> @@ -20,7 +20,7 @@
> */
> /*
> * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
> - * Copyright (c) 2013 by Delphix. All rights reserved.
> + * Copyright (c) 2011, 2014 by Delphix. All rights reserved.
> */
>
> #include <sys/zfs_context.h>
> @@ -154,7 +154,7 @@ vdev_file_close(vdev_t *vd)
> vd->vdev_tsd = NULL;
> }
>
> -static int
> +static void
> vdev_file_io_start(zio_t *zio)
> {
> vdev_t *vd = zio->io_vd;
> @@ -165,7 +165,7 @@ vdev_file_io_start(zio_t *zio)
> if (!vdev_readable(vd)) {
> zio->io_error = SET_ERROR(ENXIO);
> zio_interrupt(zio);
> - return (ZIO_PIPELINE_STOP);
> + return;
> }
>
> vf = vd->vdev_tsd;
> @@ -181,8 +181,8 @@ vdev_file_io_start(zio_t *zio)
> zio->io_error = SET_ERROR(ENOTSUP);
> }
>
> - zio_interrupt(zio);
> - return (ZIO_PIPELINE_STOP);
> + zio_execute(zio);
> + return;
> }
>
> zio->io_error = vn_rdwr(zio->io_type == ZIO_TYPE_READ ?
> @@ -194,7 +194,10 @@ vdev_file_io_start(zio_t *zio)
>
> zio_interrupt(zio);
>
> - return (ZIO_PIPELINE_STOP);
> +#ifdef illumos
> + VERIFY3U(taskq_dispatch(system_taskq, vdev_file_io_strategy, bp,
> + TQ_SLEEP), !=, 0);
> +#endif
> }
>
> /* ARGSUSED */
>
> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
> ==============================================================================
> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c Sun Nov 9 01:42:28 2014 (r274303)
> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c Sun Nov 9 07:37:00 2014 (r274304)
> @@ -788,7 +788,7 @@ vdev_geom_io_intr(struct bio *bp)
> zio_interrupt(zio);
> }
>
> -static int
> +static void
> vdev_geom_io_start(zio_t *zio)
> {
> vdev_t *vd;
> @@ -803,6 +803,8 @@ vdev_geom_io_start(zio_t *zio)
> /* XXPOLICY */
> if (!vdev_readable(vd)) {
> zio->io_error = SET_ERROR(ENXIO);
> + zio_interrupt(zio);
> + return;
> } else {
> switch (zio->io_cmd) {
> case DKIOCFLUSHWRITECACHE:
> @@ -818,23 +820,23 @@ vdev_geom_io_start(zio_t *zio)
> }
> }
>
> - zio_interrupt(zio);
> - return (ZIO_PIPELINE_STOP);
> + zio_execute(zio);
> + return;
> case ZIO_TYPE_FREE:
> if (vd->vdev_notrim) {
> zio->io_error = SET_ERROR(ENOTSUP);
> } else if (!vdev_geom_bio_delete_disable) {
> goto sendreq;
> }
> - zio_interrupt(zio);
> - return (ZIO_PIPELINE_STOP);
> + zio_execute(zio);
> + return;
> }
> sendreq:
> cp = vd->vdev_tsd;
> if (cp == NULL) {
> zio->io_error = SET_ERROR(ENXIO);
> zio_interrupt(zio);
> - return (ZIO_PIPELINE_STOP);
> + return;
> }
> bp = g_alloc_bio();
> bp->bio_caller1 = zio;
> @@ -863,8 +865,6 @@ sendreq:
> bp->bio_done = vdev_geom_io_intr;
>
> g_io_request(bp, cp);
> -
> - return (ZIO_PIPELINE_STOP);
> }
>
> static void
>
> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_mirror.c
> ==============================================================================
> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_mirror.c Sun Nov 9 01:42:28 2014 (r274303)
> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_mirror.c Sun Nov 9 07:37:00 2014 (r274304)
> @@ -24,7 +24,7 @@
> */
>
> /*
> - * Copyright (c) 2013 by Delphix. All rights reserved.
> + * Copyright (c) 2012, 2014 by Delphix. All rights reserved.
> */
>
> #include <sys/zfs_context.h>
> @@ -425,7 +425,7 @@ vdev_mirror_child_select(zio_t *zio)
> return (-1);
> }
>
> -static int
> +static void
> vdev_mirror_io_start(zio_t *zio)
> {
> mirror_map_t *mm;
> @@ -450,8 +450,8 @@ vdev_mirror_io_start(zio_t *zio)
> zio->io_type, zio->io_priority, 0,
> vdev_mirror_scrub_done, mc));
> }
> - zio_interrupt(zio);
> - return (ZIO_PIPELINE_STOP);
> + zio_execute(zio);
> + return;
> }
> /*
> * For normal reads just pick one child.
> @@ -478,8 +478,7 @@ vdev_mirror_io_start(zio_t *zio)
> c++;
> }
>
> - zio_interrupt(zio);
> - return (ZIO_PIPELINE_STOP);
> + zio_execute(zio);
> }
>
> static int
>
> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_missing.c
> ==============================================================================
> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_missing.c Sun Nov 9 01:42:28 2014 (r274303)
> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_missing.c Sun Nov 9 07:37:00 2014 (r274304)
> @@ -24,7 +24,7 @@
> */
>
> /*
> - * Copyright (c) 2013 by Delphix. All rights reserved.
> + * Copyright (c) 2012, 2014 by Delphix. All rights reserved.
> */
>
> /*
> @@ -67,12 +67,11 @@ vdev_missing_close(vdev_t *vd)
> }
>
> /* ARGSUSED */
> -static int
> +static void
> vdev_missing_io_start(zio_t *zio)
> {
> zio->io_error = SET_ERROR(ENOTSUP);
> - zio_interrupt(zio);
> - return (ZIO_PIPELINE_STOP);
> + zio_execute(zio);
> }
>
> /* ARGSUSED */
>
> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_raidz.c
> ==============================================================================
> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_raidz.c Sun Nov 9 01:42:28 2014 (r274303)
> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_raidz.c Sun Nov 9 07:37:00 2014 (r274304)
> @@ -21,7 +21,7 @@
>
> /*
> * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
> - * Copyright (c) 2013 by Delphix. All rights reserved.
> + * Copyright (c) 2012, 2014 by Delphix. All rights reserved.
> * Copyright (c) 2013, Joyent, Inc. All rights reserved.
> */
>
> @@ -1726,7 +1726,7 @@ vdev_raidz_child_done(zio_t *zio)
> * vdevs have had errors, then create zio read operations to the parity
> * columns' VDevs as well.
> */
> -static int
> +static void
> vdev_raidz_io_start(zio_t *zio)
> {
> vdev_t *vd = zio->io_vd;
> @@ -1756,8 +1756,8 @@ vdev_raidz_io_start(zio_t *zio)
> vdev_raidz_child_done, rc));
> }
>
> - zio_interrupt(zio);
> - return (ZIO_PIPELINE_STOP);
> + zio_execute(zio);
> + return;
> }
>
> if (zio->io_type == ZIO_TYPE_WRITE) {
> @@ -1789,8 +1789,8 @@ vdev_raidz_io_start(zio_t *zio)
> ZIO_FLAG_NODATA | ZIO_FLAG_OPTIONAL, NULL, NULL));
> }
>
> - zio_interrupt(zio);
> - return (ZIO_PIPELINE_STOP);
> + zio_execute(zio);
> + return;
> }
>
> ASSERT(zio->io_type == ZIO_TYPE_READ);
> @@ -1830,8 +1830,7 @@ vdev_raidz_io_start(zio_t *zio)
> }
> }
>
> - zio_interrupt(zio);
> - return (ZIO_PIPELINE_STOP);
> + zio_execute(zio);
> }
>
>
>
> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zio.c
> ==============================================================================
> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zio.c Sun Nov 9 01:42:28 2014 (r274303)
> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zio.c Sun Nov 9 07:37:00 2014 (r274304)
> @@ -90,6 +90,9 @@ kmem_cache_t *zio_data_buf_cache[SPA_MAX
> extern vmem_t *zio_alloc_arena;
> #endif
>
> +#define ZIO_PIPELINE_CONTINUE 0x100
> +#define ZIO_PIPELINE_STOP 0x101
> +
> /*
> * The following actions directly effect the spa's sync-to-convergence logic.
> * The values below define the sync pass when we start performing the action.
> @@ -2557,6 +2560,18 @@ zio_free_zil(spa_t *spa, uint64_t txg, b
> * Read, write and delete to physical devices
> * ==========================================================================
> */
> +
> +
> +/*
> + * Issue an I/O to the underlying vdev. Typically the issue pipeline
> + * stops after this stage and will resume upon I/O completion.
> + * However, there are instances where the vdev layer may need to
> + * continue the pipeline when an I/O was not issued. Since the I/O
> + * that was sent to the vdev layer might be different than the one
> + * currently active in the pipeline (see vdev_queue_io()), we explicitly
> + * force the underlying vdev layers to call either zio_execute() or
> + * zio_interrupt() to ensure that the pipeline continues with the correct I/O.
> + */
> static int
> zio_vdev_io_start(zio_t *zio)
> {
> @@ -2575,7 +2590,8 @@ zio_vdev_io_start(zio_t *zio)
> /*
> * The mirror_ops handle multiple DVAs in a single BP.
> */
> - return (vdev_mirror_ops.vdev_op_io_start(zio));
> + vdev_mirror_ops.vdev_op_io_start(zio);
> + return (ZIO_PIPELINE_STOP);
> }
>
> if (vd->vdev_ops->vdev_op_leaf && zio->io_type == ZIO_TYPE_FREE &&
> @@ -2589,7 +2605,7 @@ zio_vdev_io_start(zio_t *zio)
> * can quickly react to certain workloads. In particular, we care
> * about non-scrubbing, top-level reads and writes with the following
> * characteristics:
> - * - synchronous writes of user data to non-slog devices
> + * - synchronous writes of user data to non-slog devices
> * - any reads of user data
> * When these conditions are met, adjust the timestamp of spa_last_io
> * which allows the scan thread to adjust its workload accordingly.
> @@ -2693,10 +2709,8 @@ zio_vdev_io_start(zio_t *zio)
> return (ZIO_PIPELINE_STOP);
> }
>
> - ret = vd->vdev_ops->vdev_op_io_start(zio);
> - ASSERT(ret == ZIO_PIPELINE_STOP);
> -
> - return (ret);
> + vd->vdev_ops->vdev_op_io_start(zio);
> + return (ZIO_PIPELINE_STOP);
> }
>
> static int
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?545F6B63.1020601>