Date: Wed, 03 May 2017 14:05:09 -0700 From: John Baldwin <jhb@freebsd.org> To: Conrad Meyer <cem@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r317756 - in head/sys: kern sys Message-ID: <3289384.Hk57GyGBQs@ralph.baldwin.cx> In-Reply-To: <201705031841.v43If85k002993@repo.freebsd.org> References: <201705031841.v43If85k002993@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday, May 03, 2017 06:41:08 PM Conrad Meyer wrote: > Author: cem > Date: Wed May 3 18:41:08 2017 > New Revision: 317756 > URL: https://svnweb.freebsd.org/changeset/base/317756 > > Log: > Extend cpuset_get/setaffinity() APIs > > Add IRQ placement-only and ithread-only API variants. intr_event_bind > has been extended with sibling methods, as it has many more callsites in > existing code. > > Reviewed by: kib@, adrian@ (earlier version) > Sponsored by: Dell EMC Isilon > Differential Revision: https://reviews.freebsd.org/D10586 > > Modified: head/sys/kern/kern_intr.c > ============================================================================== > --- head/sys/kern/kern_intr.c Wed May 3 17:21:01 2017 (r317755) > +++ head/sys/kern/kern_intr.c Wed May 3 18:41:08 2017 (r317756) > @@ -287,13 +287,11 @@ intr_event_create(struct intr_event **ev > /* > * Bind an interrupt event to the specified CPU. Note that not all > * platforms support binding an interrupt to a CPU. For those > - * platforms this request will fail. For supported platforms, any > - * associated ithreads as well as the primary interrupt context will > - * be bound to the specificed CPU. Using a cpu id of NOCPU unbinds > + * platforms this request will fail. Using a cpu id of NOCPU unbinds > * the interrupt event. > */ > -int > -intr_event_bind(struct intr_event *ie, int cpu) > +static int > +_intr_event_bind(struct intr_event *ie, int cpu, bool bindirq, bool bindithread) > { > lwpid_t id; > int error; > @@ -313,35 +311,75 @@ intr_event_bind(struct intr_event *ie, i > * If we have any ithreads try to set their mask first to verify > * permissions, etc. > */ > - mtx_lock(&ie->ie_lock); > - if (ie->ie_thread != NULL) { > - id = ie->ie_thread->it_thread->td_tid; > - mtx_unlock(&ie->ie_lock); > - error = cpuset_setithread(id, cpu); > - if (error) > - return (error); > - } else > - mtx_unlock(&ie->ie_lock); > - error = ie->ie_assign_cpu(ie->ie_source, cpu); > - if (error) { > + if (bindithread) { > mtx_lock(&ie->ie_lock); > if (ie->ie_thread != NULL) { > - cpu = ie->ie_cpu; > id = ie->ie_thread->it_thread->td_tid; > mtx_unlock(&ie->ie_lock); > - (void)cpuset_setithread(id, cpu); > + error = cpuset_setithread(id, cpu); > + if (error) > + return (error); > } else > mtx_unlock(&ie->ie_lock); > + } > + if (bindirq) > + error = ie->ie_assign_cpu(ie->ie_source, cpu); > + if (error) { > + if (bindithread) { This error block should probably be under 'if (bindirq)' as it only used to "undo" an ithread binding if an irq binding failed. > + mtx_lock(&ie->ie_lock); > + if (ie->ie_thread != NULL) { > + cpu = ie->ie_cpu; > + id = ie->ie_thread->it_thread->td_tid; > + mtx_unlock(&ie->ie_lock); > + (void)cpuset_setithread(id, cpu); > + } else > + mtx_unlock(&ie->ie_lock); > + } > return (error); > } > > - mtx_lock(&ie->ie_lock); > - ie->ie_cpu = cpu; > - mtx_unlock(&ie->ie_lock); > + if (bindirq) { > + mtx_lock(&ie->ie_lock); > + ie->ie_cpu = cpu; > + mtx_unlock(&ie->ie_lock); > + } This would then be in the else. That is, the code structure would be something like: if (bindithread) { /* new code to bind ithread */ } if (bindirq) { error = ie_assign_cpu(); if (error) { /* undo bind ithread */ } else { /* set ie_cpu */ } } > > return (error); > } > > +/* > + * Bind an interrupt event to the specified CPU. For supported platforms, any > + * associated ithreads as well as the primary interrupt context will be bound > + * to the specificed CPU. > + */ > +int > +intr_event_bind(struct intr_event *ie, int cpu) > +{ > + > + return (_intr_event_bind(ie, cpu, true, true)); > +} This is used by new-bus so the wrapper needs to stay. > + > +/* > + * Bind an interrupt event to the specified CPU, but do not bind associated > + * ithreads. > + */ > +int > +intr_event_bind_irqonly(struct intr_event *ie, int cpu) > +{ > + > + return (_intr_event_bind(ie, cpu, true, false)); > +} > + > +/* > + * Bind an interrupt event's ithread to the specified CPU. > + */ > +int > +intr_event_bind_ithread(struct intr_event *ie, int cpu) > +{ > + > + return (_intr_event_bind(ie, cpu, false, true)); > +} These aren't otherwise used? Perhaps rather than having these wrappers, the switch in intr_setaffinity() could just call _intr_event_bind() directly with the right set of booleans? The compiler will probably simplify it to the equivalent of: _intr_event_bind(ie, cpu, which != CPU_WHICH_ITHREAD, which != CPU_WHICH_INTRHANDLER); -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3289384.Hk57GyGBQs>