From owner-svn-src-head@freebsd.org Fri Apr 20 17:30:08 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 8EBBCF862AC; Fri, 20 Apr 2018 17:30:08 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from mail.baldwin.cx (bigwig.baldwin.cx [96.47.65.170]) (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 344A16BFD2; Fri, 20 Apr 2018 17:30:07 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from ralph.baldwin.cx (ralph.baldwin.cx [66.234.199.215]) by mail.baldwin.cx (Postfix) with ESMTPSA id 77D4A10AFD2; Fri, 20 Apr 2018 13:30:00 -0400 (EDT) From: John Baldwin To: Eric Joyner Cc: src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r332735 - head/sys/x86/x86 Date: Fri, 20 Apr 2018 09:57:28 -0700 Message-ID: <1781933.c5USM7Fcyk@ralph.baldwin.cx> User-Agent: KMail/4.14.10 (FreeBSD/11.1-STABLE; KDE/4.14.30; amd64; ; ) In-Reply-To: References: <201804181845.w3IIjYdt037258@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); Fri, 20 Apr 2018 13:30:00 -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.25 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: Fri, 20 Apr 2018 17:30:08 -0000 I need to rework the layout of IRQ values to be more dynamic (per a thread on current@) as I've seen at least a few systems now that have more than 256 I/O APIC pins. Currently we kind of depend on interrupt_sources[] being statically sized, but I could rework this to support dynamic sizing as a result and probably provide a tunable to control the limit on MSI interrupts (and possibly derive the default limit from the number of CPUs). On Thursday, April 19, 2018 08:59:32 AM Eric Joyner wrote: > Is the limit going to be increased at some point? Is there a true limit > somewhere? > > On Wed, Apr 18, 2018 at 11:45 AM, John Baldwin wrote: > > > Author: jhb > > Date: Wed Apr 18 18:45:34 2018 > > New Revision: 332735 > > URL: https://svnweb.freebsd.org/changeset/base/332735 > > > > Log: > > Fix two off-by-one errors when allocating MSI and MSI-X interrupts. > > > > x86 enforces an (arbitray) limit on the number of available MSI and > > MSI-X interrupts to simplify code (in particular, interrupt_source[] > > is statically sized). This means that an attempt to allocate an MSI > > vector needs to fail if it would go beyond the limit, but the checks > > for exceeding the limit had an off-by-one error. In the case of MSI-X > > which allocates interrupts one at a time this meant that IRQ 768 kept > > getting handed out multiple times for msix_alloc() instead of failing > > because all MSI IRQs were in use. > > > > Tested by: lidl > > MFC after: 1 week > > > > Modified: > > head/sys/x86/x86/msi.c > > > > Modified: head/sys/x86/x86/msi.c > > ============================================================ > > ================== > > --- head/sys/x86/x86/msi.c Wed Apr 18 18:45:04 2018 (r332734) > > +++ head/sys/x86/x86/msi.c Wed Apr 18 18:45:34 2018 (r332735) > > @@ -404,7 +404,7 @@ again: > > /* Do we need to create some new sources? */ > > if (cnt < count) { > > /* If we would exceed the max, give up. */ > > - if (i + (count - cnt) > FIRST_MSI_INT + NUM_MSI_INTS) { > > + if (i + (count - cnt) >= FIRST_MSI_INT + NUM_MSI_INTS) { > > mtx_unlock(&msi_lock); > > free(mirqs, M_MSI); > > return (ENXIO); > > @@ -645,7 +645,7 @@ again: > > /* Do we need to create a new source? */ > > if (msi == NULL) { > > /* If we would exceed the max, give up. */ > > - if (i + 1 > FIRST_MSI_INT + NUM_MSI_INTS) { > > + if (i + 1 >= FIRST_MSI_INT + NUM_MSI_INTS) { > > mtx_unlock(&msi_lock); > > return (ENXIO); > > } > > > > -- John Baldwin