lkml.org 
[lkml]   [2010]   [Jun]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 2/2] module: fix bne2 "gave up waiting for init of module libcrc32c"
Date
On Wed, 2 Jun 2010 02:26:52 pm Linus Torvalds wrote:
>
> On Wed, 2 Jun 2010, Rusty Russell wrote:
> >
> > OK, this might be worthwhile. Take the very first mail you sent:
> >
> > Hmm. That does seem to be buggy. We can't just drop and re-take the lock:
> > that may make sense internally as far as resolve_symbol() itself is
> > concerned, but the caller will its own local variables, and some of those
> > will no longer be valid if the lock was dropped.
> >
> > The implication here is that that I don't know locking, and that this was
> > done without thought or care. In fact, I did worry about it and decided it
> > was less risky than a locking reduction given my limited cycles (indeed,
> > it has been slightly).
>
> Well, part of the context here is that the commit had been bisected as
> being buggy. It turns out the bug was a different issue, but at the same
> time, it very much looked like the locking was simply known a-priori to be
> buggy. No?

Hmm, you're still missing it. Let me try again.

You weren't the best person to make the call. That didn't occur to you, did
it?

Subsystem maintainers aren't just patch monkeys, we collect all the metadata
which informs these decisions. Without that, you wandered in, came to the
obvious conclusion, and were wrong.

Really grating is the arrogant lack of respect for that knowledge combined
with your multiple mistakes since then due to your lack of it. See below.

> > That commit also changes the return value semantics of "use_module()",
> > which is an exported interface where the only users seem to be
> > out-of-kernel (the only in-kernel use is in kernel/module.c itself). That
> > seems like a really really bad idea too.
> >
> > Again with the implication that this was done without consideration.
>
> No. The implication was that it's wrong to do and a bad idea. Anything
> else is just you reading things into it.
>
> It's an exported interface, and you changed it with no real reason.
> Whether you then talked to users or not is immaterial.
>
> You could easily have just changed the _internal_ thing, and exported the
> unchanged interface.

I *genuinely* thought our policy with out-of-tree code was to smother
out-of-tree code with absolute apathy so they'll get sick of riding the
churn.

Still, easy to avoid in this case.

> Even your second version is just very confused. It renames it, but then
> exports the renamed version. What does that help?

This *almost* looks like an honest "hey, I don't understand this".

But no, "very confused" already assumes you're right. You do ask if it
helps...

> It still means that the external module - for no good reason - needs to
> have basically a source-level version number check.

...and go on to assume it doesn't. I must be really confused, right?

Or maybe, I know something you don't about the code I maintain.

We have infrastructure to look up symbols dynamically: symbol_get().

Sure, they have to adapt their code, but now they can maintain their own
damn wrapper, *and* they'll discover the change as soon as they try to
build their module against the new kernel.

I'd go so far as to say it's clever...
Rusty.


\
 
 \ /
  Last update: 2010-06-02 07:55    [W:0.143 / U:0.100 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site