Re: [PATCH] staging: lustre: llite: Use kzalloc and rewrite null tests

From: Dan Carpenter
Date: Thu Sep 18 2014 - 19:43:36 EST


On Thu, Sep 18, 2014 at 10:24:02PM +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@xxxxxxx>
>
> This patch removes some kzalloc-related macros and rewrites the
> associated null tests to use !x rather than x == NULL.
>

This is sort of exactly what Oleg asked us not to do in his previous
email. ;P

I think there might be ways to get good enough tracing using standard
kernel features but it's legitimately tricky to update everything to
using mempools or whatever. Maybe we should give Oleg some breathing
room to do this.

I hate looking at the OBD_ALLOC* macros, but really it's not as if we
don't have allocation helper functions anywhere in the rest of the
kernel. It's just that the style of the lustre helpers is so very very
ugly. It took me a while to spot that OBD_ALLOC() zeroes memory, for
example.

It should be relatively easy to re-write the macros so we can change
formats like this:

old: OBD_ALLOC(ptr, size);
new: ptr = obd_zalloc(size, GFP_NOFS);

old: OBD_ALLOC_WAIT(ptr, size);
new: ptr = obd_zalloc(size, GFP_KERNEL);

old: OBD_ALLOC_PTR(ptr);
new: ptr = obd_zalloc(sizeof(*ptr), GFP_NOFS);

etc...

Writing it this way means that we can't put the name of the pointer
we're allocating in the debug output but we could use the file and line
number instead or something. Oleg, what do you think?

If we decide to mass convert to standard functions later then it's dead
simple to do that with sed.

The __OBD_MALLOC_VERBOSE() is hard to read. It has side effect bugs if
you try to call OBD_ALLOC(ptr++, size); The kernel already has a way to
inject kmalloc() failures for a specific module so that bit can be
removed. Read Documentation/fault-injection/fault-injection.txt

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/