From owner-svn-src-all@freebsd.org Sat Feb 17 14:56:53 2018 Return-Path: Delivered-To: svn-src-all@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 E36F8F157DD for ; Sat, 17 Feb 2018 14:56:52 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-it0-x22c.google.com (mail-it0-x22c.google.com [IPv6:2607:f8b0:4001:c0b::22c]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 7768082D05 for ; Sat, 17 Feb 2018 14:56:52 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-it0-x22c.google.com with SMTP id l129so622784ita.3 for ; Sat, 17 Feb 2018 06:56:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=u3PsktKgGY5ryCcKThv4IR2sx4sBfgihGaOkFI0iwrA=; b=Q1GkrQ399PB02SiGHoN7RokUnMpMsFUaMxWN5x0VlOiZ1Enq3nwaVFC4WxJk8JMp+A Lk5N+HPtVsCTiQmtgFnoiZNxin/QbMKtlkh+1lM0iJEkiyxDevxBs1S6FBTjKizFEtSo lTNM2HTBNowjrdrmgcK/uaMDf91+5Mnn7bfKGEAzSYC2DMEqDmirsmx+p0iu75Qdy9Jj m7w/z87FQxR03Mk9urxP6gSudsA6x+lAtFNm7UPrIJKneVqBeUeDrhMnL0T1tTk1MP7u CdUmgXqbvTJrBqxP4PeemRmGNSsI25EcZ62aSj8EtShdCpM0MmD99Hs4XFovBG3+rxtY rwqg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=u3PsktKgGY5ryCcKThv4IR2sx4sBfgihGaOkFI0iwrA=; b=kVKZzfmhzQ6w8fbmWwHuabVSb0+A1GaBP7yyMdGltJu/PW1v7SJIJteZ0ueFQiiocv khHV38IvWyR1Uo+whArQmNqA2oNe8EzHIl/KT14u3mdHGubMkq/GJD9RVt2yfcOmKJ1+ RGCCndcnroOvrmQy7QvG9rKb8RHxtMQSM880yiQnywRxiuRkHGbJSKDvGTSMZPHrfEOn +95UhbEPWNeqIxgwCS2S1nWiTuwsELc1XTGFnDvtQ/C1jS5HBkKos4RYeN5uWZezyXaU hTIEjEG3zEy1WCT+y3IPJXIQR4Sa6llnXSNWij7ukFo6Btgudihxh1FgaVRruOvhh7dO 8LcQ== X-Gm-Message-State: APf1xPCKQWEAiq3elW+mwnj6LssrMxa17nQJuPab+MPdCz41n20OOc2m rHNDScPhnLKPNW+iFIvub5NGeYByNbHwi0tz3GedEw== X-Google-Smtp-Source: AH8x226AT6BkfOobeyleVuiPVPCaU0chWd9UUwPrnn3Mi9Zs7+ul8qUHrECOSKyffxMTmKHXjE6f1SSKHYu6QtBxzcw= X-Received: by 10.36.55.146 with SMTP id r140mr2056292itr.57.1518879411848; Sat, 17 Feb 2018 06:56:51 -0800 (PST) MIME-Version: 1.0 Sender: wlosh@bsdimp.com Received: by 10.79.201.67 with HTTP; Sat, 17 Feb 2018 06:56:51 -0800 (PST) X-Originating-IP: [50.253.99.174] In-Reply-To: <201802171434.w1HEYl8I063603@repo.freebsd.org> References: <201802171434.w1HEYl8I063603@repo.freebsd.org> From: Warner Losh Date: Sat, 17 Feb 2018 07:56:51 -0700 X-Google-Sender-Auth: JBVwTcgFx5DKgx9VhCDB_wSnW-I Message-ID: Subject: Re: svn commit: r329458 - head/sbin/devmatch To: Hans Petter Selasky Cc: src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.25 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 17 Feb 2018 14:56:53 -0000 On Sat, Feb 17, 2018 at 7:34 AM, Hans Petter Selasky wrote: > Author: hselasky > Date: Sat Feb 17 14:34:47 2018 > New Revision: 329458 > URL: https://svnweb.freebsd.org/changeset/base/329458 > > Log: > Fix USB driver matching in devmatch(8). > > Multiple drivers can match on the same USB device and the order of > loading > decides which driver gets the device. Use the supplied mask value as an > indication of priority, so that vendor specific device drivers are loaded > before more generic ones. > This change isn't right. I mean, it might cause the special case of USB to get the driver you want loaded, but using the MASK this way is wrong. It's fixing the wrong problem. devmatch can't know who the winners will be and has to recommend all the drivers to load and those drivers have to cope with that. The right fix there, I think, is to load them all at once, in one kldload operation and not loop in /etc/rc.d/devmatch. I'd request you make no further changes to devmatch without a review. Warner > > Sponsored by: Mellanox Technologies > > Modified: > head/sbin/devmatch/devmatch.c > > Modified: head/sbin/devmatch/devmatch.c > ============================================================ > ================== > --- head/sbin/devmatch/devmatch.c Sat Feb 17 14:30:39 2018 > (r329457) > +++ head/sbin/devmatch/devmatch.c Sat Feb 17 14:34:47 2018 > (r329458) > @@ -54,6 +54,14 @@ static struct option longopts[] = { > { NULL, 0, NULL, 0 } > }; > > +#define DEVMATCH_MAX_HITS 256 > + > +static struct match_data { > + char *descr; > + int priority; > +} match_data[DEVMATCH_MAX_HITS]; > + > +static int hit_index; > static int all_flag; > static int dump_flag; > static char *linker_hints; > @@ -236,6 +244,35 @@ pnpval_as_str(const char *val, const char *pnpinfo) > return retval; > } > > +static int > +match_data_compare(const void *_pa, const void *_pb) > +{ > + const struct match_data *pa = _pa; > + const struct match_data *pb = _pb; > + > + /* biggest value first */ > + if (pa->priority > pb->priority) > + return (-1); > + else if (pa->priority < pb->priority) > + return (1); > + > + /* then sort by string */ > + return (strcmp(pa->descr, pb->descr)); > +} > + > +static int > +bitrev16(int input) > +{ > + int retval = 0; > + int x; > + > + for (x = 0; x != 16; x++) { > + if ((input >> x) & 1) > + retval |= (0x8000 >> x); > + } > + return (retval); > +} > + > static void > search_hints(const char *bus, const char *dev, const char *pnpinfo) > { > @@ -367,9 +404,20 @@ search_hints(const char *bus, const char *dev, const c > printf("\n"); > else if (!notme) { > if (!unbound_flag) { > + char *descr = NULL; > + > if (all_flag) > - printf("%s: ", > *dev ? dev : "unattached" ); > - printf("%s\n", lastmod); > + asprintf(&descr, > "%s: %s", *dev ? dev : "unattached", lastmod); > + else > + asprintf(&descr, > "%s", lastmod); > + > + if (descr != NULL && > hit_index < DEVMATCH_MAX_HITS) { > + > match_data[hit_index].descr = descr; > + > match_data[hit_index].priority = bitrev16(mask); > + hit_index++; > + } else { > + free(descr); > + } > } > found++; > } > @@ -382,6 +430,19 @@ search_hints(const char *bus, const char *dev, const c > } > walker = (void *)(len - sizeof(int) + (intptr_t)walker); > } > + if (hit_index != 0) { > + /* sort hits by priority */ > + mergesort(match_data, hit_index, sizeof(match_data[0]), > &match_data_compare); > + > + /* printout */ > + for (i = 0; i != hit_index; i++) { > + puts(match_data[i].descr); > + free(match_data[i].descr); > + } > + > + /* reset hit_index */ > + hit_index = 0; > + } > if (unbound_flag && found == 0 && *pnpinfo) { > if (verbose_flag) > printf("------------------------- "); > >