Date: Tue, 1 Apr 2014 10:43:50 +0200 From: =?iso-8859-2?Q?Edward_Tomasz_Napiera=B3a?= <trasz@FreeBSD.org> To: koobs@FreeBSD.org Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r263978 - head/sys/cam/ctl Message-ID: <AD63ABB8-AF48-4378-ABBA-7B9B39075D55@FreeBSD.org> In-Reply-To: <533A7416.7030605@FreeBSD.org> References: <201403312049.s2VKnXLr079029@svn.freebsd.org> <533A7416.7030605@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Wiadomo=B6=E6 napisana przez Kubilay Kocak w dniu 1 kwi 2014, o godz. = 10:08: > 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 >>=20 >> Log: >> Make it possible to have multiple CTL worker threads. Leave the = default >> of 1 for now. >>=20 >> Sponsored by: The FreeBSD Foundation >>=20 >> Modified: >> head/sys/cam/ctl/ctl.c >>=20 >> Modified: head/sys/cam/ctl/ctl.c >> = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >> --- 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> >>=20 >> @@ -320,6 +321,10 @@ static int ctl_is_single =3D 1; >> static int index_to_aps_page; >>=20 >> SYSCTL_NODE(_kern_cam, OID_AUTO, ctl, CTLFLAG_RD, 0, "CAM Target = Layer"); >> +static int worker_threads =3D 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"); >>=20 >> /* >> * 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 =3D0; >> -#if 0 >> - int i; >> -#endif >> - int error, retval; >> + int i, error, retval; >> //int isc_retval; >>=20 >> retval =3D 0; >> @@ -1085,17 +1087,35 @@ ctl_init(void) >> mtx_unlock(&softc->ctl_lock); >> #endif >>=20 >> - error =3D kproc_create(ctl_work_thread, softc, = &softc->work_thread, 0, 0, >> - "ctl_thrd"); >> - if (error !=3D 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 =3D=3D 0) { >> + printf("invalid kern.cam.ctl.worker_threads value; " >> + "setting to 1"); >> + worker_threads =3D 1; >> + } else if (worker_threads < 0) { >=20 > Why is it that the < 0 case is special enough that it gets the = mp_ncpus > check below, but the =3D=3D 0 and > MAXCPU cases don't? It's to be able to set it to -1 (which will probably become the new = default soon) and let the code figure out the right number by itself. >> + if (mp_ncpus > 2) { >> + /* >> + * Using more than two worker threads actually = hurts >> + * performance due to lock contention. >> + */ >=20 >> + worker_threads =3D 2; >> + } else { >> + worker_threads =3D 1; >> + } >=20 > 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? The warning is to inform the user why the value supplied was ignored as invalid. The -1 setting is always valid.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AD63ABB8-AF48-4378-ABBA-7B9B39075D55>