3:36 p.m.
Hello!
First, thanks for your patches and efforts spent on these cleanups.
On Sep 19, 2014, at 12:45 AM, Julia Lawall wrote:
With respect to the upper case lower case issue, does the thing need
to be
a macro? I think that the lowercase is more or less fine, but only if
what is behind it is a function.
I don't have a strong opinion either way as long as we have all the functionality
we need.
I say more or less fine, because normally in the kernel the special
allocators have special purposes, eg allocating and initializing the xyz
structure. Here what is wanted is a general purpose allocator with lots
of special tracing features, so it is not quite the same thing. And one
can wonder why all of these special tracing features are not relevant to
the kernel as a whole?
Like I explained in my previous email, many of the tracing features are already
possible to replace with other existing in-kernel mechanisms like kmemleak.
Except the total tally of allocations/frees so that a memleak could be visibly
easily seen on module unload time. I think this would be useful for other
kinds of modules too, not just lustre, so having that as a generic allocator
feature would be cool too.
In reading through the description of the needed features, it seems
like
only the _ptr extension requires being a macro. Do we need that? The
We only need that as a less error-prone way of having
x = obd_kzalloc(sizeof(*x), ….)
…
obd_free(…, sizeof(*x))
Real free function does not take size argument, but we need that for
total allocated/freed accounting. Easy to have disconnect with
the size argument of obd_free to be wrong.
rest of the kernel manages to do x = kzalloc(sizeof(*x),...) ok.
It's
unpleasant to have an assignment hidden in this way. And currently it is
not used consistently. There are some OBD_ALLOCs that have the same form.
Yes, those are converted as thy are noticed.
Sorry for overlooking the frees. I was focusing on trying one thing
at a
time...
I kind of think it's a related issue.
Touching ones needs to touch the other if not in the same patch then in
a next patch. And that's why I think consideations for what FREEs would need
is needed from the start, so the FREEs removal patch does not goes and patches a bunch of
just patched allocs.
Thanks.
Bye,
Oleg
2800
days inactive
2800
days old
0 comments
1 participants
participants (1)
-
Drokin, Oleg