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.c     22 Nov 2003 23:01:43 -0000      1.1.1.4
> >>>+++ head/mod.c     15 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

Reply via email to