Re: order of executing MOD_LOAD and registering module sysctl-s
On 02/08/2017 18:49, John Baldwin wrote: > sysctl nodes are created explicitly via linker_file_register_sysctls, not via > SYSINITs, so you can't order them with respect to other init functions. > > I think Andriy's suggestion of doing sysctls "inside" sysinits (so they are > registered last and unregistered first) is probably better than the current > state and is a simpler fix than changing all sysctls to use SYSINITs. Kostik (kib) suggested a possible valid use-case that depends on the current order: adding dynamic sysctl-s under static sysctl-s via the module load handler. He also offered an idea for a possible solution: holding the modules lock in the shared mode (MOD_SLOCK) around calls to sysctl-s registered from modules. -- Andriy Gapon ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: order of executing MOD_LOAD and registering module sysctl-s
On Wednesday, August 02, 2017 06:53:54 PM Hans Petter Selasky wrote: > On 08/02/17 17:49, John Baldwin wrote: > > On Wednesday, August 02, 2017 12:39:36 PM Hans Petter Selasky wrote: > >> On 08/02/17 12:13, Andriy Gapon wrote: > >>> > >>> As far as I understand a module initialization routine is executed via the > >>> sysinit mechanism. Specifically, module_register_init is set up as the > >>> sysinit > >>> function for every module and it calls MOD_EVENT(mod, MOD_LOAD) to invoke > >>> the > >>> module event handler. > >>> > >>> In linker_load_file() I see the following code: > >>> linker_file_register_sysctls(lf); > >>> linker_file_sysinit(lf); > >>> > >>> I think that this means that any statically declared sysctl-s in the > >>> module > >>> would be registered before the module receives the MOD_LOAD event. > >>> It's possible that some of the sysctl-s could have procedures as handlers > >>> and > >>> they might access data that is supposed to be initialized by the module > >>> event > >>> handler. > >>> > >>> So, for example, running sysctl -a at just the right moment during the > >>> loading > >>> of a module might end up in an expected behavior (including a crash). > >>> > >>> Is my interpretation of how the code works correct? > >>> Can the order of linker_file_sysinit and linker_file_register_sysctls be > >>> changed > >>> without a great risk? > >>> > >>> Thank you! > >>> > >>> P.S. > >>> The same applies to: > >>> linker_file_sysuninit(file); > >>> linker_file_unregister_sysctls(file); > >>> > >> > >> Hi, > >> > >> Not sure if this answers your question. > >> > > Hi, > > >> If a SYSCTL() is TUNABLE, it's procedure can be called when the sysctl > >> is created. Else the SYSCTL() procedure callback might be called right > >> after it's registered. I think there is an own subsystem in sys/kernel.h > >> which takes care of the actual SYSCTL() creation/destruction - after the > >> linker is involved. > > > > sysctl nodes are created explicitly via linker_file_register_sysctls, not > > via > > SYSINITs, so you can't order them with respect to other init functions. > > For GENERIC (non-modules) the SYSCTLS() are registered by > sysctl_register_all() at SYSINIT(sysctl, SI_SUB_KMEM, SI_ORDER_FIRST, > sysctl_register_all, 0); > > > > > I think Andriy's suggestion of doing sysctls "inside" sysinits (so they are > > registered last and unregistered first) is probably better than the current > > state and is a simpler fix than changing all sysctls to use SYSINITs. > > > > If the module provided SYSCTLS's could use the same SI_SUB_KMEM it would > be compatible. > > You have three cases to think about: > > 1) SYSCTLS's in modules loaded before the kernel is booted > 2) SYSCTLS's in modules after the kernel is booted > 3) SYSCTLS's in the GENERIC kernel. > > I'm not 100% sure, but I think 1) and 2) are treated differently. > Correct me if I'm wrong. 3) sysctls in the kernel are handled at SI_SUB_KMEM. 1) modules loaded by the loader are handled in linker_preload() at SI_SUB_KLD. Their sysctls are all registered when linker_preload() executes. Their SYSINITs are added to the pending sysinit list. Any SYSINITs earlier than SI_SUB_KLD will be executed in sorted order by mi_startup() after linker_preload() returns. Any SYSINITs after SI_SUB_KLD will be kept in the pending list in order and executed as if they were in the kernel. 2) All of the sysctl's are registered first, and afterwards the SYSINITs are run in sorted order. The race Andriy describes has always been present, but it was perhaps easier to fix by just inverting the order when TUNABLE_* were used explicitly as those registered explicit SI_SUB_TUNABLES sysinits. Note that it has always been the case with old TUNABLE_* that handlers could be run before locks were initialized (e.g. via MTX_SYSINIT which runs later at SI_SUB_LOCK), so if you have handlers that need to use locks, etc. you really shouldn't use RDTUN/RWTUN but instead you should use a dedicated SYSINIT to read a tunable and apply the right logic. There are a few different ways to fix Andriy's race while still solving the issue Ian notes that you may have SYSINITs that depend on the tunable fetches having been performed. One observation is that it shouldn't break anything to invert the order on unload and always unregister sysctls before invoking SYSUNINITs, so I think we should probably make that part of the change regardless. The variety comes in how to handle the module loading case: 1) Just extend the SYSCTL_XLOCK and hold it while invoking SYSINITs in modules during post-boot, and/or use some other explicit locking mechanism with sleep/wakeup or a cv to "pause" any sysctl requests until a kld is fully registered. This is a bit of a sledgehammer, but it is simple and kldload isn't a hot path. 2) Use two passes the add sysctl nodes. The
Re: order of executing MOD_LOAD and registering module sysctl-s
On 08/02/17 17:49, John Baldwin wrote: On Wednesday, August 02, 2017 12:39:36 PM Hans Petter Selasky wrote: On 08/02/17 12:13, Andriy Gapon wrote: As far as I understand a module initialization routine is executed via the sysinit mechanism. Specifically, module_register_init is set up as the sysinit function for every module and it calls MOD_EVENT(mod, MOD_LOAD) to invoke the module event handler. In linker_load_file() I see the following code: linker_file_register_sysctls(lf); linker_file_sysinit(lf); I think that this means that any statically declared sysctl-s in the module would be registered before the module receives the MOD_LOAD event. It's possible that some of the sysctl-s could have procedures as handlers and they might access data that is supposed to be initialized by the module event handler. So, for example, running sysctl -a at just the right moment during the loading of a module might end up in an expected behavior (including a crash). Is my interpretation of how the code works correct? Can the order of linker_file_sysinit and linker_file_register_sysctls be changed without a great risk? Thank you! P.S. The same applies to: linker_file_sysuninit(file); linker_file_unregister_sysctls(file); Hi, Not sure if this answers your question. Hi, If a SYSCTL() is TUNABLE, it's procedure can be called when the sysctl is created. Else the SYSCTL() procedure callback might be called right after it's registered. I think there is an own subsystem in sys/kernel.h which takes care of the actual SYSCTL() creation/destruction - after the linker is involved. sysctl nodes are created explicitly via linker_file_register_sysctls, not via SYSINITs, so you can't order them with respect to other init functions. For GENERIC (non-modules) the SYSCTLS() are registered by sysctl_register_all() at SYSINIT(sysctl, SI_SUB_KMEM, SI_ORDER_FIRST, sysctl_register_all, 0); I think Andriy's suggestion of doing sysctls "inside" sysinits (so they are registered last and unregistered first) is probably better than the current state and is a simpler fix than changing all sysctls to use SYSINITs. If the module provided SYSCTLS's could use the same SI_SUB_KMEM it would be compatible. You have three cases to think about: 1) SYSCTLS's in modules loaded before the kernel is booted 2) SYSCTLS's in modules after the kernel is booted 3) SYSCTLS's in the GENERIC kernel. I'm not 100% sure, but I think 1) and 2) are treated differently. Correct me if I'm wrong. --HPS ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: order of executing MOD_LOAD and registering module sysctl-s
On Wed, 2017-08-02 at 08:49 -0700, John Baldwin wrote: > On Wednesday, August 02, 2017 12:39:36 PM Hans Petter Selasky wrote: > > > > On 08/02/17 12:13, Andriy Gapon wrote: > > > > > > > > > As far as I understand a module initialization routine is > > > executed via the > > > sysinit mechanism. Specifically, module_register_init is set up > > > as the sysinit > > > function for every module and it calls MOD_EVENT(mod, MOD_LOAD) > > > to invoke the > > > module event handler. > > > > > > In linker_load_file() I see the following code: > > > linker_file_register_sysctls(lf); > > > linker_file_sysinit(lf); > > > > > > I think that this means that any statically declared sysctl-s in > > > the module > > > would be registered before the module receives the MOD_LOAD > > > event. > > > It's possible that some of the sysctl-s could have procedures as > > > handlers and > > > they might access data that is supposed to be initialized by the > > > module event > > > handler. > > > > > > So, for example, running sysctl -a at just the right moment > > > during the loading > > > of a module might end up in an expected behavior (including a > > > crash). > > > > > > Is my interpretation of how the code works correct? > > > Can the order of linker_file_sysinit and > > > linker_file_register_sysctls be changed > > > without a great risk? > > > > > > Thank you! > > > > > > P.S. > > > The same applies to: > > > linker_file_sysuninit(file); > > > linker_file_unregister_sysctls(file); > > > > > Hi, > > > > Not sure if this answers your question. > > > > If a SYSCTL() is TUNABLE, it's procedure can be called when the > > sysctl > > is created. Else the SYSCTL() procedure callback might be called > > right > > after it's registered. I think there is an own subsystem in > > sys/kernel.h > > which takes care of the actual SYSCTL() creation/destruction - > > after the > > linker is involved. > sysctl nodes are created explicitly via linker_file_register_sysctls, > not via > SYSINITs, so you can't order them with respect to other init > functions. > > I think Andriy's suggestion of doing sysctls "inside" sysinits (so > they are > registered last and unregistered first) is probably better than the > current > state and is a simpler fix than changing all sysctls to use SYSINITs. > But if module sysctls are registered last then the ones flagged as CTLFLAG_TUN won't have their tunable values populated before the MOD_LOAD handler runs, is that going to cause trouble? It would suck to have to handle things differently in a driver depending on whether you're compiled into the kernel or kldload'd interactively. -- Ian ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: zfs.ko no longer loads after r320156: unresolved symbol: abd_is_linear
On Wednesday, August 02, 2017 10:14:01 AM Andriy Gapon wrote:
> On 02/08/2017 04:00, Ngie Cooper (yaneurabeya) wrote:
> >
> >> On Aug 1, 2017, at 09:21, John Baldwin wrote:
> >>
> >> On Tuesday, August 01, 2017 09:47:41 AM Andriy Gapon wrote:
> >>> On 01/08/2017 02:31, Ngie Cooper wrote:
> Hi,
> I tried upgrading my host from 11.1-STABLE to 12.0-CURRENT, and it
> didn’t work because abd_is_linear is an undefined symbol (it exists in
> sys/conf/files, but not sys/modules/zfs/Makefile). I tried adding abd.c
> to sys/modules/zfs/Makefile and it didn’t immediately fix my compilation
> problem (ran into a linker error instead).
> If it isn’t fixed in the next few hours I’ll try my hand at fixing the
> problem.
> >>>
> >>> I am not sure what exact problem you have...
> >>> abd.c should be added to the list of source files via
> >>> .include "${SUNW}/uts/common/Makefile.files"
> >>>
> >>> Perhaps something to do with "inline"...
> >>
> >> Oh, yes. If you use -fno-inline-funcs or the like. I forgot to
> >> send this to Andriy earlier, but here is the fix I'm using:
> >>
> >> https://github.com/freebsd/freebsd/commit/574dc95cf8272e16f6d44aff6cb4e08dede08886
> >
> > Unfortunately… this is head, verbatim, which means that the bug still
> > exists.
> > This gives me an idea of where I should look though.
>
> The URL indeed suggests that the change should be in head, but it's not there
> as
> far as I can tell. I never saw it being committed.
Not yet. I'm trying to decide if 'static inline' is more correct (for me it
results in 3 separate copies of abd_is_linear in zfs.ko) vs using 'extern
inline'. The latter seems possibly more correct but more of a pain? I think
for that it needs to be extern in only a single file and 'inline' in the
header?
--
John Baldwin
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: order of executing MOD_LOAD and registering module sysctl-s
On Wednesday, August 02, 2017 12:39:36 PM Hans Petter Selasky wrote: > On 08/02/17 12:13, Andriy Gapon wrote: > > > > As far as I understand a module initialization routine is executed via the > > sysinit mechanism. Specifically, module_register_init is set up as the > > sysinit > > function for every module and it calls MOD_EVENT(mod, MOD_LOAD) to invoke > > the > > module event handler. > > > > In linker_load_file() I see the following code: > > linker_file_register_sysctls(lf); > > linker_file_sysinit(lf); > > > > I think that this means that any statically declared sysctl-s in the module > > would be registered before the module receives the MOD_LOAD event. > > It's possible that some of the sysctl-s could have procedures as handlers > > and > > they might access data that is supposed to be initialized by the module > > event > > handler. > > > > So, for example, running sysctl -a at just the right moment during the > > loading > > of a module might end up in an expected behavior (including a crash). > > > > Is my interpretation of how the code works correct? > > Can the order of linker_file_sysinit and linker_file_register_sysctls be > > changed > > without a great risk? > > > > Thank you! > > > > P.S. > > The same applies to: > > linker_file_sysuninit(file); > > linker_file_unregister_sysctls(file); > > > > Hi, > > Not sure if this answers your question. > > If a SYSCTL() is TUNABLE, it's procedure can be called when the sysctl > is created. Else the SYSCTL() procedure callback might be called right > after it's registered. I think there is an own subsystem in sys/kernel.h > which takes care of the actual SYSCTL() creation/destruction - after the > linker is involved. sysctl nodes are created explicitly via linker_file_register_sysctls, not via SYSINITs, so you can't order them with respect to other init functions. I think Andriy's suggestion of doing sysctls "inside" sysinits (so they are registered last and unregistered first) is probably better than the current state and is a simpler fix than changing all sysctls to use SYSINITs. -- John Baldwin ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: order of executing MOD_LOAD and registering module sysctl-s
On 08/02/17 12:13, Andriy Gapon wrote: As far as I understand a module initialization routine is executed via the sysinit mechanism. Specifically, module_register_init is set up as the sysinit function for every module and it calls MOD_EVENT(mod, MOD_LOAD) to invoke the module event handler. In linker_load_file() I see the following code: linker_file_register_sysctls(lf); linker_file_sysinit(lf); I think that this means that any statically declared sysctl-s in the module would be registered before the module receives the MOD_LOAD event. It's possible that some of the sysctl-s could have procedures as handlers and they might access data that is supposed to be initialized by the module event handler. So, for example, running sysctl -a at just the right moment during the loading of a module might end up in an expected behavior (including a crash). Is my interpretation of how the code works correct? Can the order of linker_file_sysinit and linker_file_register_sysctls be changed without a great risk? Thank you! P.S. The same applies to: linker_file_sysuninit(file); linker_file_unregister_sysctls(file); Hi, Not sure if this answers your question. If a SYSCTL() is TUNABLE, it's procedure can be called when the sysctl is created. Else the SYSCTL() procedure callback might be called right after it's registered. I think there is an own subsystem in sys/kernel.h which takes care of the actual SYSCTL() creation/destruction - after the linker is involved. --HPS ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
order of executing MOD_LOAD and registering module sysctl-s
As far as I understand a module initialization routine is executed via the sysinit mechanism. Specifically, module_register_init is set up as the sysinit function for every module and it calls MOD_EVENT(mod, MOD_LOAD) to invoke the module event handler. In linker_load_file() I see the following code: linker_file_register_sysctls(lf); linker_file_sysinit(lf); I think that this means that any statically declared sysctl-s in the module would be registered before the module receives the MOD_LOAD event. It's possible that some of the sysctl-s could have procedures as handlers and they might access data that is supposed to be initialized by the module event handler. So, for example, running sysctl -a at just the right moment during the loading of a module might end up in an expected behavior (including a crash). Is my interpretation of how the code works correct? Can the order of linker_file_sysinit and linker_file_register_sysctls be changed without a great risk? Thank you! P.S. The same applies to: linker_file_sysuninit(file); linker_file_unregister_sysctls(file); -- Andriy Gapon ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: zfs.ko no longer loads after r320156: unresolved symbol: abd_is_linear
On 02/08/2017 04:00, Ngie Cooper (yaneurabeya) wrote:
>
>> On Aug 1, 2017, at 09:21, John Baldwin wrote:
>>
>> On Tuesday, August 01, 2017 09:47:41 AM Andriy Gapon wrote:
>>> On 01/08/2017 02:31, Ngie Cooper wrote:
Hi,
I tried upgrading my host from 11.1-STABLE to 12.0-CURRENT, and it
didn’t work because abd_is_linear is an undefined symbol (it exists in
sys/conf/files, but not sys/modules/zfs/Makefile). I tried adding abd.c to
sys/modules/zfs/Makefile and it didn’t immediately fix my compilation
problem (ran into a linker error instead).
If it isn’t fixed in the next few hours I’ll try my hand at fixing the
problem.
>>>
>>> I am not sure what exact problem you have...
>>> abd.c should be added to the list of source files via
>>> .include "${SUNW}/uts/common/Makefile.files"
>>>
>>> Perhaps something to do with "inline"...
>>
>> Oh, yes. If you use -fno-inline-funcs or the like. I forgot to
>> send this to Andriy earlier, but here is the fix I'm using:
>>
>> https://github.com/freebsd/freebsd/commit/574dc95cf8272e16f6d44aff6cb4e08dede08886
>
> Unfortunately… this is head, verbatim, which means that the bug still
> exists.
> This gives me an idea of where I should look though.
The URL indeed suggests that the change should be in head, but it's not there as
far as I can tell. I never saw it being committed.
--
Andriy Gapon
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
