From owner-svn-src-head@freebsd.org Sat Feb 17 14:56:53 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 EB207F157DF for ; Sat, 17 Feb 2018 14:56:52 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-it0-x233.google.com (mail-it0-x233.google.com [IPv6:2607:f8b0:4001:c0b::233]) (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 806CC82D07 for ; Sat, 17 Feb 2018 14:56:52 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-it0-x233.google.com with SMTP id v186so4990527itc.5 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=YwFEFvP5BQZ46jkytUW97J4cQ+81rv4KlxZ0sZElyG+y/G5H+egRkoHpPA88rW5+79 s+JA5q8qakJkFpwBDBl7vzuGLDptjRcmd7c48q6+IzRUmiSauYA5ZlIDNnNYWebGFmgx QZfUjIf4RKcB9XlBi6n/uHWKuRM2WHpS8y7oyE2BX7dwB4vko4ach3/+KR327ZNw/Pf9 HFE1zv78ZHp3K2STw61QQ+8KE6IshVGuczHcbMT1CgbrmW2ETeFk+2EC6skSZ8YkvBpZ egGOgJo5bRUIzmRr/Wp/5VSIp6bQ3nuJ7Z4djp+W0kGNrnP+oVdN3gXwiYfF1irw6Uqc uuRA== X-Gm-Message-State: APf1xPDdfUs3e9IEojPqvc+o1+LbvAnCm6nE4Z8syfPGqpKkJ6shMMhT U8owEU/jeumWLhWjo5A3toPS0tpF88OoMBskccVTMA== 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-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: 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("------------------------- "); > >