Messages in this thread | | | From | "Drokin, Oleg" <> | Subject | Re: [HPDD-discuss] [PATCH] staging: lustre: llite: Use kzalloc and rewrite null tests | Date | Fri, 19 Sep 2014 13:36:24 +0000 |
| |
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-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |