- Linked in with LiS (always there)
- Loaded by LiS at I_PUSH time (always there after loading)
- Loaded by external insmod
lis_loadmod() is called from a couple of autopush places and from I_PUSH. If the module name is not known to LiS then it can't be implicitly loaded. So the module name must be in the table at the time that lis_loadmod() is called.
When a module is loaded externally via insmod it registers itself with LiS by calling lis_register_strmod(). This is where LiS determines whether the module was internal (or already loaded) or a new module which was therefore externally loaded.
When a module unregisters itself via lis_unregister_strmod() it makes a difference whether the module was linked in with LiS or loaded dynamically. If it was linked in then it is still in memory and the pointer to its streamtab is still valid. If it was loaded dynamically we presume that it is about to be unloaded and this pointer is (or shortly will be) invalid.
This is, I think, where Brian's problem occurred. LiS should erase all knowledge of the name of a loaded module when it unregisters itself.
The following patch is what I did to fix the problem.
Strtst has had a test in it for awhile that dynamically loads modules both via the "declared" path and the "insmod" path. But somehow it did not turn up Brian's problem. It may be that running strtst twice with no intervening unload of LiS (on 2.16.18) would exhibit the problem. I tried that with my change in place and nothing broke.
Warning: This patch is against my development 2.17 version so the line numbers are likely wrong for patching 2.16.18.
-- Dave
@@ -733,7 +733,6 @@
switch (slot->f_state & LIS_MODSTATE_MASK)
{
case LIS_MODSTATE_LINKED:
- case LIS_MODSTATE_LOADED:
break ;
case LIS_MODSTATE_LOADING:
@@ -741,8 +740,12 @@
printk("LiS: unregister module \"%s\" -- loading\n", slot->f_name) ;
return(-EBUSY) ;
+ case LIS_MODSTATE_LOADED:
case LIS_MODSTATE_UNLOADED:
slot->f_str = NULL;
+ lis_fmod_sw[id].f_state &= ~LIS_MODSTATE_MASK ;
+ lis_fmod_sw[id].f_state |= LIS_MODSTATE_UNLOADED ;
+ lis_fmod_sw[id].f_name[0] = 0 ;
break ;
}
At 11:35 PM 2/15/2004, Brian F. G. Bidulock wrote:
John,
Well, its really external loadable modules.
I_PUSH calls lis_loadmod which calls lis_findmod which doesn't
find anything on name and returns LIS_NULL_MODID which returns
from lis_loadmod with LIS_NULL_MODID which fails in I_PUSH.
I didn't have any problems with LiS included loadable modules
such as timod.
The above has not worked since 2.16.15 where lines such as:
if (id == LIS_NULL_MODID)
return LIS_NULL_MODID;
were added to the top of lis_loadmod.
I have also found that if f_name is not nulled, problems
ensue: lis_register_strmod looks for existing registered
modules on name first, and then never sets new_entry which
results in incorrect state.
I have also found that in unregister_module, f_str is never
nulled. (It is only nulled for the non-existent case where
f_state contains LIS_MODSTATE_UNLOADED, which should never
occur.) The result is that once loaded, f_str is left as a
hanging reference after module unloading and the module will
not reload.
There are so many bugs in this code I'm beginning to get angry
about it. The thing is not going to work without significant
rework. The original patch between 2.16.14 and 2.16.15 was a
very poor one. It might work for LiS included modules but is
completely busted for external modules. It was obviously
never tested for external modules.
Also, I don't see any need for the semaphore. Whoever calls
request_module already holds the big kernel lock.
--brian
On Sun, 15 Feb 2004, John A. Boyd Jr. wrote:
> Let me try to synchronize my recollection with yours.
>
> I submitted patches to Dave in response to brokenness of module (not
> driver) loading in 2.16.15, and the patched behavior was evident in
> 2.16.16. I.e., my patches were thus not the source of the problems
> you had, but were apparently a response to the same problem(s) you
> had and just might not have completely resolved them. That's pretty
> much consistent with what I first wrote about this issue last night,
> i.e., that I tried to fix it but don't know if I did it completely
> or not.
>
> But you say "in 2.16.15 forward, if the module is not found, it is never
> loaded." That's not my recollection at all. What I found was that in
> 2.16.15, if a module was not at first found to be loaded, it might
> actually get loaded, but the attempt would return failure state/status,
> thus falsely indicating that it didn't load. But since 2.16.16, my
> tests have shown that module loading works OK, as far as my tests go
> (I_PUSH/I_FIND). I just tested with 2.16.18, and I_PUSH/I_FIND still
> work OK for me.
>
> As I recall, the person who reported the problem with 2.16.15 was
> satisfied with the patched behavior of 2.16.16 as well.
>
> -John
>
> Brian F. G. Bidulock wrote:
> > John,
> >
> > Well, whoever made changes between 2.16.14 and 2.16.15 broke module
> > ("module" as opposed to "driver") demand loading altogether. Which
> > is where I started having problems.
> >
> > In 2.16.14 if lis_findmod could not find a module by name and KMOD
> > is configured, it would attempt to load module "streams-%s".
> >
> > In 2.16.15 forward, if the module is not found, it is never loaded.
> >
> > I'm patching back to the 2.16.14 lis_findmod. I'll send a patch in
> > a little while.
> >
> > --brian
> >
> > On Sun, 15 Feb 2004, John A. Boyd Jr. wrote:
> >
> >
> >>Let me restate in the form of a suggestion - instead of clearing
> >>the name, you might want to look at how to get the state change
> >>to work, so as to prevent this problem. The author apparently
> >>intended clearing the _INITED state flag to have somewhat the
> >>effect as your clearing of the name...
> >>
> >>-John
> >>
> >>Brian F. G. Bidulock wrote:
> >>
> >>>There is a module loading bug in head/mod.c When a module
> >>>is unregistered the f_name component is left alloing lis_findmod
> >>>to find it an lis_loadmod will attempt to lock a destroyed
> >>>semaphore. The problem can be recreated by manually loading a
> >>>streams kernel module (modprobe), unloading it (rmmod) and then
> >>>demand loading it (e.g. I_PUSH). The result is a kernel oops.
> >>>
> >>>The patch below fixes the problem (but may break other things).
> >>>
> >>>--brian
> >>>
> >>>Index: head/mod.c
> >>>===================================================================
> >>>RCS file: /home/common/cvsroot/LiSnew/head/mod.c,v
> >>>retrieving revision 1.1.1.4
> >>>diff -U3 -r1.1.1.4 mod.c
> >>>--- head/mod.c22 Nov 2003 23:01:43 -0000 1.1.1.4
> >>>+++ head/mod.c15 Feb 2004 18:05:31 -0000
> >>>@@ -742,6 +742,7 @@
> >>> lis_up(&slot->f_sem) ;
> >>> lis_sem_destroy(&slot->f_sem) ;
> >>> slot->f_state &= ~LIS_MODSTATE_INITED ;
> >>>+ slot->f_name[0] = '\0';
> >>>
> >>> printk("STREAMS module \"%s\" unregistered, id %d\n", name, id);
> >>>
> >>>
> >>
> >>_______________________________________________
> >>Linux-streams mailing list
> >>[EMAIL PROTECTED]
> >>http://gsyc.escet.urjc.es/mailman/listinfo/linux-streams
> >
> >
>
> _______________________________________________
> Linux-streams mailing list
> [EMAIL PROTECTED]
> http://gsyc.escet.urjc.es/mailman/listinfo/linux-streams
--
Brian F. G. Bidulock � The reasonable man adapts himself to the �
[EMAIL PROTECTED] � world; the unreasonable one persists in �
http://www.openss7.org/ � trying to adapt the world to himself. �
� Therefore all progress depends on the �
� unreasonable man. -- George Bernard Shaw �
_______________________________________________
Linux-streams mailing list
[EMAIL PROTECTED]
http://gsyc.escet.urjc.es/mailman/listinfo/linux-streams
---
Incoming mail is certified Virus Free.
Checked by AVG anti-virus system (http://www.grisoft.com).
Version: 6.0.587 / Virus Database: 371 - Release Date: 2/12/2004
--- Outgoing mail is certified Virus Free. Checked by AVG anti-virus system (http://www.grisoft.com). Version: 6.0.587 / Virus Database: 371 - Release Date: 2/12/2004
