lkml.org 
[lkml]   [2010]   [May]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] module: fix bne2 "gave up waiting for init of module libcrc32c"


On Tue, 1 Jun 2010, Rusty Russell wrote:

> On Tue, 1 Jun 2010 05:45:37 am Linus Torvalds wrote:
> >
> > basically, we should do the whole module dependency list regardless
> > of whether we can unload modules or not
>
> Why?

Because the current non-CONFIG_MODULE_UNLOAD code is currently broken, and
it was broken exactly because the code had two totally different paths and
totally different logic. And one part simply missed the case.

We'd be much better off having as much of the logic shared as possible.
No?

Your 2/2 actually fixed that, because it moved the broken
wait_event_interruptible_timeout() out of the (non-shared) use_module()
into the (shared) resolve_symbol_wait(). But even that seemed to be almost
accidental, and seemed to be more about the fact that now the locking
rules required it (if you wanted to wait without holding the lock), rather
than anything else.

In contrast, wouldn't it be nice if we just made the waiting be a separate
event entirely? And have the !MODULE_UNLOAD case also able to share the
/proc/module format, for example?

You do realize - although maybe you don't - that right now, even with your
2/2 case, the !MODULE_UNLOAD case seems to get the module ref-counts
wrong. I didn't look through it all, but it _seems_ to increment the
ref-count for every symbol it resolves.

What happens if the module load fails in the middle (say, because some
symbols referred to module 'a' that was loaded successfully, and others
refer to module 'b' that had some probelsm)? I think the !MODULE_UNLOAD
case is _broken_ again.

Why?

Take a small guess. It's broken - even with your 2/2 - exactly because the
!MODULE_UNLOAD case does something fundamentally different from the
regular case. It increments the module counts for each symbol it looks up,
exactly BECAUSE IT DOESN'T TRACK MODULE DEPENDENCIES!

Which means that it then cannot clean up properly afterwards if an error
happens in the middle.

So I'd suggest we should just track those module dependencies, and share
more of the code and the logic. Because it looks to me like not sharing it
continually results in bugs.

No?

Linus


\
 
 \ /
  Last update: 2010-06-01 05:47    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site