Date: Sat, 14 Sep 2013 19:29:58 -0400 From: Leonardo Passos <lnrdpss@gmail.com> To: freebsd-drivers@freebsd.org, jmg@funkthat.com Subject: Re: Device hierarchy Message-ID: <CAO10wb7BduMDG3aNF3f7077vq8cAxNR3h3sf3uYNT_ZOJ_yAaw@mail.gmail.com> In-Reply-To: <20130914182537.GZ68682@funkthat.com> References: <CAO10wb6_AoZ9maGhx326tjZ6-aqvyzVV7rBxfOv%2BLOABixs_cw@mail.gmail.com> <20130914182537.GZ68682@funkthat.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Thanks John-Mark. That was very insightful :)
On Sat, Sep 14, 2013 at 2:25 PM, John-Mark Gurney <jmg@funkthat.com> wrote:
> Leonardo Passos wrote this message on Sat, Sep 14, 2013 at 14:09 -0400:
> > Hi all,
> >
> > I am checking some code from the FreeBSD repository, and I came across a
> > situation that I could not explain, at least from the existing
> > documentation.
> >
> > Looking at sys/dev/ata/ata-raid.c in the 9.1 release, I see the following
> > module declaration (line 4616):
> >
> > static moduledata_t ata_raid_moduledata =
> > { "ataraid", ata_raid_module_event_handler, NULL };
> > DECLARE_MODULE(ata, ata_raid_moduledata, SI_SUB_RAID, SI_ORDER_FIRST);
> > MODULE_VERSION(ataraid, 1);
> > MODULE_DEPEND(ataraid, ata, 1, 1, 1);
> > MODULE_DEPEND(ataraid, ad, 1, 1, 1);
>
> Read below, it looks like DECLARE_MODULE's name is the same:
> #define DECLARE_MODULE_WITH_MAXVER(name, data, sub, order, maxver) \
> MODULE_DEPEND(name, kernel, __FreeBSD_version, \
> __FreeBSD_version, maxver); \
> MODULE_METADATA(_md_##name, MDT_MODULE, &data, #name); \
> SYSINIT(name##module, sub, order, module_register_init, &data); \
> struct __hack
>
> #define DECLARE_MODULE(name, data, sub, order) \
> DECLARE_MODULE_WITH_MAXVER(name, data, sub, order,
> MODULE_KERNEL_MAXVER)
>
> and the first arg to SYSINIT is uniquifier...
>
> > Another weird situation appears in sys/dev/ata-isa.c (line 201):
> >
> > static driver_t ata_isa_driver = {
> > "ata",
> > ata_isa_methods,
> > sizeof(struct ata_channel),
> > };
> >
> > DRIVER_MODULE(ata, isa, ata_isa_driver, ata_devclass, 0, 0);
> > MODULE_DEPEND(ata, ata, 1, 1, 1);
> >
> > Here, the driver module name is set to "ata", and later, it is stated
> that
> > ata depends on ata. Is the dependency referring to another ata module,
> at a
> > different level in the driver stacking? How are these names and
> > dependencies resolved?
>
> Here, MODULE_DEPEND should probably be:
> MODULE_DEPEND(isa/ata, ata, 1, 1, 1);
>
> but I'm not sure how that would/will work... Here DRIVER_MODULE does
> something special and names the module w/ the bus/name instead of just
> the first one, and why if you run kldstat -v, you'll see something like:
> 45 isa/ata
>
> in the output... But after looking at the MODULE_DEPEND, what really
> matters is that the first arg is unique per file since the macro only
> uses it to name symbols:
> #define MODULE_METADATA(uniquifier, type, data, cval) \
> static struct mod_metadata _mod_metadata##uniquifier = { \
> MDT_STRUCT_VERSION, \
> type, \
> data, \
> cval \
> }; \
> DATA_SET(modmetadata_set, _mod_metadata##uniquifier)
>
> #define MODULE_DEPEND(module, mdepend, vmin, vpref, vmax) \
> static struct mod_depend _##module##_depend_on_##mdepend = { \
> vmin, \
> vpref, \
> vmax \
> }; \
> MODULE_METADATA(_md_##module##_on_##mdepend, MDT_DEPEND, \
> &_##module##_depend_on_##mdepend, #mdepend)
>
>
> So, adding the slash is probably not useful... It should probably be
> made clear in MODULE_DEPEND that name is only used to uniquify the
> symbols and have no useful info... Maybe we should switch to using
> __LINE__ instead, and make name option/unused?
>
> We should probably change the first arg for MODULE_DEPEND and
> DECLARE_MODULE to be like SYSINIT's and name them uniquifier..
>
> P.S. The macros I pasted came from sys/module.h.
>
> --
> John-Mark Gurney Voice: +1 415 225 5579
>
> "All that I will do, has been done, All that I have, has not."
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAO10wb7BduMDG3aNF3f7077vq8cAxNR3h3sf3uYNT_ZOJ_yAaw>