From owner-svn-src-head@freebsd.org Fri Aug 23 05:24:13 2019 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id BD79EDA9AA for ; Fri, 23 Aug 2019 05:24:13 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-qk1-x742.google.com (mail-qk1-x742.google.com [IPv6:2607:f8b0:4864:20::742]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 46F8vw6LYsz4Kcc for ; Fri, 23 Aug 2019 05:24:12 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-qk1-x742.google.com with SMTP id p13so7224339qkg.13 for ; Thu, 22 Aug 2019 22:24:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ATqVYdg7EiguXaEPQYsvuoKgDhS/Lt3lcixvB9uAK20=; b=Fvf30ftSqtj4iAz3iLLdTm4jc4lP7SlsdJyaUs5U8zDTe4ZQuUbdTG3uOOlM/k6l8M zOWQiAUct4vq/b2LPzZAsnCWR5OcxOemyhucK7MJdtLzY3Y49zf9NIBtHI5LZ9IoQ3LF P/9iBge888wav3a96UsoiSPQAOvkJDw+16JCPmhQnjUzxi9bD81EN5cP4RPjW00s/yG6 YOrWAeModEJ3BECe/UnKSI/txnut3E4webHeRnpA82edve3A1/av1Gqko/HAQKnkXZNc NQkGUSo9NO1R4FFlFrqSJ7eQTBYRZ6hI46hPZJ7fVzeWhrwxT0e6APCL86fj9RfOpYIO wVsQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ATqVYdg7EiguXaEPQYsvuoKgDhS/Lt3lcixvB9uAK20=; b=HK07G3JTd512pNTmnnG/ICjptvlrZRP2jzLNPshFgh6s13iXrnoSJ0/7CrbblnwJoz h0iSwQteABluXFP+234McJfn6cs8vW/1UnPLIxOiqG1xdBYN3+LsxIRbycOV5E9YjTzg aLhn4knncydqaEo3rNpUgxlnCBSRlJ2rOqczvjkW+Uzq2tK8CbYlpJH19XpvdtMTHd/g UsP2F43PFBx2pPGkktfQgP0nTJ5yItq4sgQe4fOH5ye4ywWsfRuOAX0/rdVEgvenihX3 cuG9Lq7A/UtGXTjZt0/Cgdf4S6l+bwI7xR2D/0uFW4WY6TlLcfEgN8JHOWoM0d2qv0Au h6xQ== X-Gm-Message-State: APjAAAXL9clfZmsCVKjyJoOcVqJ2yoKkzFkguHYjwhBUxpQXL7UHn6io 2o9Ghn9MU+nE0y5EgGDrOOyy3+uEQ1dv9Su8GBfbHg== X-Google-Smtp-Source: APXvYqyotHCXAUBb/8jusMVsviPjCSOSFNJRNqATVXnR7tg+6RSIX4KCV1jDNDcmdF6/I4VruAWEzSCtuihUQS4kEe8= X-Received: by 2002:a37:4b03:: with SMTP id y3mr2506294qka.215.1566537851537; Thu, 22 Aug 2019 22:24:11 -0700 (PDT) MIME-Version: 1.0 References: <201908222112.x7MLCpbt023647@repo.freebsd.org> In-Reply-To: From: Warner Losh Date: Thu, 22 Aug 2019 23:24:00 -0600 Message-ID: Subject: Re: svn commit: r351406 - head/sys/dev/nvme To: Jung-uk Kim Cc: John Baldwin , Warner Losh , src-committers , svn-src-all , svn-src-head X-Rspamd-Queue-Id: 46F8vw6LYsz4Kcc X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org; dkim=pass header.d=bsdimp-com.20150623.gappssmtp.com header.s=20150623 header.b=Fvf30ftS; dmarc=none; spf=none (mx1.freebsd.org: domain of wlosh@bsdimp.com has no SPF policy when checking 2607:f8b0:4864:20::742) smtp.mailfrom=wlosh@bsdimp.com X-Spamd-Result: default: False [-2.33 / 15.00]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-0.998,0]; R_DKIM_ALLOW(-0.20)[bsdimp-com.20150623.gappssmtp.com:s=20150623]; FROM_HAS_DN(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; PREVIOUSLY_DELIVERED(0.00)[svn-src-head@freebsd.org]; DMARC_NA(0.00)[bsdimp.com]; URI_COUNT_ODD(1.00)[3]; RCPT_COUNT_FIVE(0.00)[6]; TO_MATCH_ENVRCPT_SOME(0.00)[]; TO_DN_ALL(0.00)[]; DKIM_TRACE(0.00)[bsdimp-com.20150623.gappssmtp.com:+]; NEURAL_HAM_SHORT(-0.94)[-0.936,0]; RCVD_IN_DNSWL_NONE(0.00)[2.4.7.0.0.0.0.0.0.0.0.0.0.0.0.0.0.2.0.0.4.6.8.4.0.b.8.f.7.0.6.2.list.dnswl.org : 127.0.5.0]; R_SPF_NA(0.00)[]; FORGED_SENDER(0.30)[imp@bsdimp.com,wlosh@bsdimp.com]; MIME_TRACE(0.00)[0:+,1:+,2:~]; IP_SCORE(-0.40)[ip: (3.29), ipnet: 2607:f8b0::/32(-2.90), asn: 15169(-2.35), country: US(-0.05)]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; FROM_NEQ_ENVFROM(0.00)[imp@bsdimp.com,wlosh@bsdimp.com]; RCVD_TLS_ALL(0.00)[]; RCVD_COUNT_TWO(0.00)[2] Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 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, 23 Aug 2019 05:24:13 -0000 This creates some funky names for the modules (like nvme_pci_pci)... I'd prefer we not do that. Here's a patch that I think will create the module in a way that avoids the bug that we're hitting. It's late here, and I've not had time to test it on all the cases yet. Does it work for you? It eliminates the duplicate module message in the kernel boot I failed to notice before. Warner diff --git a/sys/dev/nvme/nvme.c b/sys/dev/nvme/nvme.c index e694fcb7747..2963029fc06 100644 --- a/sys/dev/nvme/nvme.c +++ b/sys/dev/nvme/nvme.c @@ -364,3 +364,19 @@ nvme_completion_poll_cb(void *arg, const struct nvme_completion *cpl) memcpy(&status->cpl, cpl, sizeof(*cpl)); atomic_store_rel_int(&status->done, 1); } + +static int +nvme_modevent(module_t mod __unused, int type __unused, void *argp __unused) +{ + return (0); +} + +static moduledata_t nvme_mod = { + "nvme", + nvme_modevent, + 0 +}; + +DECLARE_MODULE(nvme, nvme_mod, SI_SUB_DRIVERS, SI_ORDER_FIRST); +MODULE_VERSION(nvme, 1); +MODULE_DEPEND(nvme, cam, 1, 1, 1); diff --git a/sys/dev/nvme/nvme_ahci.c b/sys/dev/nvme/nvme_ahci.c index 29c40d919a7..eae607bcce8 100644 --- a/sys/dev/nvme/nvme_ahci.c +++ b/sys/dev/nvme/nvme_ahci.c @@ -55,8 +55,6 @@ static driver_t nvme_ahci_driver = { }; DRIVER_MODULE(nvme, ahci, nvme_ahci_driver, nvme_devclass, NULL, 0); -MODULE_VERSION(nvme, 1); -MODULE_DEPEND(nvme, cam, 1, 1, 1); static int nvme_ahci_probe (device_t device) diff --git a/sys/dev/nvme/nvme_pci.c b/sys/dev/nvme/nvme_pci.c index 0d4923910ff..68142518404 100644 --- a/sys/dev/nvme/nvme_pci.c +++ b/sys/dev/nvme/nvme_pci.c @@ -62,8 +62,6 @@ static driver_t nvme_pci_driver = { }; DRIVER_MODULE(nvme, pci, nvme_pci_driver, nvme_devclass, NULL, 0); -MODULE_VERSION(nvme, 1); -MODULE_DEPEND(nvme, cam, 1, 1, 1); static struct _pcsid { On Thu, Aug 22, 2019 at 6:53 PM Jung-uk Kim wrote: > On 19. 8. 22., Warner Losh wrote: > > On Thu, Aug 22, 2019, 3:27 PM John Baldwin > > wrote: > > > > On 8/22/19 2:12 PM, Warner Losh wrote: > > > Author: imp > > > Date: Thu Aug 22 21:12:51 2019 > > > New Revision: 351406 > > > URL: https://svnweb.freebsd.org/changeset/base/351406 > > > > > > Log: > > > We need to define version 1 of nvme, not nvme_foo. Otherwise nvd > > won't > > > load and people who pull in nvme/nvd from modules can't load > nvd.ko > > > since it depends on nvme, not nvme_foo. The duplicate doesn't > matter > > > since kldxref properly handles that case. > > > > I would perhaps have put the MODULE_VERSION for nvme and its > dependency > > on cam into nvme.c instead of duplicating it. I think that is more > > consistent > > with what we have done elsewhere in the tree. > > > > > > That can't be true. Doing that doesn't work. We wind up dereferencing > > freed memory, as we discussed on IRC. Each DRIVER_MODULE needs its own > > MODULE_VERSION given the current code. I'd call that a bug, but not one > > I could dig into today... > > FYI, the attached patch works for me. > > Jung-uk Kim >