[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: Proposal: kmem_valloc [was: Re: raspberry pi panic 7.0_BETA after install fs resize]
Le 09/11/2014 09:43, Michael van Elst a écrit :
On Sat, Nov 08, 2014 at 07:46:08PM +0100, Maxime Villard wrote:
Le 08/11/2014 19:08, Michael van Elst a écrit :
As a side effect, it removed the debugging features of malloc/free.
As well as it allowed many bugs.
I doubt that it allowed many bugs. It made code slightly more complex,
but it also made people look at what the code does.
Yes; it made code slightly more complex, complex enough to introduce
bugs, bugs that then got fixed after looking at what the code does.
That's exactly what we saw in NetBSD-SA2014-014.
For the most recent issue in ffs, it might even have exposed a bug
as the change to the fs structure apparently did not cause a
re-allocation of the csum structure, which is the real problem.
Fixing only the de-allocation just hides that bug.
Yes, ffs_reload() is completely buggy: it re-reads and changes many
values without reallocating anything. You will also notice that the
whole thing is ready to collapse: bread() takes an int argument,
which can trigger a panic later; there are also several divisions
by zero, etc.
But what you say is I think a bit paradoxical. Yes, kmem_alloc might
have unearthed a bug in ffs. But why did it find this bug? Because I
enabled KMEM_SIZE. If I hadn't enabled it some months ago, kmem_alloc
wouldn't have detected a bug, and the situation would have been even
worse: there is a bug in the allocation, *and* a memory corruption
This is all the more paradoxical when we know that my proposal of
enabling KMEM_SIZE and KMEM_REDZONE was welcomed with scepticism.
So no, kmem_alloc hasn't been designed precisely to unroot lurking
bugs. Now it finds bugs only in architectures where DIAGNOSTIC
is enabled; when it is not, it adds a memory corruption.
That's what motivates my proposal: creating kmem_valloc and kmem_vfree,
to definitely get rid of malloc, and have another set of functions
that can perform allocations/deallocations without needing a size
argument. The 'v' stands for "variable".
Looks like the worst of both worlds.
Could you elaborate a bit?
- almost irrelevant optimization
- requires tracking of allocations
+ could validate alloc vs. free size
- some overhead for tracking + tagging allocations
+ simpler code for fixed data structures
-/+ not really a constructor/deconstructor
+ tagging allocations helps debugging (*)
(*) Yes, I know that this is only a dummy parameter now.
Your suggestion would
- add another interface
- add some overhead
- be used for a few fringe cases, unless it replaces kmem_alloc/kmem_free
-/+ would not help debugging like malloc/free
Help debugging? Tagging allocations?
Look at sys/sys/malloc.h:
#define malloc(size, type, flags) kern_malloc(size, flags)
The tag is not even compiled in. Thus it can hardly help debugging
You understand why I say that malloc is dead.
You can see that I consider the malloc/free API better than
kmem_alloc/kmem_free. But going half-a-step to create yet
another interface isn't the right thing.
It creates an interface, that is supposed to replace an old, dead one:
Main Index |
Thread Index |