[lkml]   [2010]   [May]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    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.

    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.


    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,

    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.



     \ /
      Last update: 2010-06-01 05:47    [W:0.023 / U:0.064 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site