From owner-svn-src-all@freebsd.org Fri Aug 23 05:24:13 2019 Return-Path: Delivered-To: svn-src-all@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 BD821DA9AB 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 46F8vw6Lbgz4Kcd 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 s145so7237070qke.7 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=s7gSSpQaaWyKQO9afELLKDH6G9NE1ZtVnvmhd+LpSpt0uEGX7oGMOTfMbCXDpZjjlj dl62O2kMDqNiQzfczxuNSHmmBUyMGsyJIG86sT+O1a/X4chcJaAc/EH9Hq9raNriaitA 2FPsPooDyDQpoMVrSMdbehwBNKqEcaOoJ4Dn+uNbdRFF2A8m81WZwfnL1u/iNm9gojI1 XtbPedwqAFns8aO9h4RYfeZmJ0IMiSTK6CK/mdwvoNDu/JiYiNj5XZ63uebgtayPfBtT HSSgL/NrmeVPhzMUESeZbws9eAGS5CNH/ZJxJDPRXwbIhJsF5dtIeGCjlsOf7VEAzOJ9 aj1Q== X-Gm-Message-State: APjAAAWcE7NDfkUBVLbClpMfLclh404GDFCoTX1CeVMlW6XQRnEVVshE BgeXQ3uEftoPXQVptuqwhnlagLl8rT0D9y7SvkRpWw== 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: 46F8vw6Lbgz4Kcd 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-all@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-all@freebsd.org X-Mailman-Version: 2.1.29 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: 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 >