Date: Tue, 01 Apr 2014 19:08:54 +1100 From: Kubilay Kocak <koobs.freebsd@gmail.com> To: Edward Tomasz Napierala <trasz@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r263978 - head/sys/cam/ctl Message-ID: <533A7416.7030605@FreeBSD.org> In-Reply-To: <201403312049.s2VKnXLr079029@svn.freebsd.org> References: <201403312049.s2VKnXLr079029@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 1/04/2014 7:49 AM, Edward Tomasz Napierala wrote: > Author: trasz > Date: Mon Mar 31 20:49:33 2014 > New Revision: 263978 > URL: http://svnweb.freebsd.org/changeset/base/263978 > > Log: > Make it possible to have multiple CTL worker threads. Leave the default > of 1 for now. > > Sponsored by: The FreeBSD Foundation > > Modified: > head/sys/cam/ctl/ctl.c > > Modified: head/sys/cam/ctl/ctl.c > ============================================================================== > --- head/sys/cam/ctl/ctl.c Mon Mar 31 19:58:08 2014 (r263977) > +++ head/sys/cam/ctl/ctl.c Mon Mar 31 20:49:33 2014 (r263978) > @@ -60,6 +60,7 @@ __FBSDID("$FreeBSD$"); > #include <sys/ioccom.h> > #include <sys/queue.h> > #include <sys/sbuf.h> > +#include <sys/smp.h> > #include <sys/endian.h> > #include <sys/sysctl.h> > > @@ -320,6 +321,10 @@ static int ctl_is_single = 1; > static int index_to_aps_page; > > SYSCTL_NODE(_kern_cam, OID_AUTO, ctl, CTLFLAG_RD, 0, "CAM Target Layer"); > +static int worker_threads = 1; > +TUNABLE_INT("kern.cam.ctl.worker_threads", &worker_threads); > +SYSCTL_INT(_kern_cam_ctl, OID_AUTO, worker_threads, CTLFLAG_RDTUN, > + &worker_threads, 1, "Number of worker threads"); > > /* > * Serial number (0x80), device id (0x83), and supported pages (0x00) > @@ -950,10 +955,7 @@ ctl_init(void) > struct ctl_frontend *fe; > struct ctl_lun *lun; > uint8_t sc_id =0; > -#if 0 > - int i; > -#endif > - int error, retval; > + int i, error, retval; > //int isc_retval; > > retval = 0; > @@ -1085,17 +1087,35 @@ ctl_init(void) > mtx_unlock(&softc->ctl_lock); > #endif > > - error = kproc_create(ctl_work_thread, softc, &softc->work_thread, 0, 0, > - "ctl_thrd"); > - if (error != 0) { > - printf("error creating CTL work thread!\n"); > - mtx_lock(&softc->ctl_lock); > - ctl_free_lun(lun); > - mtx_unlock(&softc->ctl_lock); > - ctl_pool_free(internal_pool); > - ctl_pool_free(emergency_pool); > - ctl_pool_free(other_pool); > - return (error); > + if (worker_threads > MAXCPU || worker_threads == 0) { > + printf("invalid kern.cam.ctl.worker_threads value; " > + "setting to 1"); > + worker_threads = 1; > + } else if (worker_threads < 0) { Why is it that the < 0 case is special enough that it gets the mp_ncpus check below, but the == 0 and > MAXCPU cases don't? > + if (mp_ncpus > 2) { > + /* > + * Using more than two worker threads actually hurts > + * performance due to lock contention. > + */ > + worker_threads = 2; > + } else { > + worker_threads = 1; > + } Are printf("Setting to N")'s worthwhile here as well given worker_threads is set to a value that wasn't specified by the user, as above? > + } > + > + for (i = 0; i < worker_threads; i++) { > + error = kproc_create(ctl_work_thread, softc, &softc->work_thread, 0, 0, > + "ctl_thrd%d", i); > + if (error != 0) { > + printf("error creating CTL work thread!\n"); > + mtx_lock(&softc->ctl_lock); > + ctl_free_lun(lun); > + mtx_unlock(&softc->ctl_lock); > + ctl_pool_free(internal_pool); > + ctl_pool_free(emergency_pool); > + ctl_pool_free(other_pool); > + return (error); > + } > } > if (bootverbose) > printf("ctl: CAM Target Layer loaded\n"); > @@ -12991,7 +13011,11 @@ ctl_work_thread(void *arg) > if (io != NULL) { > STAILQ_REMOVE_HEAD(&softc->rtr_queue, links); > mtx_unlock(&softc->ctl_lock); > - goto execute; > + retval = ctl_scsiio(&io->scsiio); > + if (retval != CTL_RETVAL_COMPLETE) > + CTL_DEBUG_PRINT(("ctl_scsiio failed\n")); > + mtx_lock(&softc->ctl_lock); > + continue; > } > } > io = (union ctl_io *)STAILQ_FIRST(&softc->incoming_queue); > @@ -13022,19 +13046,6 @@ ctl_work_thread(void *arg) > > /* Back to the top of the loop to see what woke us up. */ > continue; > - > -execute: > - retval = ctl_scsiio(&io->scsiio); > - switch (retval) { > - case CTL_RETVAL_COMPLETE: > - break; > - default: > - /* > - * Probably need to make sure this doesn't happen. > - */ > - break; > - } > - mtx_lock(&softc->ctl_lock); > } > } > > _______________________________________________ > svn-src-head@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/svn-src-head > To unsubscribe, send any mail to "svn-src-head-unsubscribe@freebsd.org" >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?533A7416.7030605>