svn commit: r351406 - head/sys/dev/nvme

[Warner Losh]

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 <jkim at freebsd.org> wrote:

> On 19. 8. 22., Warner Losh wrote:
> > On Thu, Aug 22, 2019, 3:27 PM John Baldwin <jhb at freebsd.org
> > <mailto:jhb at freebsd.org>> 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
>