-- Dave
At 12:42 PM 2/16/2004, Brian F. G. Bidulock wrote:
Dave,
Been there, done that. That change does not help much. The
external module will not demand load because of the null modID
returned from lis_findmod (which only searches on a name that
is not there yet). Also, your patch exhibits the
characteristic that a "configured" module that is unloaded (by
rmmod) it will not load again. There are more problems. I
found that f_str is not set to NULL (and I did the same
LIS_MODSTATE_LOADED) thing.
If you would like to test your patch, as I did, try the
following:
Demand load a configured/loadable driver (e.g. sad) with open.
Remove the driver with rmmod. Demand load the same configured
driver.
Do the same thing (I_PUSH) for a configured/loadable module
(e.g. pipemod).
Do the same thing for an external/loadable driver.
Do the same thing for an external/loadable module.
The patch I sumbitted works under all these circumstances.
--brian
On Mon, 16 Feb 2004, Dave Grothe wrote:
>
> Modules can be loaded by any of the following processes:
> * Linked in with LiS (always there)
> * Loaded by LiS at I_PUSH time (always there after loading)
> * Loaded by external insmod
>
> In the first two cases the module has been "declared" to LiS in the
> Config file. In the third case LiS has no foreknowledge of the
> module.
> 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.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]
> > >>[1]http://gsyc.escet.urjc.es/mailman/listinfo/linux-streams
> > >
> > >
> >
> > _______________________________________________
> > Linux-streams mailing list
> > [EMAIL PROTECTED]
> > [2]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
> �
> [3]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]
> [4]http://gsyc.escet.urjc.es/mailman/listinfo/linux-streams
> ---
> Incoming mail is certified Virus Free.
> Checked by AVG anti-virus system ([5]http://www.grisoft.com).
> Version: 6.0.587 / Virus Database: 371 - Release Date: 2/12/2004
>
> References
>
> 1. http://gsyc.escet.urjc.es/mailman/listinfo/linux-streams
> 2. http://gsyc.escet.urjc.es/mailman/listinfo/linux-streams
> 3. http://www.openss7.org/
> 4. http://gsyc.escet.urjc.es/mailman/listinfo/linux-streams
> 5. http://www.grisoft.com/
>
> ---
> 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
--
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.591 / Virus Database: 374 - Release Date: 2/17/2004
