Date: Wed, 25 Jun 2014 17:54:36 +0000 (UTC) From: Alexander Motin <mav@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r267877 - head/sys/cam/ctl Message-ID: <201406251754.s5PHsa83048099@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: mav Date: Wed Jun 25 17:54:36 2014 New Revision: 267877 URL: http://svnweb.freebsd.org/changeset/base/267877 Log: Lock devstat updates in block backend to make it usable. Polish lock names. MFC after: 2 weeks Sponsored by: iXsystems, Inc. Modified: head/sys/cam/ctl/ctl_backend_block.c head/sys/cam/ctl/ctl_backend_ramdisk.c Modified: head/sys/cam/ctl/ctl_backend_block.c ============================================================================== --- head/sys/cam/ctl/ctl_backend_block.c Wed Jun 25 17:34:04 2014 (r267876) +++ head/sys/cam/ctl/ctl_backend_block.c Wed Jun 25 17:54:36 2014 (r267877) @@ -160,7 +160,6 @@ struct ctl_be_block_lun { cbb_dispatch_t dispatch; cbb_dispatch_t lun_flush; cbb_dispatch_t unmap; - struct mtx lock; uma_zone_t lun_zone; uint64_t size_blocks; uint64_t size_bytes; @@ -179,6 +178,8 @@ struct ctl_be_block_lun { STAILQ_HEAD(, ctl_io_hdr) input_queue; STAILQ_HEAD(, ctl_io_hdr) config_write_queue; STAILQ_HEAD(, ctl_io_hdr) datamove_queue; + struct mtx_padalign io_lock; + struct mtx_padalign queue_lock; }; /* @@ -336,22 +337,7 @@ ctl_free_beio(struct ctl_be_block_io *be static void ctl_complete_beio(struct ctl_be_block_io *beio) { - union ctl_io *io; - int io_len; - - io = beio->io; - - if ((io->io_hdr.status & CTL_STATUS_MASK) == CTL_SUCCESS) - io_len = beio->io_len; - else - io_len = 0; - - devstat_end_transaction(beio->lun->disk_stats, - /*bytes*/ io_len, - beio->ds_tag_type, - beio->ds_trans_type, - /*now*/ NULL, - /*then*/&beio->ds_t0); + union ctl_io *io = beio->io; if (beio->beio_cont != NULL) { beio->beio_cont(beio); @@ -449,14 +435,14 @@ ctl_be_block_move_done(union ctl_io *io) * This move done routine is generally called in the SIM's * interrupt context, and therefore we cannot block. */ - mtx_lock(&be_lun->lock); + mtx_lock(&be_lun->queue_lock); /* * XXX KDM make sure that links is okay to use at this point. * Otherwise, we either need to add another field to ctl_io_hdr, * or deal with resource allocation here. */ STAILQ_INSERT_TAIL(&be_lun->datamove_queue, &io->io_hdr, links); - mtx_unlock(&be_lun->lock); + mtx_unlock(&be_lun->queue_lock); taskqueue_enqueue(be_lun->io_taskqueue, &be_lun->io_task); @@ -478,7 +464,7 @@ ctl_be_block_biodone(struct bio *bio) DPRINTF("entered\n"); error = bio->bio_error; - mtx_lock(&be_lun->lock); + mtx_lock(&be_lun->io_lock); if (error != 0) beio->num_errors++; @@ -496,7 +482,7 @@ ctl_be_block_biodone(struct bio *bio) */ if ((beio->send_complete == 0) || (beio->num_bios_done < beio->num_bios_sent)) { - mtx_unlock(&be_lun->lock); + mtx_unlock(&be_lun->io_lock); return; } @@ -504,7 +490,10 @@ ctl_be_block_biodone(struct bio *bio) * At this point, we've verified that we are the last I/O to * complete, so it's safe to drop the lock. */ - mtx_unlock(&be_lun->lock); + devstat_end_transaction(beio->lun->disk_stats, beio->io_len, + beio->ds_tag_type, beio->ds_trans_type, + /*now*/ NULL, /*then*/&beio->ds_t0); + mtx_unlock(&be_lun->io_lock); /* * If there are any errors from the backing device, we fail the @@ -546,15 +535,18 @@ static void ctl_be_block_flush_file(struct ctl_be_block_lun *be_lun, struct ctl_be_block_io *beio) { - union ctl_io *io; + union ctl_io *io = beio->io; struct mount *mountpoint; int error, lock_flags; DPRINTF("entered\n"); - io = beio->io; + binuptime(&beio->ds_t0); + mtx_lock(&be_lun->io_lock); + devstat_start_transaction(beio->lun->disk_stats, &beio->ds_t0); + mtx_unlock(&be_lun->io_lock); - (void) vn_start_write(be_lun->vn, &mountpoint, V_WAIT); + (void) vn_start_write(be_lun->vn, &mountpoint, V_WAIT); if (MNT_SHARED_WRITES(mountpoint) || ((mountpoint == NULL) @@ -565,14 +557,17 @@ ctl_be_block_flush_file(struct ctl_be_bl vn_lock(be_lun->vn, lock_flags | LK_RETRY); - binuptime(&beio->ds_t0); - devstat_start_transaction(beio->lun->disk_stats, &beio->ds_t0); - error = VOP_FSYNC(be_lun->vn, MNT_WAIT, curthread); VOP_UNLOCK(be_lun->vn, 0); vn_finished_write(mountpoint); + mtx_lock(&be_lun->io_lock); + devstat_end_transaction(beio->lun->disk_stats, beio->io_len, + beio->ds_tag_type, beio->ds_trans_type, + /*now*/ NULL, /*then*/&beio->ds_t0); + mtx_unlock(&be_lun->io_lock); + if (error == 0) ctl_set_success(&io->scsiio); else { @@ -627,12 +622,14 @@ ctl_be_block_dispatch_file(struct ctl_be xiovec->iov_len = beio->sg_segs[i].len; } + binuptime(&beio->ds_t0); + mtx_lock(&be_lun->io_lock); + devstat_start_transaction(beio->lun->disk_stats, &beio->ds_t0); + mtx_unlock(&be_lun->io_lock); + if (beio->bio_cmd == BIO_READ) { vn_lock(be_lun->vn, LK_SHARED | LK_RETRY); - binuptime(&beio->ds_t0); - devstat_start_transaction(beio->lun->disk_stats, &beio->ds_t0); - /* * UFS pays attention to IO_DIRECT for reads. If the * DIRECTIO option is configured into the kernel, it calls @@ -673,9 +670,6 @@ ctl_be_block_dispatch_file(struct ctl_be vn_lock(be_lun->vn, lock_flags | LK_RETRY); - binuptime(&beio->ds_t0); - devstat_start_transaction(beio->lun->disk_stats, &beio->ds_t0); - /* * UFS pays attention to IO_DIRECT for writes. The write * is done asynchronously. (Normally the write would just @@ -702,6 +696,12 @@ ctl_be_block_dispatch_file(struct ctl_be SDT_PROBE(cbb, kernel, write, file_done, 0, 0, 0, 0, 0); } + mtx_lock(&be_lun->io_lock); + devstat_end_transaction(beio->lun->disk_stats, beio->io_len, + beio->ds_tag_type, beio->ds_trans_type, + /*now*/ NULL, /*then*/&beio->ds_t0); + mtx_unlock(&be_lun->io_lock); + /* * If we got an error, set the sense data to "MEDIUM ERROR" and * return the I/O to the user. @@ -771,7 +771,9 @@ ctl_be_block_flush_dev(struct ctl_be_blo beio->send_complete = 1; binuptime(&beio->ds_t0); + mtx_lock(&be_lun->io_lock); devstat_start_transaction(be_lun->disk_stats, &beio->ds_t0); + mtx_unlock(&be_lun->io_lock); (*dev_data->csw->d_strategy)(bio); } @@ -802,11 +804,11 @@ ctl_be_block_unmap_dev_range(struct ctl_ off += bio->bio_length; len -= bio->bio_length; - mtx_lock(&be_lun->lock); + mtx_lock(&be_lun->io_lock); beio->num_bios_sent++; if (last && len == 0) beio->send_complete = 1; - mtx_unlock(&be_lun->lock); + mtx_unlock(&be_lun->io_lock); (*dev_data->csw->d_strategy)(bio); } @@ -828,7 +830,9 @@ ctl_be_block_unmap_dev(struct ctl_be_blo DPRINTF("entered\n"); binuptime(&beio->ds_t0); + mtx_lock(&be_lun->io_lock); devstat_start_transaction(be_lun->disk_stats, &beio->ds_t0); + mtx_unlock(&be_lun->io_lock); if (beio->io_offset == -1) { beio->io_len = 0; @@ -852,6 +856,7 @@ static void ctl_be_block_dispatch_dev(struct ctl_be_block_lun *be_lun, struct ctl_be_block_io *beio) { + TAILQ_HEAD(, bio) queue = TAILQ_HEAD_INITIALIZER(queue); int i; struct bio *bio; struct ctl_be_block_devdata *dev_data; @@ -872,14 +877,6 @@ ctl_be_block_dispatch_dev(struct ctl_be_ max_iosize = DFLTPHYS; cur_offset = beio->io_offset; - - /* - * XXX KDM need to accurately reflect the number of I/Os outstanding - * to a device. - */ - binuptime(&beio->ds_t0); - devstat_start_transaction(be_lun->disk_stats, &beio->ds_t0); - for (i = 0; i < beio->num_segs; i++) { size_t cur_size; uint8_t *cur_ptr; @@ -907,32 +904,23 @@ ctl_be_block_dispatch_dev(struct ctl_be_ cur_ptr += bio->bio_length; cur_size -= bio->bio_length; - /* - * Make sure we set the complete bit just before we - * issue the last bio so we don't wind up with a - * race. - * - * Use the LUN mutex here instead of a combination - * of atomic variables for simplicity. - * - * XXX KDM we could have a per-IO lock, but that - * would cause additional per-IO setup and teardown - * overhead. Hopefully there won't be too much - * contention on the LUN lock. - */ - mtx_lock(&be_lun->lock); - + TAILQ_INSERT_TAIL(&queue, bio, bio_queue); beio->num_bios_sent++; - - if ((i == beio->num_segs - 1) - && (cur_size == 0)) - beio->send_complete = 1; - - mtx_unlock(&be_lun->lock); - - (*dev_data->csw->d_strategy)(bio); } } + binuptime(&beio->ds_t0); + mtx_lock(&be_lun->io_lock); + devstat_start_transaction(be_lun->disk_stats, &beio->ds_t0); + beio->send_complete = 1; + mtx_unlock(&be_lun->io_lock); + + /* + * Fire off all allocated requests! + */ + while ((bio = TAILQ_FIRST(&queue)) != NULL) { + TAILQ_REMOVE(&queue, bio, bio_queue); + (*dev_data->csw->d_strategy)(bio); + } } static void @@ -1195,14 +1183,14 @@ ctl_be_block_next(struct ctl_be_block_io io->io_hdr.status &= ~CTL_STATUS_MASK; io->io_hdr.status |= CTL_STATUS_NONE; - mtx_lock(&be_lun->lock); + mtx_lock(&be_lun->queue_lock); /* * XXX KDM make sure that links is okay to use at this point. * Otherwise, we either need to add another field to ctl_io_hdr, * or deal with resource allocation here. */ STAILQ_INSERT_TAIL(&be_lun->input_queue, &io->io_hdr, links); - mtx_unlock(&be_lun->lock); + mtx_unlock(&be_lun->queue_lock); taskqueue_enqueue(be_lun->io_taskqueue, &be_lun->io_task); } @@ -1350,7 +1338,7 @@ ctl_be_block_worker(void *context, int p DPRINTF("entered\n"); - mtx_lock(&be_lun->lock); + mtx_lock(&be_lun->queue_lock); for (;;) { io = (union ctl_io *)STAILQ_FIRST(&be_lun->datamove_queue); if (io != NULL) { @@ -1361,13 +1349,13 @@ ctl_be_block_worker(void *context, int p STAILQ_REMOVE(&be_lun->datamove_queue, &io->io_hdr, ctl_io_hdr, links); - mtx_unlock(&be_lun->lock); + mtx_unlock(&be_lun->queue_lock); beio = (struct ctl_be_block_io *)PRIV(io)->ptr; be_lun->dispatch(be_lun, beio); - mtx_lock(&be_lun->lock); + mtx_lock(&be_lun->queue_lock); continue; } io = (union ctl_io *)STAILQ_FIRST(&be_lun->config_write_queue); @@ -1378,11 +1366,11 @@ ctl_be_block_worker(void *context, int p STAILQ_REMOVE(&be_lun->config_write_queue, &io->io_hdr, ctl_io_hdr, links); - mtx_unlock(&be_lun->lock); + mtx_unlock(&be_lun->queue_lock); ctl_be_block_cw_dispatch(be_lun, io); - mtx_lock(&be_lun->lock); + mtx_lock(&be_lun->queue_lock); continue; } io = (union ctl_io *)STAILQ_FIRST(&be_lun->input_queue); @@ -1391,7 +1379,7 @@ ctl_be_block_worker(void *context, int p STAILQ_REMOVE(&be_lun->input_queue, &io->io_hdr, ctl_io_hdr, links); - mtx_unlock(&be_lun->lock); + mtx_unlock(&be_lun->queue_lock); /* * We must drop the lock, since this routine and @@ -1399,7 +1387,7 @@ ctl_be_block_worker(void *context, int p */ ctl_be_block_dispatch(be_lun, io); - mtx_lock(&be_lun->lock); + mtx_lock(&be_lun->queue_lock); continue; } @@ -1409,7 +1397,7 @@ ctl_be_block_worker(void *context, int p */ break; } - mtx_unlock(&be_lun->lock); + mtx_unlock(&be_lun->queue_lock); } /* @@ -1437,14 +1425,14 @@ ctl_be_block_submit(union ctl_io *io) PRIV(io)->len = 0; - mtx_lock(&be_lun->lock); + mtx_lock(&be_lun->queue_lock); /* * XXX KDM make sure that links is okay to use at this point. * Otherwise, we either need to add another field to ctl_io_hdr, * or deal with resource allocation here. */ STAILQ_INSERT_TAIL(&be_lun->input_queue, &io->io_hdr, links); - mtx_unlock(&be_lun->lock); + mtx_unlock(&be_lun->queue_lock); taskqueue_enqueue(be_lun->io_taskqueue, &be_lun->io_task); return (CTL_RETVAL_COMPLETE); @@ -1868,7 +1856,8 @@ ctl_be_block_create(struct ctl_be_block_ STAILQ_INIT(&be_lun->config_write_queue); STAILQ_INIT(&be_lun->datamove_queue); sprintf(be_lun->lunname, "cblk%d", softc->num_luns); - mtx_init(&be_lun->lock, be_lun->lunname, NULL, MTX_DEF); + mtx_init(&be_lun->io_lock, "cblk io lock", NULL, MTX_DEF); + mtx_init(&be_lun->queue_lock, "cblk queue lock", NULL, MTX_DEF); ctl_init_opts(&be_lun->ctl_be_lun, req); be_lun->lun_zone = uma_zcreate(be_lun->lunname, CTLBLK_MAX_SEG, @@ -2115,7 +2104,8 @@ bailout_error: if (be_lun->lun_zone != NULL) uma_zdestroy(be_lun->lun_zone); ctl_free_opts(&be_lun->ctl_be_lun); - mtx_destroy(&be_lun->lock); + mtx_destroy(&be_lun->queue_lock); + mtx_destroy(&be_lun->io_lock); free(be_lun, M_CTLBLK); return (retval); @@ -2203,7 +2193,8 @@ ctl_be_block_rm(struct ctl_be_block_soft ctl_free_opts(&be_lun->ctl_be_lun); free(be_lun->dev_path, M_CTLBLK); - + mtx_destroy(&be_lun->queue_lock); + mtx_destroy(&be_lun->io_lock); free(be_lun, M_CTLBLK); req->status = CTL_LUN_OK; @@ -2450,10 +2441,10 @@ ctl_be_block_config_write(union ctl_io * * user asked to be synced out. When they issue a sync * cache command, we'll sync out the whole thing. */ - mtx_lock(&be_lun->lock); + mtx_lock(&be_lun->queue_lock); STAILQ_INSERT_TAIL(&be_lun->config_write_queue, &io->io_hdr, links); - mtx_unlock(&be_lun->lock); + mtx_unlock(&be_lun->queue_lock); taskqueue_enqueue(be_lun->io_taskqueue, &be_lun->io_task); break; case START_STOP_UNIT: { @@ -2544,7 +2535,7 @@ ctl_be_block_init(void) softc = &backend_block_softc; retval = 0; - mtx_init(&softc->lock, "ctlblk", NULL, MTX_DEF); + mtx_init(&softc->lock, "ctlblock", NULL, MTX_DEF); beio_zone = uma_zcreate("beio", sizeof(struct ctl_be_block_io), NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, 0); STAILQ_INIT(&softc->disk_list); Modified: head/sys/cam/ctl/ctl_backend_ramdisk.c ============================================================================== --- head/sys/cam/ctl/ctl_backend_ramdisk.c Wed Jun 25 17:34:04 2014 (r267876) +++ head/sys/cam/ctl/ctl_backend_ramdisk.c Wed Jun 25 17:54:36 2014 (r267877) @@ -84,7 +84,7 @@ struct ctl_be_ramdisk_lun { struct taskqueue *io_taskqueue; struct task io_task; STAILQ_HEAD(, ctl_io_hdr) cont_queue; - struct mtx lock; + struct mtx_padalign queue_lock; }; struct ctl_be_ramdisk_softc { @@ -150,7 +150,7 @@ ctl_backend_ramdisk_init(void) memset(softc, 0, sizeof(*softc)); - mtx_init(&softc->lock, "ramdisk", NULL, MTX_DEF); + mtx_init(&softc->lock, "ctlramdisk", NULL, MTX_DEF); STAILQ_INIT(&softc->lun_list); softc->rd_size = 1024 * 1024; @@ -242,10 +242,10 @@ ctl_backend_ramdisk_move_done(union ctl_ && ((io->io_hdr.flags & CTL_FLAG_ABORT) == 0) && ((io->io_hdr.status & CTL_STATUS_MASK) == CTL_STATUS_NONE)) { if (io->io_hdr.ctl_private[CTL_PRIV_BACKEND].integer > 0) { - mtx_lock(&be_lun->lock); + mtx_lock(&be_lun->queue_lock); STAILQ_INSERT_TAIL(&be_lun->cont_queue, &io->io_hdr, links); - mtx_unlock(&be_lun->lock); + mtx_unlock(&be_lun->queue_lock); taskqueue_enqueue(be_lun->io_taskqueue, &be_lun->io_task); return (0); @@ -350,18 +350,18 @@ ctl_backend_ramdisk_worker(void *context be_lun = (struct ctl_be_ramdisk_lun *)context; softc = be_lun->softc; - mtx_lock(&be_lun->lock); + mtx_lock(&be_lun->queue_lock); for (;;) { io = (union ctl_io *)STAILQ_FIRST(&be_lun->cont_queue); if (io != NULL) { STAILQ_REMOVE(&be_lun->cont_queue, &io->io_hdr, ctl_io_hdr, links); - mtx_unlock(&be_lun->lock); + mtx_unlock(&be_lun->queue_lock); ctl_backend_ramdisk_continue(io); - mtx_lock(&be_lun->lock); + mtx_lock(&be_lun->queue_lock); continue; } @@ -371,7 +371,7 @@ ctl_backend_ramdisk_worker(void *context */ break; } - mtx_unlock(&be_lun->lock); + mtx_unlock(&be_lun->queue_lock); } static int @@ -506,7 +506,7 @@ ctl_backend_ramdisk_rm(struct ctl_be_ram taskqueue_drain(be_lun->io_taskqueue, &be_lun->io_task); taskqueue_free(be_lun->io_taskqueue); ctl_free_opts(&be_lun->ctl_be_lun); - mtx_destroy(&be_lun->lock); + mtx_destroy(&be_lun->queue_lock); free(be_lun, M_RAMDISK); } @@ -639,7 +639,7 @@ ctl_backend_ramdisk_create(struct ctl_be } STAILQ_INIT(&be_lun->cont_queue); - mtx_init(&be_lun->lock, "CTL ramdisk", NULL, MTX_DEF); + mtx_init(&be_lun->queue_lock, "cram queue lock", NULL, MTX_DEF); TASK_INIT(&be_lun->io_task, /*priority*/0, ctl_backend_ramdisk_worker, be_lun); @@ -722,7 +722,7 @@ bailout_error: taskqueue_free(be_lun->io_taskqueue); } ctl_free_opts(&be_lun->ctl_be_lun); - mtx_destroy(&be_lun->lock); + mtx_destroy(&be_lun->queue_lock); free(be_lun, M_RAMDISK); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201406251754.s5PHsa83048099>