From owner-svn-src-head@freebsd.org Wed May 3 21:05:16 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 69214D5CCF8; Wed, 3 May 2017 21:05:16 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from mail.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 29BB61E21; Wed, 3 May 2017 21:05:16 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from ralph.baldwin.cx (c-73-231-226-104.hsd1.ca.comcast.net [73.231.226.104]) by mail.baldwin.cx (Postfix) with ESMTPSA id 4AD4710A7DB; Wed, 3 May 2017 17:05:13 -0400 (EDT) From: John Baldwin To: Conrad Meyer 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 Date: Wed, 03 May 2017 14:05:09 -0700 Message-ID: <3289384.Hk57GyGBQs@ralph.baldwin.cx> User-Agent: KMail/4.14.10 (FreeBSD/11.0-STABLE; KDE/4.14.10; amd64; ; ) In-Reply-To: <201705031841.v43If85k002993@repo.freebsd.org> References: <201705031841.v43If85k002993@repo.freebsd.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (mail.baldwin.cx); Wed, 03 May 2017 17:05:13 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.99.2 at mail.baldwin.cx X-Virus-Status: Clean X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 May 2017 21:05:16 -0000 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