[ewg] Re: B/ipoib: Fix neigh destructor oops for kernels older than 2.6.21

[Dave Olson]

On Thu, 2008-05-15 at 08:27 +0700, Eli Cohen wrote:
| On Thu, 2008-05-15 at 18:10 +0300, Or Gerlitz wrote:
| > 1 day is not enough time to review the patch, as of the importance, I 
| > suggest you ask Roland and Alexey to review it and say what their 
| > opinion on the solution, now once we all understand the problem.
| > 
| I think I did ask, not explicitly but I guess it is understood that the
| patch was sent for review. Anyway now it's explicit: I am asking all the
| folks to review the patch and send comments. Alternate approaches
| towards solving the same problem are welcome too.

I don't think this patch actually fixes anything (or even really works
around the problem).   As soon as somebody unloads the new module, exactlyr
the same problem occurs.

When testing for memory leaks, and other bugs, you want to unload all
of the modules that you have loaded, over and over, and typically
you automate that, and such tests are going to unload this module as well,
since the new module is loaded by modprobe due to it's dependency, and
therefore is in the list of modules loaded by the test.

So I think it's better to do nothing at all, than a bandaid like this.

The "right" fix is to do the cleanup such that the core networking code
won't do the callback.  That may not be trivial or simple, but it's
really the only viable fix for the long run.   Bandaiding over it
doesn't seem very useful to me.

By the way, when a patch is proposed for an bug in the openfabrics
bugzilla (985/1021/1028) in this case), I think it would be helpful
(i.e., good policy) to attach the proposed patch to the bug, so that
people following the bug have some idea that work is being done (and
probably mention that discussion is occurring, and on what list; I would
have expected this discussion to be on the general list, rather than ewg).

In summary, my preference would be to simply leave the bug open, and not
fix the problem at all, rather than to bandaid over it in a fashion that
itself doesn't really solve any problems, just moves them over one step.
Of course, what I'd really like is a real fix, so that the neighbour
code is correctly cleaned up...

Thanks,

Dave Olson
dave.olson at qlogic.com