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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140918234309.GP17875@mwanda>
Date:	Fri, 19 Sep 2014 02:43:09 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Julia Lawall <Julia.Lawall@...6.fr>
Cc:	Oleg Drokin <oleg.drokin@...el.com>, devel@...verdev.osuosl.org,
	Andreas Dilger <andreas.dilger@...el.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	kernel-janitors@...r.kernel.org, linux-kernel@...r.kernel.org,
	HPDD-discuss@...1.01.org
Subject: Re: [PATCH] staging: lustre: llite: Use kzalloc and rewrite null
 tests

On Thu, Sep 18, 2014 at 10:24:02PM +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@...6.fr>
> 
> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/